-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
More tidying prior to Models in Browse Data #38126
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
1fd7280
Make embedding-full-app.cy.spec.js and associated components more sem…
rafpaf 72a2733
Make signin.cy.spec.js and associated components more semantic
rafpaf 493a772
Fix comment
rafpaf 586336c
minor tweak
rafpaf 86ea8af
Remove aria-label=breadcrumbs
rafpaf cafd5b0
Fix variable name
rafpaf f6d1e82
Fix e2e tests
rafpaf 507caec
Remove cy.heading
rafpaf d67e988
Restore test name
rafpaf 5d75fe7
Merge branch 'master' into models-in-browse-data--more-initial-cleanup
rafpaf File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,40 +30,36 @@ describe("scenarios > auth > signin", () => { | |
cy.visit("/"); | ||
cy.findByLabelText("Email address").type(admin.email); | ||
cy.findByLabelText("Password").type("INVALID" + admin.password); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.findByText("Sign in").click(); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.contains("did not match stored password"); | ||
cy.button("Sign in").click(); | ||
cy.findByRole("alert") | ||
.filter(':contains("did not match stored password")') | ||
.should("be.visible"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
}); | ||
|
||
it("should display same error for unknown users (to avoid leaking the existence of accounts)", () => { | ||
cy.visit("/"); | ||
cy.findByLabelText("Email address").type("INVALID" + admin.email); | ||
cy.findByLabelText("Password").type(admin.password); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.findByText("Sign in").click(); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.contains("did not match stored password"); | ||
cy.button("Sign in").click(); | ||
cy.findByRole("alert") | ||
.filter(':contains("did not match stored password")') | ||
.should("be.visible"); | ||
}); | ||
|
||
it("should greet users after successful login", () => { | ||
cy.visit("/auth/login"); | ||
cy.findByLabelText("Email address").should("be.focused").type(admin.email); | ||
cy.findByLabelText("Password").type(admin.password); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.findByText("Sign in").click(); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.contains(/[a-z ]+, Bob/i); | ||
cy.button("Sign in").click(); | ||
cy.findByTestId("greeting-message").should("contain.text", "Bobby"); | ||
}); | ||
|
||
it("should allow login regardless of login email case", () => { | ||
cy.visit("/auth/login"); | ||
cy.findByLabelText("Email address").type(admin.email.toUpperCase()); | ||
cy.findByLabelText("Password").type(admin.password); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.findByText("Sign in").click(); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.contains(/[a-z ]+, Bob/i); | ||
cy.button("Sign in").click(); | ||
cy.findByTestId("greeting-message").should("contain.text", "Bobby"); | ||
}); | ||
|
||
it("should allow toggling of Remember Me", () => { | ||
|
@@ -76,34 +72,26 @@ describe("scenarios > auth > signin", () => { | |
cy.findByRole("checkbox").should("not.be.checked"); | ||
}); | ||
|
||
it("should redirect to a unsaved question after login", () => { | ||
it("should redirect to an unsaved question after login", () => { | ||
cy.signInAsAdmin(); | ||
cy.visit("/"); | ||
// Browse data moved to an icon | ||
browse().click(); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.contains("Sample Database").click(); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.contains("Orders").click(); | ||
cy.findByRole("heading", { name: "Sample Database" }).click(); | ||
cy.findByRole("heading", { name: "Orders" }).click(); | ||
cy.wait("@dataset"); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.contains("37.65"); | ||
cy.findAllByRole("gridcell", { name: "37.65" }); | ||
|
||
// signout and reload page with question hash in url | ||
cy.signOut(); | ||
cy.reload(); | ||
|
||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.contains("Sign in to Metabase"); | ||
cy.findByRole("heading", { name: "Sign in to Metabase" }); | ||
cy.findByLabelText("Email address").type(admin.email); | ||
cy.findByLabelText("Password").type(admin.password); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.findByText("Sign in").click(); | ||
cy.button("Sign in").click(); | ||
|
||
// order table should load after login | ||
cy.wait("@dataset"); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.contains("37.65"); | ||
cy.findAllByRole("gridcell", { name: "37.65" }); | ||
}); | ||
|
||
sizes.forEach(size => { | ||
|
@@ -116,11 +104,11 @@ describe("scenarios > auth > signin", () => { | |
|
||
cy.visit("/"); | ||
cy.url().should("contain", "auth/login"); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.findByText("I seem to have forgotten my password").click(); | ||
cy.findByRole("link", { | ||
name: "I seem to have forgotten my password", | ||
}).click(); | ||
cy.url().should("contain", "auth/forgot_password"); | ||
// eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | ||
cy.findByText("Back to sign in").click(); | ||
cy.findByRole("link", { name: "Back to sign in" }).click(); | ||
cy.url().should("contain", "auth/login"); | ||
}); | ||
}); | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this test is actually ensuring that the side nav, not the top nav, is not rendered
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, @WiNloSt originally wrote these tests.
I think the test is doing what it says on the tin - if we hide the side navigation and hide the logo, we want to make sure the top bar is not rendered.
There is a test below that explicitly makes sure nav bar is hidden via param.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for helping me understand. I've reverted that change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nemanjaglumac got it exactly right. That test was added during the implementation of hiding Metabase logo.
For context if you're interested: