test: add API coverage for users.register settings flows#40754
test: add API coverage for users.register settings flows#40754jessicaschelly wants to merge 3 commits into
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughThis PR expands e2e tests for ChangesUsers registration e2e tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 @@
## develop #40754 +/- ##
===========================================
- Coverage 69.76% 69.74% -0.03%
===========================================
Files 3327 3327
Lines 123134 123134
Branches 21987 21964 -23
===========================================
- Hits 85904 85875 -29
- Misses 33869 33907 +38
+ Partials 3361 3352 -9
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/meteor/tests/end-to-end/api/users.ts (1)
1197-1199:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKnown limitation: endpoint error formatting inconsistency.
The TODO comment and workaround assertion indicate that the endpoint currently wraps the custom field validation error inside an
error-invalid-bodyresponse instead of returning it directly. While this is documented in the PR objectives as a known limitation, this test pattern has a maintenance concern: when the endpoint is fixed to returnerror-user-registration-custom-fielddirectly, this test will continue passing with the old assertion and won't catch the improvement.Consider opening a follow-up issue to track the endpoint error formatting fix, and update this test assertion once that's resolved.
🤖 Prompt for 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. In `@apps/meteor/tests/end-to-end/api/users.ts` around lines 1197 - 1199, Create a follow-up issue to track fixing the endpoint error formatting and then update the test where the TODO and workaround exist: remove the TODO and the current nested assertion that checks for errorType 'error-invalid-body' and the nested 'body.error' value, and replace it with a direct assertion that res.body.errorType (or res.body.error) equals 'error-user-registration-custom-field' so the test will fail once the endpoint returns the correct error shape; reference the existing expect lines in this test to locate and change the assertions.
🤖 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.
Outside diff comments:
In `@apps/meteor/tests/end-to-end/api/users.ts`:
- Around line 1197-1199: Create a follow-up issue to track fixing the endpoint
error formatting and then update the test where the TODO and workaround exist:
remove the TODO and the current nested assertion that checks for errorType
'error-invalid-body' and the nested 'body.error' value, and replace it with a
direct assertion that res.body.errorType (or res.body.error) equals
'error-user-registration-custom-field' so the test will fail once the endpoint
returns the correct error shape; reference the existing expect lines in this
test to locate and change the assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3eab151a-07f3-4dcf-8fe2-7fa5aeb81ce5
📒 Files selected for processing (1)
apps/meteor/tests/end-to-end/api/users.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: Hacktron Security Check
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/tests/end-to-end/api/users.ts
🧠 Learnings (3)
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/tests/end-to-end/api/users.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/tests/end-to-end/api/users.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/tests/end-to-end/api/users.ts
🔇 Additional comments (6)
apps/meteor/tests/end-to-end/api/users.ts (6)
836-840: LGTM!
849-849: LGTM!Also applies to: 870-870, 888-888, 905-905, 924-924
935-970: LGTM!
972-1063: LGTM!
1065-1104: LGTM!
1106-1164: LGTM!
KevLehman
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
2 participants
Proposed changes (including videos or screenshots)
Adds API integration test coverage for POST
/api/v1/users.register settings-driven registrationflows inapps/meteor/tests/end-to-end/api/users.ts.Accounts_RegistrationFormDisabledAccounts_RegistrationForm+Accounts_RegistrationForm_SecretURLAccounts_ManuallyApproveNewUsersreasonAccounts_AllowedDomainsListAccounts_CustomFieldsOther changes:
userto ausers[]array so all successfully registered test users are deleted inafterbeforeEach/afterEachto avoid polluting other testsIssue(s)
QA-116
Steps to test or reproduce
Further comments
Known limitation — custom field rejection error format: The empty required custom field test currently asserts
error-invalid-body(with the underlyingerror-user-registration-custom-fieldnested inbody.error). This is becauseusers.registerpasses the rawMeteor.ErrortoAPI.v1.failure(e), producing a response that fails schema validation in test mode. A TODO is left to asserterror-user-registration-custom-fielddirectly once the endpoint formats the error consistently withusers.create.Summary by CodeRabbit