-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
feat: add unit tests for multiOrgDomains #41076
Conversation
WalkthroughA new test suite for the Changes
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
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
Documentation and Community
|
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.
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 sincemockCookieSetter
is guaranteed to exist from the property descriptor above.- mockCookieSetter?.mockClear?.(); + mockCookieSetter.mockClear();
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 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.
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?
Summary by CodeRabbit