Cypress Test for language selection and link redirection #9238 #9258
Conversation
WalkthroughThe changes introduce enhancements to the Cypress end-to-end tests for the login functionality, focusing on language support and link redirection. New constant objects for language mappings and sidebars are added, along with additional test cases to verify the functionality of language switching and link redirection. The Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (6)
cypress/e2e/auth_spec/redirect.cy.ts (2)
4-18: Consider moving translations to dedicated i18n filesWhile the current implementation works, having translations directly in the test file makes maintenance harder. Consider moving these translations to dedicated i18n files and importing them.
Example structure:
// cypress/fixtures/i18n/login.json { "buttonText": { "hi": "लॉग इन करें", // ... other languages }, "sidebar": { "hi": "देखभालहमारा लक्ष्य...", // ... other languages } }Also, fix the formatting issues:
- kr: "ಲಾಗಿನ್", //kannada + kr: "ಲಾಗಿನ್", // Kannada🧰 Tools
🪛 eslint
[error] 4-4: Delete
·(prettier/prettier)
[error] 11-12: Delete
⏎(prettier/prettier)
[error] 17-17: Replace
·//kannada·with,·//kannada(prettier/prettier)
49-59: Add error handling for failed redirectionsWhile the tests verify successful redirections, they should also handle failed redirections gracefully. Consider adding timeout and error handling.
it("Verify redirection of 'Contribute on GitHub' link", () => { loginPage.clickContributeOnGitHub(); - cy.url().should("include", "https://github.com/ohcnetwork"); - cy.get("body").contains("Contribute on GitHub") + cy.url({ timeout: 10000 }).should("include", "https://github.com/ohcnetwork"); + cy.get("body", { timeout: 10000 }) + .should("be.visible") + .and("contain", "Contribute on GitHub"); });🧰 Tools
🪛 eslint
[error] 49-49: Delete
··(prettier/prettier)
[error] 52-52: Insert
;(prettier/prettier)
[error] 54-54: Delete
··(prettier/prettier)
cypress/pageobject/Login/LoginPage.ts (4)
41-46: Consider defining selectors for username and password fieldsFor consistency and ease of maintenance, consider adding class properties for the username and password input selectors, similar to other selectors.
Apply this enhancement:
+ usernameSelector = "input[id='username']"; + passwordSelector = "input[id='password']"; ensurePageLoaded() { - cy.get("input[id='username']").should("be.visible"); - cy.get("input[id='password']").should("be.visible"); + cy.get(this.usernameSelector).should("be.visible"); + cy.get(this.passwordSelector).should("be.visible"); }
60-62: Ensure text comparison handles whitespaceWhen verifying the submit button text, consider trimming whitespace or using a regex to prevent false negatives due to formatting.
Update the assertion:
- cy.get(this.submitButtonSelector).should("have.text", expectedText); + cy.get(this.submitButtonSelector).should(($btn) => { + expect($btn.text().trim()).to.equal(expectedText); + });
80-82: ReuseselectLanguagemethod for sidebar language selectionThe
selectSidebarLanguagemethod duplicates the functionality ofselectLanguage. Consider reusingselectLanguagefor consistency.Replace:
- selectSidebarLanguage(languageCode: string) { - cy.get(this.languageSelector).select(languageCode); - }With:
// Remove `selectSidebarLanguage` method and use `selectLanguage` instead.
84-86: Ensure accurate verification of sidebar textThe
verifySidebarTextmethod checks the entire text content of the sidebar. If the sidebar contains nested elements or formatting, this may cause false negatives.Consider updating the assertion:
- cy.get(this.sidebarSelector).should("have.text", expectedText); + cy.get(this.sidebarSelector).should("contain.text", expectedText);Or verify specific elements within the sidebar for more precise assertions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
cypress/e2e/auth_spec/redirect.cy.ts(2 hunks)cypress/pageobject/Login/LoginPage.ts(2 hunks)
🧰 Additional context used
🪛 eslint
cypress/e2e/auth_spec/redirect.cy.ts
[error] 4-4: Delete ·
(prettier/prettier)
[error] 11-12: Delete ⏎
(prettier/prettier)
[error] 17-17: Replace ·//kannada· with ,·//kannada
(prettier/prettier)
[error] 49-49: Delete ··
(prettier/prettier)
[error] 52-52: Insert ;
(prettier/prettier)
[error] 54-54: Delete ··
(prettier/prettier)
[error] 65-65: Insert ·
(prettier/prettier)
[error] 66-66: Insert ·
(prettier/prettier)
[error] 67-68: Delete ⏎
(prettier/prettier)
🪛 Biome (1.9.4)
cypress/pageobject/Login/LoginPage.ts
[error] 70-70: Expected an expression but instead found '.'.
Expected an expression here.
(parse)
[error] 74-74: Expected an expression but instead found '.'.
Expected an expression here.
(parse)
[error] 76-76: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
[error] 92-92: Expected an expression but instead found '.'.
Expected an expression here.
(parse)
[error] 72-76: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
🔇 Additional comments (4)
cypress/e2e/auth_spec/redirect.cy.ts (1)
23-24: LGTM! Good addition of page load verification
Adding ensurePageLoaded() in the beforeEach hook improves test reliability by ensuring the page is fully loaded before each test.
cypress/pageobject/Login/LoginPage.ts (3)
4-6: Addition of selectors improves maintainability
The addition of submitButtonSelector, languageSelector, and sidebarSelector centralizes the selectors, enhancing code readability and maintainability.
56-58: Language selection method is appropriately implemented
The selectLanguage method correctly selects the desired language based on the provided language code.
88-95:
Fix syntax errors in switchLanguageAndVerifySidebars method
Similar to the previous method, there's a syntax error due to improper chaining. Ensure the commands are correctly chained without unnecessary braces.
Apply this fix:
Object.entries(languageMappings).forEach(([languageCode, expectedText]) => {
this.selectLanguage(languageCode);
- cy.get(this.sidebarSelector, { timeout: 10000 })
- .should("be.visible")
- .and("have.text", expectedText);
+ cy.get(this.sidebarSelector, { timeout: 10000 })
+ .should("be.visible")
+ .and("contain.text", expectedText);
});Additionally, using .and("contain.text", expectedText) accounts for partial matches and nested elements.
Likely invalid or redundant comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 92-92: Expected an expression but instead found '.'.
Expected an expression here.
(parse)
| it("Switch languages and verify login button text", () => { | ||
| loginPage.switchLanguageAndVerifyButtonText(languageMappings); | ||
| }); | ||
|
|
||
| it("Switch languages and verify sidebar items", ()=> { | ||
| loginPage.switchLanguageAndVerifySidebars(languageSidebars); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add verification for language persistence
The tests verify language switching but don't check if the selected language persists after page reload or navigation. Consider adding these scenarios:
it("Verify language persistence after page reload", () => {
loginPage.switchLanguage("hi");
cy.reload();
loginPage.verifyButtonText(languageMappings.hi);
});
it("Verify language persistence after navigation", () => {
loginPage.switchLanguage("hi");
cy.visit("/some-other-page");
cy.go("back");
loginPage.verifyButtonText(languageMappings.hi);
});🧰 Tools
🪛 eslint
[error] 65-65: Insert ·
(prettier/prettier)
[error] 66-66: Insert ·
(prettier/prettier)
🛠️ Refactor suggestion
Add test for login flow with different languages
The PR objectives mention testing actual login with non-English languages. Add a test to verify the complete login flow:
it("Successfully login with non-English language", () => {
// Test with Hindi
loginPage.switchLanguage("hi");
loginPage.loginManuallyAsDistrictAdmin();
loginPage.ensureLoggedIn();
cy.url().should("include", "/resource/board");
// Verify sidebar is in Hindi
cy.get("sidebar-selector").should("contain", languageSidebars.hi);
});🧰 Tools
🪛 eslint
[error] 65-65: Insert ·
(prettier/prettier)
[error] 66-66: Insert ·
(prettier/prettier)
| clickThirdPartyLicense() { | ||
| cy.get('a[href="/licenses"]').scrollIntoView().click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using hardcoded URLs in selectors
Similarly, replace the hardcoded URL selector 'a[href="/licenses"]' with a data attribute or stable selector to prevent fragility.
Apply this change:
- cy.get('a[href="/licenses"]').scrollIntoView().click();
+ cy.get('[data-testid="third-party-license-link"]').scrollIntoView().click();Add data-testid="third-party-license-link" to the corresponding link element in the application's HTML.
Committable suggestion skipped: line range outside the PR's diff.
| clickContributeOnGitHub() { | ||
| cy.get('a[href="https://github.com/ohcnetwork"]').scrollIntoView().click(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid using hardcoded URLs in selectors
Using hardcoded URLs like 'a[href="https://github.com/ohcnetwork"]' can make tests brittle if the URL changes. Instead, use a data attribute or a more stable selector.
Apply this change:
- cy.get('a[href="https://github.com/ohcnetwork"]').scrollIntoView().click();
+ cy.get('[data-testid="contribute-github-link"]').scrollIntoView().click();Ensure the link element in the application has the attribute data-testid="contribute-github-link".
Committable suggestion skipped: line range outside the PR's diff.
| switchLanguageAndVerifyButtonText(languageMappings: { | ||
| [key: string]: string; | ||
| }) { | ||
| Object.entries(languageMappings).forEach(([languageCode, expectedText]) => { | ||
| this.selectLanguage(languageCode); | ||
| + cy.get(this.languageSelector) | ||
| + .find(`option[value="${languageCode}"]`) | ||
| + .should('exist') | ||
| + .then(() => { | ||
| + cy.get(this.submitButtonSelector, { timeout: 10000 }) | ||
| + .should("be.visible") | ||
| + .should("have.text", expectedText); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Fix syntax errors in switchLanguageAndVerifyButtonText method
There is a syntax error due to improper use of .then() and unmatched braces. The .then() is unnecessary here and can be removed to fix the issue.
Apply this fix:
Object.entries(languageMappings).forEach(([languageCode, expectedText]) => {
this.selectLanguage(languageCode);
- cy.get(this.languageSelector)
- .find(`option[value="${languageCode}"]`)
- .should('exist')
- .then(() => {
- cy.get(this.submitButtonSelector, { timeout: 10000 })
- .should("be.visible")
- .should("have.text", expectedText);
- });
+ cy.get(this.languageSelector)
+ .find(`option[value="${languageCode}"]`)
+ .should('exist');
+ cy.get(this.submitButtonSelector, { timeout: 10000 })
+ .should("be.visible")
+ .and("have.text", expectedText);
});This removes the unnecessary .then() and ensures proper chaining of Cypress commands.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| switchLanguageAndVerifyButtonText(languageMappings: { | |
| [key: string]: string; | |
| }) { | |
| Object.entries(languageMappings).forEach(([languageCode, expectedText]) => { | |
| this.selectLanguage(languageCode); | |
| + cy.get(this.languageSelector) | |
| + .find(`option[value="${languageCode}"]`) | |
| + .should('exist') | |
| + .then(() => { | |
| + cy.get(this.submitButtonSelector, { timeout: 10000 }) | |
| + .should("be.visible") | |
| + .should("have.text", expectedText); | |
| }); | |
| }); | |
| } | |
| switchLanguageAndVerifyButtonText(languageMappings: { | |
| [key: string]: string; | |
| }) { | |
| Object.entries(languageMappings).forEach(([languageCode, expectedText]) => { | |
| this.selectLanguage(languageCode); | |
| cy.get(this.languageSelector) | |
| .find(`option[value="${languageCode}"]`) | |
| .should('exist'); | |
| cy.get(this.submitButtonSelector, { timeout: 10000 }) | |
| .should("be.visible") | |
| .and("have.text", expectedText); | |
| }); | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 70-70: Expected an expression but instead found '.'.
Expected an expression here.
(parse)
[error] 74-74: Expected an expression but instead found '.'.
Expected an expression here.
(parse)
[error] 76-76: Expected a statement but instead found ')'.
Expected a statement here.
(parse)
[error] 72-76: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
|
👋 Hi, @Kaushikgtm, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
| @@ -1,10 +1,27 @@ | |||
| import LoginPage from "../../pageobject/Login/LoginPage"; | |||
|
|
|||
| describe("redirect", () => { | |||
| const languageMappings = { | |||
| hi: "लॉग इन करें", // Hindi | |||
There was a problem hiding this comment.
Import from the json files.
| }; | ||
| const languageSidebars = { | ||
|
|
||
| hi: "देखभालहमारा लक्ष्य डिजिटल उपकरणों का उपयोग करके सार्वजनिक स्वास्थ्य सेवाओं की गुणवत्ता और पहुंच में निरंतर सुधार करना है। ओपन हेल्थकेयर नेटवर्क एक ओपन-सोर्स पब्लिक यूटिलिटी है जिसे इनोवेटर्स और स्वयंसेवकों की एक बहु-विषयक टीम द्वारा डिज़ाइन किया गया है। ओपन हेल्थकेयर नेटवर्क केयर एक डिजिटल पब्लिक गुड है जिसे संयुक्त राष्ट्र द्वारा मान्यता प्राप्त है।", // Hindi |
| mr: "डिजिटल साधनांचा वापर करून सार्वजनिक आरोग्य सेवांची गुणवत्ता आणि सुलभता सतत सुधारणे हे आमचे ध्येय आहे. कोरोनासेफ नेटवर्क ही एक मुक्त-स्त्रोत सार्वजनिक सुविधा आहे जी केरळ सरकारच्या पूर्ण मदतीने आणि समर्थनासह सरकारच्या प्रयत्नांना पाठिंबा देण्यासाठी मॉडेलवर काम करणारे नवीन-स्वयंसेवक आणि स्वयंसेवकांच्या एकाधिक-शिस्तबद्ध टीमद्वारे डिझाइन केलेले आहे.", // Marathi | ||
| kr: "ಕಾಳಜಿಡಿಜಿಟಲ್ ಉಪಕರಣಗಳನ್ನು ಬಳಸಿಕೊಂಡು ಸಾರ್ವಜನಿಕ ಆರೋಗ್ಯ ಸೇವೆಗಳ ಗುಣಮಟ್ಟ ಮತ್ತು ಪ್ರವೇಶವನ್ನು ನಿರಂತರವಾಗಿ ಸುಧಾರಿಸುವುದು ನಮ್ಮ ಗುರಿಯಾಗಿದೆ ಕೊರೊನಾಸೇಫ್ ನೆಟ್ವರ್ಕ್ ಎಂಬುದು ತೆರೆದ ಮೂಲ ಸಾರ್ವಜನಿಕ ಉಪಯುಕ್ತತೆಯಾಗಿದ್ದು, ನಾವೀನ್ಯಕಾರರು ಮತ್ತು ಸ್ವಯಂಸೇವಕರ ಬಹು-ಶಿಸ್ತಿನ ತಂಡದಿಂದ ವಿನ್ಯಾಸಗೊಳಿಸಲಾಗಿದೆ. ಕರೋನಾ ಸೇಫ್ ಕೇರ್ ವಿಶ್ವಸಂಸ್ಥೆಯಿಂದ ಗುರುತಿಸಲ್ಪಟ್ಟ ಡಿಜಿಟಲ್ ಸಾರ್ವಜನಿಕ ಸೇವೆಯಾಗಿದೆ." //kannada | ||
| }; | ||
|
|
||
| const loginPage = new LoginPage(); | ||
|
|
||
| beforeEach(() => { | ||
| cy.log("Logging in the user devdistrictadmin"); |
There was a problem hiding this comment.
Add
cy.awaitUrl("/", true);
Otherwise cypress won't be visiting the homepage.
Remove the log.
|
|
||
| class LoginPage { | ||
| submitButtonSelector = "#login"; | ||
| languageSelector = "#language-selector"; | ||
| sidebarSelector = "#sidebar"; |
There was a problem hiding this comment.
Hmm, neither language or sidebar selectors are present in Login.tsx; You have to add them. Have you tested this locally?
|
Just noticed that there are two PRs open; please make your changes in original PR #9238. Closing this. |
Jacobjeevan
added
duplicate
Proposed Changes
-Change 1 Added a Cypress test to verify that the login page's "Submit" button updates text dynamically when switching between different languages
Change 2 Created a test to log in with a non-English language (e.g., Hindi, Tamil, Kannada).
Verified that sidebar items display correctly in the chosen language.
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation