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

Cypress Test for create a new facility in organization #9846

Merged
merged 7 commits into from
Jan 9, 2025

Conversation

nihal467
Copy link
Member

@nihal467 nihal467 commented Jan 8, 2025

Summary by CodeRabbit

Release Notes

  • New Features

    • Added facility creation test suite with comprehensive test cases.
    • Introduced new utility functions for generating test data.
    • Enhanced localization support with a new notification message.
  • Testing Improvements

    • Added data attributes to components for better test coverage.
    • Implemented robust facility creation and validation tests.
    • Created utility functions for generating random facility and phone number data.
  • Chores

    • Updated login and user management fixtures.
    • Improved test infrastructure with new page objects and utility methods.

@nihal467 nihal467 requested a review from a team as a code owner January 8, 2025 10:22
Copy link
Contributor

coderabbitai bot commented Jan 8, 2025

Walkthrough

This pull request introduces comprehensive Cypress end-to-end testing for facility management functionality. The changes include creating a new test suite in facility_creation.cy.ts that covers two primary scenarios: successfully creating a new facility and validating required field errors. Supporting changes include new utility functions for generating facility data, a page object for facility creation, and adding data attributes to various components to improve testability. Additionally, modifications to existing commands and localization enhance the overall testing framework.

Changes

File Change Summary
cypress/e2e/facility_spec/facility_creation.cy.ts New test suite for facility management with two test cases: facility creation and validation error checking
cypress/pageObject/auth/LoginPage.ts Removed verifyLoginResponse method
cypress/pageObject/facility/FacilityCreation.ts New page object class with methods for facility creation and verification
cypress/support/commands.ts Updated login command to loginByApi with role parameter; removed refreshApiLogin command; updated notification and error message verification methods
cypress/utils/commonUtils.ts Added generatePhoneNumber utility function
cypress/utils/facilityData.ts Added interface, constants, and functions for generating facility test data
public/locale/en.json Added localization entry for facility creation success message
src/components/Facility/CreateFacilityForm.tsx Added data-cy attributes for improved test accessibility
src/pages/Organization/OrganizationFacilities.tsx Added data-cy attributes for improved test accessibility
src/pages/Organization/components/AddFacilitySheet.tsx Added data-cy attribute to button for testing
src/pages/Organization/components/OrganizationLayout.tsx Added data-testid attribute to MenubarTrigger for testing
src/pages/UserDashboard.tsx Added data-cy attribute to organization list for testing
cypress/e2e/login_spec/loginpage.cy.ts Modified login process to use API-based login, removing local storage management
cypress/fixtures/users.json Added new user entry for "staff"
cypress/docs/best-practices.md Added new section outlining best practices for Cypress testing
cypress/docs/cypress.md Restructured documentation with new sections and updated titles
cypress/docs/file-structure.md New documentation file outlining the directory structure of a Cypress project
cypress/docs/patterns.md Added new section outlining testing patterns and best practices
cypress/e2e/patient_spec/patient_search.cy.ts Modified login process to use API-based login, removing page object model
cypress/support/index.ts Updated Cypress.Chainable interface by removing old login methods and updating loginByApi method signature

Possibly related issues

Possibly related PRs

Suggested Labels

needs review, tested

Suggested Reviewers

  • rithviknishad
  • khavinshankar

Poem

🐰 Hopping through code with glee,
Facilities dance, tests run free!
Cypress clicks, data takes flight,
Validation errors shine so bright,
A rabbit's testing jubilee! 🧪

Finishing Touches

  • 📝 Generate Docstrings

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Jan 8, 2025

Deploy Preview for care-ohc ready!

Name Link
🔨 Latest commit 2f01ef7
🔍 Latest deploy log https://app.netlify.com/sites/care-ohc/deploys/677f838a549cb50008b25574
😎 Deploy Preview https://deploy-preview-9846--care-ohc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

cloudflare-workers-and-pages bot commented Jan 8, 2025

Deploying care-fe with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2f01ef7
Status: ✅  Deploy successful!
Preview URL: https://d36218f8.care-fe.pages.dev
Branch Preview URL: https://facility-creation.care-fe.pages.dev

View logs

Copy link

cypress bot commented Jan 8, 2025

CARE    Run #4204

Run Properties:  status check passed Passed #4204  •  git commit 2f01ef7705: Cypress Test for create a new facility in organization
Project CARE
Branch Review facility-creation
Run status status check passed Passed #4204
Run duration 01m 36s
Commit git commit 2f01ef7705: Cypress Test for create a new facility in organization
Committer Mohammed Nihal
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 6
View all changes introduced in this branch ↗︎

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (4)
cypress/utils/commonUtils.ts (1)

8-8: Ensure consistent 10-digit phone numbers.

The slice operation might not always return exactly 9 digits if Math.random() generates a number with fewer decimal places.

Consider padding the number with zeros:

-  const remainingDigits = Math.random().toString().slice(2, 11);
+  const remainingDigits = Math.random().toString().slice(2, 11).padEnd(9, '0');
cypress/utils/facilityData.ts (1)

166-168: Simplify array shuffling using a dedicated utility function.

The current shuffling implementation using sort() with random comparison isn't the most reliable shuffling method.

Consider implementing Fisher-Yates shuffle:

-  const shuffledFeatures = [...FACILITY_FEATURES].sort(
-    () => Math.random() - 0.5,
-  );
+  const shuffledFeatures = [...FACILITY_FEATURES];
+  for (let i = shuffledFeatures.length - 1; i > 0; i--) {
+    const j = Math.floor(Math.random() * (i + 1));
+    [shuffledFeatures[i], shuffledFeatures[j]] = [shuffledFeatures[j], shuffledFeatures[i]];
+  }
cypress/e2e/facility_spec/facility_creation.cy.ts (1)

22-57: Add assertions for facility details in search results.

The test verifies the facility name but doesn't check other important details.

Add more comprehensive verification:

   // Search for the facility and verify in card
   facilityPage.searchFacility(testFacility.name);
   facilityPage.verifyFacilityNameInCard(testFacility.name);
+  facilityPage.verifyFacilityDetailsInCard({
+    type: testFacility.type,
+    address: testFacility.address,
+    features: testFacility.features
+  });
src/pages/Organization/OrganizationFacilities.tsx (1)

72-72: Review grid layout changes and add mobile breakpoint.

While adding test attributes improves testability, removing grid-cols-1 might affect the mobile layout. Consider keeping it for better mobile responsiveness.

-          className="grid gap-4 md:grid-cols-2 lg:grid-cols-3"
+          className="grid grid-cols-1 gap-4 md:grid-cols-2 lg:grid-cols-3"

Also applies to: 76-79

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0088123 and bb2aa77.

📒 Files selected for processing (12)
  • cypress/e2e/facility_spec/facility_creation.cy.ts (1 hunks)
  • cypress/pageObject/auth/LoginPage.ts (0 hunks)
  • cypress/pageObject/facility/FacilityCreation.ts (1 hunks)
  • cypress/support/commands.ts (1 hunks)
  • cypress/utils/commonUtils.ts (1 hunks)
  • cypress/utils/facilityData.ts (1 hunks)
  • public/locale/en.json (1 hunks)
  • src/components/Facility/CreateFacilityForm.tsx (12 hunks)
  • src/pages/Organization/OrganizationFacilities.tsx (1 hunks)
  • src/pages/Organization/components/AddFacilitySheet.tsx (1 hunks)
  • src/pages/Organization/components/OrganizationLayout.tsx (1 hunks)
  • src/pages/UserDashboard.tsx (1 hunks)
💤 Files with no reviewable changes (1)
  • cypress/pageObject/auth/LoginPage.ts
✅ Files skipped from review due to trivial changes (2)
  • src/pages/UserDashboard.tsx
  • src/pages/Organization/components/AddFacilitySheet.tsx
🧰 Additional context used
📓 Learnings (3)
📓 Common learnings
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
cypress/e2e/facility_spec/facility_creation.cy.ts (2)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
Learnt from: kihan2518B
PR: ohcnetwork/care_fe#8956
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:261-261
Timestamp: 2024-12-04T18:58:47.241Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, the test case titled "Create a new facility with no bed and doctor capacity" includes steps to select bed types and specializations before validation. This is done intentionally to verify that error messages are shown properly when no capacity is specified.
cypress/pageObject/facility/FacilityCreation.ts (1)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
🪛 GitHub Check: CodeQL
cypress/e2e/facility_spec/facility_creation.cy.ts

[failure] 10-10: Insecure randomness
This uses a cryptographically insecure random number generated at Math.random() in a security context.
This uses a cryptographically insecure random number generated at Math.random() in a security context.

cypress/pageObject/facility/FacilityCreation.ts

[failure] 66-66: Insecure randomness
This uses a cryptographically insecure random number generated at Math.random() in a security context.


[failure] 66-66: Insecure randomness
This uses a cryptographically insecure random number generated at Math.random() in a security context.

🪛 GitHub Actions: Cypress Tests
cypress/pageObject/facility/FacilityCreation.ts

[error] 9-9: Element '[data-cy="organization-list"]' not found in page. Test timed out after 10000ms waiting for the element.


[error] 70-70: Method 'verifyErrorMessage' is not defined. TypeError: cy.verifyErrorMessage is not a function

🔇 Additional comments (3)
src/pages/Organization/components/OrganizationLayout.tsx (1)

182-182: LGTM! Well-structured test attribute.

The data-testid attribute follows testing best practices with a consistent prefix and descriptive naming pattern.

Let's verify the uniqueness of the test IDs across menu items:

✅ Verification successful

✅ Test attribute implementation verified as unique and consistent

The dynamic generation of test IDs using item.title.toLowerCase() ensures uniqueness across menu items, and we can see it's already being successfully used in tests.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify uniqueness of data-testid attributes for menu items
# Expected: Each menu item (Organizations, Users, Patients, Facilities) should have a unique data-testid

# Search for all data-testid attributes in the codebase to check for potential conflicts
rg 'data-testid="org-nav-' --type ts --type tsx

Length of output: 77


Script:

#!/bin/bash
# Search for data-testid patterns in all ts/tsx files
fd -e ts -e tsx --type f --exec rg -A 1 -B 1 'data-testid.*org-nav-' {}

Length of output: 366

src/components/Facility/CreateFacilityForm.tsx (1)

Line range hint 180-449: LGTM! Test attributes are well-placed for facility creation form.

The data-cy attributes have been strategically added to all form elements, providing good test coverage for the facility creation workflow.

Let's verify that all critical form elements have test attributes:

✅ Verification successful

All form elements have proper test attributes

The verification confirms that all critical form elements in the facility creation form have appropriate data-cy attributes, providing comprehensive test coverage for the entire workflow. The review comment's assessment is accurate.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify test coverage of form elements
# Check for any form elements without data-cy attributes

rg -U 'FormField[\s\S]*?(?=FormField|$)' | rg -v 'data-cy'

Length of output: 311


Script:

#!/bin/bash
# Search for form elements without data-cy attributes
echo "=== Searching for Input components without data-cy ==="
ast-grep --pattern 'Input $$$' | rg -v 'data-cy'

echo -e "\n=== Searching for Select components without data-cy ==="
ast-grep --pattern 'Select $$$' | rg -v 'data-cy'

echo -e "\n=== Searching for Textarea components without data-cy ==="
ast-grep --pattern 'Textarea $$$' | rg -v 'data-cy'

echo -e "\n=== Searching for FormControl without data-cy in children ==="
ast-grep --pattern 'FormControl>
  $$$
</FormControl>' | rg -v 'data-cy'

Length of output: 739


Script:

#!/bin/bash
# Search for form elements without data-cy attributes in TSX files
echo "=== Searching for form elements without data-cy in CreateFacilityForm.tsx ==="
rg -t tsx '<(Input|Select|Textarea|Button|Checkbox)' src/components/Facility/CreateFacilityForm.tsx | rg -v 'data-cy'

echo -e "\n=== Checking other form components in the Facility directory ==="
fd CreateFacilityForm.tsx src/components/Facility -x rg -l '<FormControl>' {} | xargs rg '<(Input|Select|Textarea|Button|Checkbox)' | rg -v 'data-cy'

Length of output: 1087

public/locale/en.json (1)

926-926: LGTM! Success message added for facility creation.

The localization entry for facility creation success message has been added correctly.

cypress/pageObject/facility/FacilityCreation.ts Outdated Show resolved Hide resolved
cypress/pageObject/facility/FacilityCreation.ts Outdated Show resolved Hide resolved
cypress/e2e/facility_spec/facility_creation.cy.ts Outdated Show resolved Hide resolved
cypress/support/commands.ts Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🔭 Outside diff range comments (1)
cypress/fixtures/users.json (1)

Line range hint 2-4: Security: Update weak admin credentials

The admin credentials ("admin"/"admin") are extremely weak and appear to be default values. Even in test environments, it's recommended to use strong passwords.

Consider updating admin credentials to match the strength of other users:

   "admin": {
     "username": "admin",
-    "password": "admin"
+    "password": "Test@123"
   },
♻️ Duplicate comments (1)
cypress/pageObject/facility/FacilityCreation.ts (1)

80-99: ⚠️ Potential issue

Add retry logic and fix custom command usage.

  1. Add retry logic for facility search:
   verifyFacilityNameInCard(facilityName: string) {
-    cy.get('[data-cy="facility-cards"]').should("contain", facilityName);
+    cy.get('[data-cy="facility-cards"]', { timeout: 10000 })
+      .should("be.visible")
+      .and("contain", facilityName);
   }
  1. Based on the past review comments, ensure the custom command is defined in cypress/support/commands.ts:
// Add to cypress/support/commands.ts
Cypress.Commands.add('verifyErrorMessages', (messages: string[]) => {
  messages.forEach(message => {
    cy.get('[data-cy="error-message"]')
      .should('be.visible')
      .and('contain', message);
  });
});
🧹 Nitpick comments (1)
cypress/pageObject/facility/FacilityCreation.ts (1)

1-100: Consider adding error handling and logging capabilities.

To improve test debugging and maintenance:

  1. Add a logger utility:
// cypress/utils/logger.ts
export const log = {
  info: (message: string) => cy.log(`INFO: ${message}`),
  error: (message: string) => cy.log(`ERROR: ${message}`),
  debug: (message: string) => cy.log(`DEBUG: ${message}`)
};
  1. Wrap critical operations with try-catch blocks and logging:
import { log } from '../utils/logger';

async fillBasicDetails(name: string, description: string) {
  try {
    log.info(`Filling basic details: ${name}`);
    await this.enterFacilityName(name);
    await this.selectFacilityType();
    await this.enterDescription(description);
    log.info('Basic details filled successfully');
  } catch (error) {
    log.error(`Failed to fill basic details: ${error.message}`);
    throw error;
  }
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bb2aa77 and 6fba94c.

📒 Files selected for processing (7)
  • cypress/e2e/facility_spec/facility_creation.cy.ts (1 hunks)
  • cypress/e2e/login_spec/loginpage.cy.ts (1 hunks)
  • cypress/fixtures/users.json (1 hunks)
  • cypress/pageObject/facility/FacilityCreation.ts (1 hunks)
  • cypress/support/commands.ts (2 hunks)
  • cypress/utils/commonUtils.ts (1 hunks)
  • cypress/utils/facilityData.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • cypress/utils/commonUtils.ts
  • cypress/e2e/facility_spec/facility_creation.cy.ts
  • cypress/support/commands.ts
  • cypress/utils/facilityData.ts
🧰 Additional context used
📓 Learnings (1)
cypress/pageObject/facility/FacilityCreation.ts (2)
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
Learnt from: nihal467
PR: ohcnetwork/care_fe#9846
File: cypress/pageObject/facility/FacilityCreation.ts:18-23
Timestamp: 2025-01-08T10:57:45.377Z
Learning: In the facility creation form, the facility type dropdown is hardcoded and its options are always available, making additional error handling unnecessary.
🔇 Additional comments (2)
cypress/e2e/login_spec/loginpage.cy.ts (1)

17-20: Verify the removal of login response verification

The login response verification was removed from this test. This reduces the test's reliability as it only checks the URL redirect without verifying the actual login response.

Let's check if other tests cover the login response verification:

cypress/fixtures/users.json (1)

Line range hint 1-14: Verify test data isolation

The test credentials appear to be used directly in the test environment. This could potentially interfere with real user data or cause test flakiness.

Let's check if these credentials are properly isolated to the test environment:

cypress/pageObject/facility/FacilityCreation.ts Outdated Show resolved Hide resolved
cypress/pageObject/facility/FacilityCreation.ts Outdated Show resolved Hide resolved
cypress/pageObject/facility/FacilityCreation.ts Outdated Show resolved Hide resolved
cypress/e2e/login_spec/loginpage.cy.ts Outdated Show resolved Hide resolved
cypress/fixtures/users.json Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

♻️ Duplicate comments (1)
cypress/support/commands.ts (1)

84-88: ⚠️ Potential issue

Notification system migration remains incomplete.

While verifyNotification has been updated to use Sonner toast selectors, closeNotification still uses the old PNotify selector.

Apply this diff to maintain consistency:

-  cy.get(".pnotify")
+  cy.get("li[data-sonner-toast]")
🧹 Nitpick comments (16)
cypress/support/index.ts (2)

7-7: Consider adding type safety for roles.

The role parameter is currently typed as a string, which could lead to runtime errors if an invalid role is provided.

Consider using a union type to restrict allowed values:

type UserRole = 'staff' | 'admin' | 'patient'; // add other valid roles
loginByApi(role: UserRole): Chainable<Subject>;

7-7: Document the available roles.

Add JSDoc comments to document the available roles and their purposes to improve developer experience.

/**
 * Logs in a user with the specified role using API authentication.
 * @param role The user role to authenticate as. Available roles:
 *   - 'staff': Regular staff member with basic permissions
 *   - 'admin': Administrator with full access
 *   - Add other roles and their descriptions
 * @returns Chainable for continued commands
 * @example
 * cy.loginByApi('staff')
 */
loginByApi(role: string): Chainable<Subject>;
cypress/docs/cypress.md (3)

1-4: Enhance the overview section with more context.

While the overview provides a basic introduction, it would be more helpful to include:

  • The purpose of Cypress testing in our project
  • Types of tests covered (e.g., E2E, integration)
  • Prerequisites for writing tests

18-23: Add specific examples for facility creation tests.

Consider enhancing the Getting Started section with:

  • A link to the facility creation test as a reference implementation
  • Code snippets demonstrating the use of Page Object Model
  • Examples of commonly used commands and patterns

24-25: Expand the support section with specific channels.

Consider adding:

  • GitHub issue templates for test-related issues
  • Contact information for the testing team
  • Guidelines for contributing to test documentation
cypress/docs/patterns.md (2)

3-11: Enhance documentation with more context and explanations.

While the example clearly shows the preferred pattern, it would be more helpful to:

  1. Explain why verifyAndClickElement is preferred over the chain approach
  2. Document what verifyAndClickElement does internally
  3. Include information about timeout and retry behavior
 ## Element Interaction
 ### Preferred Command Usage
 ```typescript
 // Correct
 cy.verifyAndClickElement('[data-cy="element"]', "Button Text");
 
 // Avoid
 cy.get('[data-cy="element"]').should('be.visible').click();

+### Why use verifyAndClickElement?
+- Handles both element verification and click in a single command
+- Includes built-in retry-ability and proper timeout handling
+- Ensures consistent behavior across tests
+
+### Internal Behavior
+The command:
+1. Waits for element to exist in DOM
+2. Verifies visibility
+3. Checks if text matches (if provided)
+4. Ensures element is clickable
+5. Performs the click operation


---

`34-50`: **Enhance test structure example with more realistic scenarios.**

The current example could be improved with:
1. Complete setup in `beforeEach`
2. Error handling examples
3. Proper assertions
4. Documentation for the PageObject pattern


```diff
 ## Test Structure
 ```typescript
+/**
+ * Page Object for facility management
+ */
+class FacilityPage {
+  private readonly selectors = {
+    createButton: '[data-testid="create-facility"]',
+    nameInput: '[data-testid="facility-name"]'
+  };
+
+  createFacility(data: FacilityData): void {
+    cy.verifyAndClickElement(this.selectors.createButton);
+    // Implementation...
+  }
+}
+
 describe("Feature Name", () => {
-  const page = new PageObject();
+  const page = new FacilityPage();
   const facilityType = "Primary Health Centre";
   const testData = generateTestData();

   beforeEach(() => {
-    // Setup
+    cy.login();
+    cy.intercept('GET', '/api/facilities').as('getFacilities');
+    cy.visit('/facilities');
+    cy.wait('@getFacilities');
   });

   it("should perform action", () => {
     page.navigateToOrganization("Kerala");
     page.navigateToFacilitiesList();
+    
+    // Create new facility
+    page.createFacility(testData);
+    
+    // Verify creation
+    cy.get('[data-testid="facility-name"]')
+      .should('contain', testData.name);
   });
+  
+  it("should handle errors gracefully", () => {
+    // Error handling example
+    cy.intercept('POST', '/api/facilities', {
+      statusCode: 400,
+      body: { error: 'Invalid data' }
+    }).as('createError');
+    
+    page.createFacility(testData);
+    
+    // Verify error handling
+    cy.get('[data-testid="error-message"]')
+      .should('be.visible')
+      .and('contain', 'Invalid data');
+  });
 });
cypress/support/commands.ts (1)

211-214: Consider adding data attributes for error messages.

The current implementation searches the entire body for error messages, which could match unintended text. Consider:

  1. Adding data attributes (e.g., data-cy="error-message") to error message elements
  2. Using these attributes for more precise targeting
-  cy.get("body").within(() => {
-    errorMessages.forEach((message) => {
-      cy.contains(message).scrollIntoView().should("be.visible");
-    });
+  errorMessages.forEach((message) => {
+    cy.get('[data-cy="error-message"]')
+      .contains(message)
+      .scrollIntoView()
+      .should("be.visible");
+  });
cypress/docs/best-practices.md (3)

4-6: Add cleanup examples for test independence.

Consider adding examples of cleanup patterns to help developers implement proper test isolation:

 ## Test Independence
 - Each test should be independent and isolated
 - Clean up test data after tests
 - Don't rely on the state from other tests
+
+Example cleanup pattern:
+```typescript
+beforeEach(() => {
+  // Setup test data
+});
+
+afterEach(() => {
+  // Clean up created data
+  cy.task('cleanupTestData');
+});
+```

8-11: Add examples for API testing patterns.

Consider adding examples to illustrate the recommended API testing patterns:

 ## API Testing
 - Use cy.intercept() for API verification
 - Use waitUntil() for API completion
 - Avoid cy.wait() except for API responses
+
+Example:
+```typescript
+cy.intercept('POST', '/api/facilities').as('createFacility');
+// Perform action that triggers API call
+cy.wait('@createFacility').then((interception) => {
+  expect(interception.response.statusCode).to.eq(201);
+});
+```

20-20: Add "the" before "AAA pattern".

Fix the grammar in the Code Organization section.

-Follow AAA pattern (Arrange, Act, Assert)
+Follow the AAA pattern (Arrange, Act, Assert)
🧰 Tools
🪛 LanguageTool

[uncategorized] ~20-~20: You might be missing the article “the” here.
Context: ...Keep tests focused and concise - Follow AAA pattern (Arrange, Act, Assert) - Use me...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

cypress/pageObject/facility/FacilityCreation.ts (3)

58-60: Use consistent selector pattern.

The selector for features uses an id selector while other methods use data-cy attributes.

   selectFeatures(features: string[]) {
-    cy.clickAndMultiSelectOption("#facility-features", features);
+    cy.clickAndMultiSelectOption('[data-cy="facility-features"]', features);
   }

86-93: Make validation error messages configurable.

Consider making the error messages configurable to support internationalization and avoid hardcoding.

   verifyValidationErrors() {
+    const errorMessages = {
+      name: "Name is required",
+      facilityType: "Facility type is required",
+      address: "Address is required",
+      phone: "Phone number must start with +91 followed by 10 digits"
+    };
     cy.verifyErrorMessages([
-      "Name is required",
-      "Facility type is required",
-      "Address is required",
-      "Phone number must start with +91 followed by 10 digits",
+      errorMessages.name,
+      errorMessages.facilityType,
+      errorMessages.address,
+      errorMessages.phone
     ]);
   }

99-101: Enhance facility card verification.

The current implementation might be fragile if multiple facilities contain similar names.

   verifyFacilityNameInCard(facilityName: string) {
-    cy.get('[data-cy="facility-cards"]').should("contain", facilityName);
+    cy.get('[data-cy="facility-cards"]')
+      .find(`[data-cy="facility-name"][title="${facilityName}"]`)
+      .should("be.visible");
   }
cypress/docs/file-structure.md (2)

4-18: Add language specification to code fence.

Specify the language for the code fence to enable proper syntax highlighting.

-```
+```bash
 cypress/
 ├── docs/
 │   ├── README.md
🧰 Tools
🪛 Markdownlint (0.37.0)

4-4: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)


37-39: Enhance storage management section with examples.

The storage management section would benefit from practical examples.

 ## Storage Management
 - Use cy.saveLocalStorage() and cy.restoreLocalStorage()
 - Manage test data cleanup 
+
+Example:
+```typescript
+beforeEach(() => {
+  cy.restoreLocalStorage();
+});
+
+afterEach(() => {
+  cy.saveLocalStorage();
+});
+
+after(() => {
+  // Cleanup test data
+  cy.task('cleanDatabase');
+});
+```
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6fba94c and 2f01ef7.

📒 Files selected for processing (12)
  • .cursorrules (1 hunks)
  • cypress/docs/best-practices.md (1 hunks)
  • cypress/docs/cypress.md (1 hunks)
  • cypress/docs/file-structure.md (1 hunks)
  • cypress/docs/patterns.md (1 hunks)
  • cypress/e2e/facility_spec/facility_creation.cy.ts (1 hunks)
  • cypress/e2e/login_spec/loginpage.cy.ts (1 hunks)
  • cypress/e2e/patient_spec/patient_search.cy.ts (1 hunks)
  • cypress/pageObject/facility/FacilityCreation.ts (1 hunks)
  • cypress/support/commands.ts (3 hunks)
  • cypress/support/index.ts (1 hunks)
  • public/locale/en.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .cursorrules
🚧 Files skipped from review as they are similar to previous changes (3)
  • public/locale/en.json
  • cypress/e2e/login_spec/loginpage.cy.ts
  • cypress/e2e/facility_spec/facility_creation.cy.ts
🧰 Additional context used
📓 Learnings (2)
cypress/e2e/patient_spec/patient_search.cy.ts (1)
Learnt from: nihal467
PR: ohcnetwork/care_fe#9285
File: cypress/e2e/patient_spec/PatientDoctorNotes.cy.ts:17-17
Timestamp: 2024-12-04T07:06:49.613Z
Learning: In `PatientDoctorNotes.cy.ts`, the login methods `loginManuallyAsNurse()` and `loginByRole("nurse")` serve different purposes and approaches. Refactoring `loginManuallyAsNurse()` to use `loginByRole()` is not necessary in this context.
cypress/pageObject/facility/FacilityCreation.ts (3)
Learnt from: nihal467
PR: ohcnetwork/care_fe#9846
File: cypress/pageObject/facility/FacilityCreation.ts:14-47
Timestamp: 2025-01-09T05:37:17.121Z
Learning: In the care_fe project's Cypress tests, facility test data is managed through the facilityData.ts utility which provides pre-validated data through the FacilityTestData interface, making additional validation in the FacilityCreation page object unnecessary.
Learnt from: Jacobjeevan
PR: ohcnetwork/care_fe#9145
File: cypress/e2e/facility_spec/FacilityCreation.cy.ts:177-180
Timestamp: 2024-11-18T10:44:30.303Z
Learning: In `cypress/e2e/facility_spec/FacilityCreation.cy.ts`, when testing bed and staff capacity individually for additional error verification, we prefer to use separate method calls to handle bed and staff capacity, rather than using `facilityPage.createNewFacility(testFacilityData)` which also handles bed management.
Learnt from: nihal467
PR: ohcnetwork/care_fe#9846
File: cypress/pageObject/facility/FacilityCreation.ts:18-23
Timestamp: 2025-01-08T10:57:45.377Z
Learning: In the facility creation form, the facility type dropdown is hardcoded and its options are always available, making additional error handling unnecessary.
🪛 LanguageTool
cypress/docs/best-practices.md

[uncategorized] ~20-~20: You might be missing the article “the” here.
Context: ...Keep tests focused and concise - Follow AAA pattern (Arrange, Act, Assert) - Use me...

(AI_EN_LECTOR_MISSING_DETERMINER_THE)

🪛 Markdownlint (0.37.0)
cypress/docs/file-structure.md

4-4: null
Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (5)
cypress/support/index.ts (1)

7-7: Verify updates to existing test files.

The loginByApi method signature has changed from accepting username/password to accepting a role parameter. This change requires updates to all existing test files using this method.

Let's verify the impact:

cypress/docs/cypress.md (2)

12-16: LGTM! These principles directly support the facility creation tests.

The principles are well-defined and particularly relevant for the facility creation tests in this PR:

  • Using data-cy attributes will make the facility form tests more reliable
  • Page Object Model pattern will help organize the facility creation page logic
  • TypeScript usage will ensure type safety for facility data

6-9: Verify the existence of referenced documentation files.

The quick links section references three markdown files. Let's ensure they exist in the repository.

✅ Verification successful

All referenced documentation files exist

The quick links are correctly pointing to existing documentation files in the repository.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the referenced documentation files exist
for file in "file-structure.md" "patterns.md" "best-practices.md"; do
  echo "Checking for cypress/docs/$file..."
  if [ ! -f "cypress/docs/$file" ]; then
    echo "Warning: $file not found"
  fi
done

Length of output: 638

cypress/e2e/patient_spec/patient_search.cy.ts (1)

12-13: LGTM! Login implementation aligns with established patterns.

The shift to using loginByApi is consistent with our learnings about different login approaches serving different purposes.

cypress/pageObject/facility/FacilityCreation.ts (1)

77-79: Verify custom command type definition.

Ensure that the clickSubmitButton command is properly typed in cypress/support/commands.ts.

✅ Verification successful

Command is properly typed

The clickSubmitButton command is correctly typed in cypress/support/index.ts with proper parameter and return types, and its implementation in cypress/support/commands.ts matches the type definition.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if clickSubmitButton command is defined
rg -A 5 "Cypress.Commands.add\('clickSubmitButton'" cypress/support/

Length of output: 76


Script:

#!/bin/bash
# Search for any mention of clickSubmitButton in cypress/support
rg "clickSubmitButton" cypress/support/

# Find type definition files
fd "\.d\.ts$" cypress/

Length of output: 254

cypress/docs/patterns.md Show resolved Hide resolved
cypress/docs/patterns.md Show resolved Hide resolved
cypress/support/commands.ts Show resolved Hide resolved
@rithviknishad rithviknishad merged commit 0592e63 into develop Jan 9, 2025
31 checks passed
@rithviknishad rithviknishad deleted the facility-creation branch January 9, 2025 09:09
Copy link

github-actions bot commented Jan 9, 2025

@nihal467 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants