Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature/expose-login-method #1652

Merged
merged 4 commits into from
Oct 13, 2020
Merged

feature/expose-login-method #1652

merged 4 commits into from
Oct 13, 2020

Conversation

ionutcirja
Copy link
Contributor

Description

Changes proposed in this pull request:

  • expose login method

@CLAassistant
Copy link

CLAassistant commented Oct 8, 2020

CLA assistant check
All committers have signed the CLA.

@hardl hardl added this to the Sprint 13 milestone Oct 8, 2020
@alexandra-simeonova alexandra-simeonova self-assigned this Oct 9, 2020
@ndricimrr ndricimrr self-assigned this Oct 9, 2020
@alexandra-simeonova
Copy link
Contributor

Thank you for your contribution! 🚀
A friendly tip: because API documentation is different from other documentation in Luigi (it is automatically generated), we'd like to ask you to run lerna run docu from the root Luigi folder. (If you didn't install Lerna yet, look here.) This should then generate the documentation.

@ionutcirja
Copy link
Contributor Author

Hello guys, do you have any idea how soon can you merge this? Thanks.

@maxmarkus
Copy link
Contributor

maxmarkus commented Oct 12, 2020

Hello @ionutcirja

I have checked your PR and it is working. Since I would like to see the the stuff tested, I have created the e2e test for you.
Would you please add it to fiddle-navigation.spec.js:338 after the No profile, logged intest?

      it('Trigger Login and Logout with Core API', () => {
        newConfig.navigation.profile = undefined;
        newConfig.auth.disableAutoLogin = true;
        visitWithAuthConfig('/', newConfig);

        cy.get('[data-testid="luigi-topnav-profile"] button').click();
        loginLink().should('exist');

        cy.window().then(win => {
          cy.log('Trigger auth().login()');
          win.Luigi.auth().login();
        });

        cy.login('[email protected]', 'tets', true);

        cy.get('[data-testid="luigi-topnav-profile"] button').click();
        logoutLink().should('exist');

        cy.window().then(win => {
          cy.log('Trigger auth().logout()');
          win.Luigi.auth().logout();
        });

        cy.contains('Login again');
      });

FYI: To let this test run locally, run website/fiddle: npm start and test/e2e-test-application: npm run e2e:open and click on the fiddle-navigation.spec.js.

We will try to merge it today, then you would be able to use the nightly (-dev) build tomorrow.

Copy link
Contributor

@maxmarkus maxmarkus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks! 👍

@maxmarkus
Copy link
Contributor

Thank you for your contribution! 🚀
A friendly tip: because API documentation is different from other documentation in Luigi (it is automatically generated), we'd like to ask you to run lerna run docu from the root Luigi folder. (If you didn't install Lerna yet, look here.) This should then generate the documentation.

I guess a missing npm install in the root folder is the root-cause, it would install the pre-commit and pre-push hooks that do the job. @ionutcirja be sure to run npm install in the main repo folder to be safe for the future.

@maxmarkus maxmarkus merged commit 13ef2c3 into SAP:master Oct 13, 2020
@ndricimrr ndricimrr added the enhancement New feature or request label Oct 28, 2020
@ndricimrr ndricimrr mentioned this pull request Oct 28, 2020
stanleychh pushed a commit to stanleychh/luigi that referenced this pull request Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants