test(organizations): rewrite domainJoin E2E for Node D5 always-create flow#4182
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a local Playwright config and COOKIE_PREFIX export; introduces API connectivity helpers and conditional owner-test skips; updates member/owner domain-join E2E expectations to always-create, adds a suggestedJoin CTA test that verifies join request creation, and guards permission tests when orgId is missing. ChangesDomain-join E2E test updates for always-create behavior
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4182 +/- ##
=======================================
Coverage 99.55% 99.55%
=======================================
Files 31 31
Lines 1136 1136
Branches 328 328
=======================================
Hits 1131 1131
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/organizations/tests/organizations.domainJoin.e2e.tests.js`:
- Around line 53-63: The test currently catches all errors from signupViaAPI and
always calls test.skip, which hides real failures; change the catch to only skip
when the error is a connectivity/network issue (e.g., check err.code for
'ECONNREFUSED'|'ENOTFOUND' or err.message includes 'connect' or, for HTTP
clients, err.isAxiosError && !err.response) and rethrow the error for all other
cases so real regressions fail the test; update the catch surrounding the
signupViaAPI call to perform this conditional check and call test.skip(true,
`API connection failed during signup: ${err.message}`) only for connectivity
errors, otherwise throw err.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3e193524-3c1e-485e-b569-f0693829cb92
📒 Files selected for processing (1)
src/modules/organizations/tests/organizations.domainJoin.e2e.tests.js
There was a problem hiding this comment.
Pull request overview
Updates the Organizations domain-join Playwright E2E suite to match the Node “D5 always-create workspace” behavior, where domain-matched signups no longer create a pending join by default and instead surface a suggestedJoin banner that can trigger a join request.
Changes:
- Update signup/signin assertions to reflect always-created workspaces landing on
/tasks(no/organization-requiredflow). - Add an E2E covering the
suggestedJoinbanner CTA (“Request access”) and backend join-request creation. - Tighten the account-page chevron assertion to apply only to the domain org list item.
| async function isApiAvailable(request) { | ||
| try { | ||
| const res = await request.get(`${API_ORIGIN}/api`); | ||
| return res.ok(); |
There was a problem hiding this comment.
Addressed in latest commit. Skip now scoped to connectivity errors via CONNECTIVITY_ERROR_RE pattern; isApiAvailable uses API_URL (configured base); localStorage key derived from new COOKIE_PREFIX export sourced from config.cookie.prefix; errorMessage() helper safely extracts string from unknown thrown values.
| lastName: 'Test', | ||
| }); | ||
| } catch (err) { | ||
| test.skip(true, `API connection failed during signup: ${err.message}`); |
There was a problem hiding this comment.
Addressed in latest commit. Skip now scoped to connectivity errors via CONNECTIVITY_ERROR_RE pattern; isApiAvailable uses API_URL (configured base); localStorage key derived from new COOKIE_PREFIX export sourced from config.cookie.prefix; errorMessage() helper safely extracts string from unknown thrown values.
| // Replicates what signup stores: key = `${config.cookie.prefix}SuggestedJoin` | ||
| // = 'devkitSuggestedJoin' in dev. | ||
| await page.context().addInitScript( | ||
| ([id, name]) => { | ||
| window.localStorage.setItem('devkitSuggestedJoin', JSON.stringify({ orgId: id, orgName: name })); | ||
| }, |
There was a problem hiding this comment.
Addressed in latest commit. Skip now scoped to connectivity errors via CONNECTIVITY_ERROR_RE pattern; isApiAvailable uses API_URL (configured base); localStorage key derived from new COOKIE_PREFIX export sourced from config.cookie.prefix; errorMessage() helper safely extracts string from unknown thrown values.
|
Removed stale repo var |
… flow Node #3680 dropped pendingJoin limbo — every signup now gets its own active workspace immediately, with a suggestedJoin hint when the domain matches. Rewrite the E2E suite to assert the new flow: member lands in organizationWelcome (not pending-join), suggestedJoin banner is pre-seeded via addInitScript and the Request access CTA creates a join request, access-control assertions updated for the member's own workspace (owner chevron) vs the joined domain org (no chevron).
PierreBrisorgueil
force-pushed
the
fix/domainjoin-e2e-always-create
branch
from
3d0f942 to
0ff51d1
Compare
CI runs with NODE_ENV=test which uses config.sign.route='/' (per src/config/defaults/test.config.js). Local dev uses '/tasks'. Subagent's hardcoded `/tasks` assertions failed in CI even though the flow worked correctly. Fix: assert we've LEFT the signup wizard / organization-required pages, not that we're on any specific destination URL. Matches both env routes.
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
…in E2E
- isApiAvailable: use API_URL (configured base path) instead of hardcoded
'${API_ORIGIN}/api' — projects overriding config.api.base no longer
false-negative as 'backend down'. (Copilot line 22)
- Setup test catch: skip ONLY on connectivity errors (ECONNREFUSED/
ENOTFOUND/ETIMEDOUT/EAI_AGAIN/'fetch failed'/'socket hang up'/
'network error'). Real backend regressions (4xx/5xx, schema drift)
now surface as failures instead of silent skips. (CodeRabbit line 63)
- errorMessage() helper: safely extract a string from unknown thrown
values. Prevents `err.message` itself throwing on null/undefined.
(Copilot line 61)
- localStorage key: derived from new COOKIE_PREFIX export on
lib/helpers/e2e/config.js (sourced from `config.cookie.prefix`).
Projects overriding the prefix no longer silently break this test.
(Copilot line 174)
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/organizations/tests/organizations.domainJoin.e2e.tests.js`:
- Around line 201-206: Extract the inline arrow passed to
page.context().addInitScript into a named function (e.g.,
setOrgLocalStorageInitScript) and replace the inline callback with that name;
add a JSDoc header above the new function with a one-line description, `@param`
{string} key, `@param` {string} id, `@param` {string} name describing each arg, and
an `@returns` {void} (or `@returns` {Promise<void>} if you make it async) to satisfy
the project guideline; ensure the call remains
page.context().addInitScript(setOrgLocalStorageInitScript, [storageKey, orgId,
`DomainOrg${timestamp}`]).
- Around line 39-45: The isApiAvailable function currently treats any non-2xx
HTTP status as "unavailable" by returning res.ok(); change it to perform a
transport-level reachability check: call request.get(API_URL) inside the try and
return true as long as a response is received (regardless of status), and only
return false when the request throws (network/connection errors). Update the
function around the request.get(API_URL) invocation so it uses the
absence/presence of an exception to decide availability instead of res.ok();
keep API_URL and the existing request object usage.
- Around line 136-137: Extract the inline URL predicate into a named helper
function (e.g., isPostSignupRoute) and replace the duplicated inline predicate
calls in organizations.domainJoin.e2e.tests.js (the four page.waitForURL usages)
with that function; ensure the helper has a JSDoc block per .js coding
guidelines describing parameters and return value, and update the predicate
logic to explicitly exclude '/signup', '/organization-required', and '/signin'
so the wait guards against partial auth failures and future changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cd7a3a54-9b4a-4d1b-92a8-5e89da1ede37
📒 Files selected for processing (3)
playwright.local.config.jssrc/lib/helpers/e2e/config.jssrc/modules/organizations/tests/organizations.domainJoin.e2e.tests.js
PierreBrisorgueil
dismissed
coderabbitai[bot]’s stale review
CodeRabbit confirmed in inline reply at 12:25 UTC that the latest commit fully addresses the actionable concern (connectivity-error-scoped skip via CONNECTIVITY_ERROR_RE + safer errorMessage helper). The top-level CHANGES_REQUESTED state does not auto-upgrade after fix; dismissing stale review per feedback_coderabbit_stale_review.
CodeRabbit follow-up review on previous fix commit: - isApiAvailable: return reachable=true on ANY HTTP response (was: res.ok() filtering 2xx). 4xx/5xx means the backend IS running and answered — the test should exercise the real code path, not skip. Only network/connect errors indicate the backend is genuinely unreachable. - isPostSignupRoute: extract the 4-times-duplicated inline waitForURL predicate into a named helper with JSDoc; also exclude '/signin' so the predicate guards against partial-auth races landing the user back on the auth surfaces. - seedSuggestedJoinLocalStorage: extract the inline addInitScript arrow into a named function with JSDoc. Also: remove playwright.local.config.js — local debris that leaked in on the env-agnostic-URL commit (dev override for port 5174). Add to .gitignore so it can't sneak back in.
PierreBrisorgueil
dismissed
coderabbitai[bot]’s stale review
Addressed in latest commit (3 fixes + local-debris cleanup): isApiAvailable now returns true on any HTTP response (transport-level reachability only); inline waitForURL predicate extracted into named isPostSignupRoute helper with JSDoc + added /signin exclusion; inline addInitScript arrow extracted into seedSuggestedJoinLocalStorage with JSDoc; dropped accidentally-committed playwright.local.config.js + added to .gitignore. Dismissing stale CHANGES_REQUESTED per feedback_coderabbit_stale_review.
Summary
organizations.domainJoin.e2e.tests.jsfor Node PR fix(seo): skip runtime JSON-LD when seo-inject handles schema #3680 (D5 always-create):pendingJoinis never returned from signup; every user always lands in their own auto-created workspace with asuggestedJoinbanner hintpage.context().addInitScript()to injectdevkitSuggestedJoininto localStorage before every page load, soinitFromStorage()restores it reliably on the/signinmountDomainOrglist item only — the member now legitimately owns their own auto-created workspace (chevron present there), but has rolememberon the joined domain org (no chevron)Test plan
npm run lint— no issuessrc/modules/organizations/tests/organizations.domainJoin.e2e.tests.jsmodifiedSummary by CodeRabbit