Skip to content

feat: add unit tests for multiOrgDomains #41076

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

Merged
merged 1 commit into from
Jul 7, 2025

Conversation

jacquesikot
Copy link
Contributor

@jacquesikot jacquesikot commented Jul 3, 2025

Description

Add unit tests for multiOrgDomains

Automation

/ok-to-test tags="@tag.Sanity"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/16044975912
Commit: 9f865d9
Cypress dashboard.
Tags: @tag.Sanity
Spec:


Thu, 03 Jul 2025 08:48:25 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • Tests
    • Added comprehensive unit tests for multi-organization domain utilities, including domain validation, recent domain tracking, retrieval, and clearing of stored domains.
    • Ensured robust handling of edge cases, malformed data, and cookie management behaviors.

@jacquesikot jacquesikot self-assigned this Jul 3, 2025
@jacquesikot jacquesikot added the ok-to-test Required label for CI label Jul 3, 2025
Copy link
Contributor

coderabbitai bot commented Jul 3, 2025

Walkthrough

A new test suite for the multiOrgDomains utility functions has been introduced. This suite comprehensively tests domain validation, tracking, retrieval, and clearing logic, including cookie handling and edge cases. Jest mocks are used to simulate browser environment behaviors and verify robust error handling.

Changes

File(s) Change Summary
app/client/src/utils/multiOrgDomains.test.ts Added comprehensive unit tests for domain validation, tracking, retrieval, clearing, and cookie logic

Sequence Diagram(s)

sequenceDiagram
    participant TestSuite
    participant multiOrgDomains
    participant Cookie
    participant Date
    participant Location

    TestSuite->>multiOrgDomains: isValidAppsmithDomain(domain)
    TestSuite->>multiOrgDomains: trackCurrentDomain()
    multiOrgDomains->>Cookie: set cookie with current domain
    multiOrgDomains->>Date: check domain age
    TestSuite->>multiOrgDomains: getRecentDomains()
    multiOrgDomains->>Cookie: read cookie
    multiOrgDomains->>Location: compare with current domain
    TestSuite->>multiOrgDomains: clearRecentDomains()
    multiOrgDomains->>Cookie: clear cookie
Loading

Possibly related PRs

Suggested labels

Enhancement

Suggested reviewers

  • ankitakinger
  • trishaanand

Poem

Domains are tracked, their cookies stored,
Edge cases tested, errors ignored.
With Jest in hand, the suite runs deep,
Ensuring logic strong and neat.
Now multi-orgs can rest assured,
Their recent hosts are well secured!
🧪✨

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ 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.
    • Explain this complex logic.
    • 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 explain this code block.
    • @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 explain its main purpose.
    • @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.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @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.

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.

@github-actions github-actions bot added the Enhancement New feature or request label Jul 3, 2025
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: 0

🧹 Nitpick comments (1)
app/client/src/utils/multiOrgDomains.test.ts (1)

49-49: Simplify optional chaining for mock cleanup.

The optional chaining mockCookieSetter?.mockClear?.() is unnecessary since mockCookieSetter is guaranteed to exist from the property descriptor above.

-    mockCookieSetter?.mockClear?.();
+    mockCookieSetter.mockClear();
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b4d5685 and 9f865d9.

📒 Files selected for processing (1)
  • app/client/src/utils/multiOrgDomains.test.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: appsmithorg/appsmith#0
File: .cursor/rules/index.mdc:0-0
Timestamp: 2025-06-23T12:22:10.042Z
Learning: Pull request titles in the Appsmith codebase should follow the Conventional Commits specification, using the format 'type(scope): description' with valid types such as feat, fix, docs, style, refactor, perf, test, build, ci, chore, and revert.
Learnt from: Aishwarya-U-R
PR: appsmithorg/appsmith#29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: Aishwarya-U-R
PR: appsmithorg/appsmith#29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: sneha122
PR: appsmithorg/appsmith#29377
File: app/client/src/pages/Editor/IntegrationEditor/CreateNewDatasourceTab.tsx:316-329
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user clarified that the feature being introduced is experimental and that automated tests will be added once the decision to keep the feature is made. This context is important for future reviews as it explains the absence of tests for certain features under development.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/utils/autocomplete/__tests__/TernServer.test.ts:254-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Integration tests cover the `hasCreateDSActionPermissionInApp` function, as clarified by the user.
Learnt from: ankitakinger
PR: appsmithorg/appsmith#29573
File: app/client/src/utils/autocomplete/__tests__/TernServer.test.ts:254-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: Integration tests cover the `hasCreateDSActionPermissionInApp` function, as clarified by the user.
Learnt from: brayn003
PR: appsmithorg/appsmith#40462
File: app/client/src/instrumentation/index.ts:0-0
Timestamp: 2025-04-29T09:12:14.552Z
Learning: Only comment on files that are directly related to the PR's objectives, even if other files appear in the diff. For PR #40462, the focus is on the import override feature for artifacts, not on instrumentation or telemetry files.
Learnt from: abhishek-bandameedi
PR: appsmithorg/appsmith#35133
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0
Timestamp: 2024-07-24T08:29:41.208Z
Learning: In Cypress tests for the Appsmith project, ensure the use of locator variables for selectors and include multiple assertions for comprehensive testing.
Learnt from: abhishek-bandameedi
PR: appsmithorg/appsmith#35133
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In Cypress tests for the Appsmith project, ensure the use of locator variables for selectors and include multiple assertions for comprehensive testing.
app/client/src/utils/multiOrgDomains.test.ts (5)
Learnt from: abhishek-bandameedi
PR: appsmithorg/appsmith#35133
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In Cypress tests for the Appsmith project, ensure the use of locator variables for selectors and include multiple assertions for comprehensive testing.
Learnt from: abhishek-bandameedi
PR: appsmithorg/appsmith#35133
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0
Timestamp: 2024-07-24T08:29:41.208Z
Learning: In Cypress tests for the Appsmith project, ensure the use of locator variables for selectors and include multiple assertions for comprehensive testing.
Learnt from: rahulbarwal
PR: appsmithorg/appsmith#37289
File: app/client/src/widgets/JSONFormWidget/fields/SelectField.test.tsx:137-138
Timestamp: 2024-11-08T09:17:10.831Z
Learning: In TypeScript test files like `SelectField.test.tsx`, it's acceptable to use the `delete` operator to remove mock objects such as `ResizeObserver` when their eventual removal is required.
Learnt from: sagar-qa007
PR: appsmithorg/appsmith#34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Learnt from: sagar-qa007
PR: appsmithorg/appsmith#34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-07-16T06:44:55.118Z
Learning: Avoid using sleep functions like `agHelper.Sleep`, `this.Sleep`, and other related sleep functions in Cypress tests within the `app/client/cypress` directory to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable.
Learnt from: brayn003
PR: appsmithorg/appsmith#38635
File: app/client/cypress/e2e/Regression/ClientSide/Git/GitSync/MergeViaRemote_spec.ts:81-82
Timestamp: 2025-01-13T15:14:42.085Z
Learning: In Cypress test files, unsafe optional chaining is acceptable when the intention is for the test to fail if the expected data structure is not present, as it helps validate the API response structure.
🧬 Code Graph Analysis (1)
app/client/src/utils/multiOrgDomains.test.ts (1)
app/client/src/utils/multiOrgDomains.ts (4)
  • isValidAppsmithDomain (105-105)
  • trackCurrentDomain (63-87)
  • getRecentDomains (89-99)
  • clearRecentDomains (101-103)
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: client-lint / client-lint
  • GitHub Check: client-prettier / prettier-check
  • GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
  • GitHub Check: client-unit-tests / client-unit-tests
  • GitHub Check: client-build / client-build
🔇 Additional comments (7)
app/client/src/utils/multiOrgDomains.test.ts (7)

1-55: Excellent mocking setup with comprehensive test environment simulation.

The mock configuration properly simulates browser behavior for Date.now, location.hostname, and document.cookie with sophisticated cookie expiry handling. The beforeEach/afterEach cleanup ensures test isolation.


56-81: Comprehensive domain validation tests with good edge case coverage.

Tests properly validate the isValidAppsmithDomain function against valid domains, invalid domains, excluded subdomains, and edge cases including empty strings and nested subdomains.


83-220: Thorough testing of domain tracking functionality.

Tests cover all critical aspects of trackCurrentDomain: multi-org domain validation, duplicate removal, ordering (newest first), MAX_DOMAINS limit (10), 30-day expiry filtering, and graceful error handling for malformed cookie data.


222-286: Complete validation of domain retrieval logic.

Tests verify getRecentDomains correctly filters current domain, applies 30-day expiry, validates domains, and handles missing/invalid cookie data gracefully.


288-304: Proper verification of cookie clearing functionality.

Tests confirm clearRecentDomains sets the correct expiry cookie string and the mock properly simulates cookie removal.


306-325: Good coverage of edge cases and error scenarios.

Tests handle malformed cookies, empty domains, and special characters appropriately, ensuring robustness.


327-355: Validates cookie management attributes and encoding.

Tests verify cookie expiry, domain, path, SameSite attributes, and proper JSON encoding/decoding of cookie values.

@jacquesikot jacquesikot added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Jul 3, 2025
@jacquesikot jacquesikot merged commit 2af8ef3 into release Jul 7, 2025
55 of 56 checks passed
@jacquesikot jacquesikot deleted the feat/tests-for-multi-org-domain-org-dropdown branch July 7, 2025 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request ok-to-test Required label for CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants