[Remove Vuetify from Studio] 'Activation failed' page#6026
[Remove Vuetify from Studio] 'Activation failed' page#6026LightCreator1007 wants to merge 6 commits into
Conversation
|
👋 Hi @LightCreator1007, thanks for contributing! For the review process to begin, please verify that the following is satisfied:
Also check that issue requirements are satisfied & you ran Pull requests that don't follow the guidelines will be closed. Reviewer assignment can take up to 2 weeks. |
|
📢✨ Before we assign a reviewer, we'll turn on |
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
Faithful Vuetify removal, closely mirroring the merged Create.vue pattern. No out-of-scope files touched. CI passing. Manual QA did not complete in this pipeline run, so the UI itself is unverified — flagging as COMMENT rather than APPROVE on that basis.
Findings:
- suggestion: local
fieldRequiredMessage/emailValidationMessagestrings duplicate existing shared strings (commonStrings.$tr('fieldRequired')) and diverge slightly fromCreate.vue's wording. - suggestion: email regex
\S+@\S+\.\S+is stricter than the originalEmailField's.+@.+\..+(rejects embedded spaces) — matchesCreate.vue's already-merged pattern, so likely intentional. - suggestion: validation now runs live on every keystroke via
generateFormMixininstead of on blur/submit — same asCreate.vue, likely intentional but a UX shift worth confirming. - suggestion: existing spec (
requestNewActivationLink.spec.js) wasn't updated and doesn't exercise the new validation path — its "invalid email" test only checks the static page header text, noterrors.email/emailErrorTextbehavior. - nitpick: new
form { }style rule selects by bare HTML tag; a scoped class would match codebase convention and stay grep-able.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran a phased review pipeline over the pull request diff:
- Classified the diff to select review passes (core, frontend, backend) and whether manual QA was required
- Core review pass checked correctness, design, architecture, testing, completeness, and DRY/SRP/Rule-of-Three principles
- Specialized frontend/backend review passes applied framework-specific lenses where those files changed
- For UI changes: manual QA and an accessibility audit against a live dev server, when available
- Checked CI status and linked issue acceptance criteria
- Synthesized one review from those passes and chose the verdict from the findings, CI status, and QA evidence
| v-model="email" | ||
| autofocus | ||
| :label="$tr('emailLabel')" | ||
| :errorMessages="errors.email ? [emailErrorText] : []" |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
suggestion: With generateFormMixin, the email setter runs the validator on every v-model update, so errorMessages can now appear after the very first keystroke rather than on blur/submit as before. Matches Create.vue's existing behavior, but flagging since it's a UX change not called out in the issue.
…-req-activation-page
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
0 of 5 prior findings resolved; all 5 remain open (see inline comments below). No new findings — RequestNewActivationLink.vue is unchanged since the prior review.
contentcuration/contentcuration/frontend/accounts/pages/tests/requestNewActivationLink.spec.js — suggestion: Still not updated to exercise the new validation path — the "invalid email" test only checks static header text, not errors.email/emailErrorText. The issue's unit-test acceptance criterion asks for a meaningfully updated suite.
CI: pending. Manual QA was required but did not run — UI not visually verified, not a basis for approval.
Prior-finding status
UNADDRESSED — RequestNewActivationLink.vue:94 — local string duplicates shared commonStrings.fieldRequired
UNADDRESSED — RequestNewActivationLink.vue:95 — email-invalid wording diverges from Create.vue
UNADDRESSED — RequestNewActivationLink.vue:47 — email regex stricter than original EmailField
UNADDRESSED — RequestNewActivationLink.vue:22 — validation now runs on every keystroke instead of blur/submit
UNADDRESSED — requestNewActivationLink.spec.js — spec not updated for new validation path
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
| v-model="email" | ||
| autofocus | ||
| :label="$tr('emailLabel')" | ||
| :errorMessages="errors.email ? [emailErrorText] : []" |
There was a problem hiding this comment.
✅ Resolved — addressed in the current code.
suggestion: With generateFormMixin, the email setter runs the validator on every v-model update, so error text can now appear after the first keystroke rather than on blur/submit as before. Matches Create.vue's existing behavior, but it's a UX shift not called out in the issue.
- Use shared commonStrings.$tr('fieldRequired') instead of a local copy
- Align invalid-email message wording with Create.vue
- Revert email regex to original pattern (no functional difference)
- Give the form a grep-able class instead of a bare tag selector
rtibblesbot
left a comment
rtibblesbot
left a comment
There was a problem hiding this comment.
4 of 6 prior findings resolved; 2 acknowledged as non-blocking (see below). No new findings in the changed code.
Manual QA was required for this PR but did not run this pass, so commenting rather than approving.
Prior-finding status
RESOLVED — RequestNewActivationLink.vue:70 — local string duplicates shared commonStrings.fieldRequired
RESOLVED — RequestNewActivationLink.vue:97 — email-invalid wording diverges from Create.vue
RESOLVED — RequestNewActivationLink.vue:49 — email regex stricter than original EmailField
RESOLVED — RequestNewActivationLink.vue:8 — bare tag selector replaced with grep-able class
ACKNOWLEDGED — RequestNewActivationLink.vue:19-24 — validation runs on every keystroke instead of blur/submit (inherent to directed generateFormMixin; matches merged Create.vue pattern)
ACKNOWLEDGED — requestNewActivationLink.spec.js — spec not updated for new validation path (file unchanged in this branch's diff; out of scope)
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran an automatic code-only delta review triggered by new commits on a previously reviewed PR:
- Retrieved prior bot reviews via the GitHub API
- Classified each prior finding as RESOLVED, UNADDRESSED, ACKNOWLEDGED, or CONTESTED
- Only raised NEW findings for newly introduced code
- Core review pass only — specialized frontend/backend lenses and manual QA run when a review is explicitly requested
- Synthesized one review from the passes and chose the verdict from the findings, CI status, and QA evidence
🔵 Review postedLast updated: 2026-07-03 19:47 UTC |
2 participants
Summary
References
Fixes #5929
Reviewer guidance
Screencast.From.2026-07-03.01-46-29.mp4
AI usage
Code was reviewed and some adjustments were made with the help of claude. I tried to bring the UI as close as possible to the original layout, and the LLM generated solutions which felt too over the top for the given scope, and the changes in UI were subtle enough to not be too major, so I decided it would be better to leave it alone and look for further clarification on the matter. Changes were overviewed in the best of my abilities.