[Remove Vuetify from Studio] 'Activation failed' page by LightCreator1007 · Pull Request #6026 · learningequality/studio

LightCreator1007 · 2026-07-02T20:31:39Z

Summary

Replaced MessageLayout with StudioMessageLayout
Replaced VForm with and use generateFormMixin for validation
Replaced Banner with StudioBanner
Replaced EmailField with StudioEmailField

References

Fixes #5929

Reviewer guidance

Screencast.From.2026-07-03.01-46-29.mp4

there were minor UI changes from the original.
The tests still use the unit test suite, should i port it to VTL in this PR or a separate one?

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.

learning-equality-bot · 2026-07-02T20:32:20Z

👋 Hi @LightCreator1007, thanks for contributing!

For the review process to begin, please verify that the following is satisfied:

Contribution is aligned with our contributing guidelines
Pull request description has correctly filled AI usage section & follows our AI guidance:
AI guidance

State explicitly whether you didn't use or used AI & how.

If you used it, ensure that the PR is aligned with Using AI as well as our DEEP framework. DEEP asks you:
- Disclose — Be open about when you've used AI for support.
- Engage critically — Question what is generated. Review code for correctness and unnecessary complexity.
- Edit — Review and refine AI output. Remove unnecessary code and verify it still works after your edits.
- Process sharing — Explain how you used the AI so others can learn.
Examples of good disclosures:

"I used Claude Code to implement the component, prompting it to follow the pattern in ComponentX. I reviewed the generated code, removed unnecessary error handling, and verified the tests pass."

"I brainstormed the approach with Gemini, then had it write failing tests for the feature. After reviewing the tests, I used Claude Code to generate the implementation. I refactored the output to reduce verbosity and ran the full test suite."

Also check that issue requirements are satisfied & you ran pre-commit locally.

Pull requests that don't follow the guidelines will be closed.

Reviewer assignment can take up to 2 weeks.

learning-equality-bot · 2026-07-03T04:18:54Z

📢✨ Before we assign a reviewer, we'll turn on @rtibblesbot to pre-review. Its comments are generated by an LLM, and should be evaluated accordingly.

rtibblesbot

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/emailValidationMessage strings duplicate existing shared strings (commonStrings.$tr('fieldRequired')) and diverge slightly from Create.vue's wording.
suggestion: email regex \S+@\S+\.\S+ is stricter than the original EmailField's .+@.+\..+ (rejects embedded spaces) — matches Create.vue's already-merged pattern, so likely intentional.
suggestion: validation now runs live on every keystroke via generateFormMixin instead of on blur/submit — same as Create.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, not errors.email/emailErrorText behavior.
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

rtibblesbot · 2026-07-03T04:33:54Z

        v-model="email"
        autofocus
+        :label="$tr('emailLabel')"
+        :errorMessages="errors.email ? [emailErrorText] : []"


✅ 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

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

rtibblesbot · 2026-07-03T13:16:49Z

        v-model="email"
        autofocus
+        :label="$tr('emailLabel')"
+        :errorMessages="errors.email ? [emailErrorText] : []"


✅ 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

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

rtibblesbot · 2026-07-03T22:14:11Z

🟡 Waiting for changes

Last updated: 2026-08-18 11:43 UTC

akolson

Thanks @LightCreator1007! Code changes look great generally! As you point out in your review notes, its probably good to update the the test file too. It has already been migrated to to VTL so a few updates should be good enough in case something has changed. It's good time time get rid of the hard-coded strings and use the actual translations as guided here.

studio/contentcuration/contentcuration/frontend/accounts/pages/__tests__/requestNewActivationLink.spec.js

Lines 46 to 76 in 020c8dd

    
           describe('requestNewActivationLink', () => { 
        
             it('should show validation error when submitting with invalid email', async () => { 
        
               const user = userEvent.setup(); 
        
               renderComponent(); 
        
               const submitButton = screen.getByRole('button', { name: /submit/i }); 
        
               await user.click(submitButton); 
        
               await waitFor(() => { 
        
                 expect(screen.getByText(/activation failed/i)).toBeInTheDocument(); 
        
               }); 
        
             }); 
        
             it('should submit when email is valid', async () => { 
        
               const user = userEvent.setup(); 
        
               renderComponent(); 
        
               const sendActivationLink = jest.spyOn(testStore, 'dispatch'); 
        
               const emailInput = screen.getByLabelText(/email/i); 
        
               const submitButton = screen.getByRole('button', { name: /submit/i }); 
        
               await user.type(emailInput, 'test@test.com'); 
        
               await user.click(submitButton); 
        
               await waitFor(() => { 
        
                 expect(sendActivationLink).toHaveBeenCalledWith( 
        
                   'account/sendActivationLink', 
        
                   'test@test.com', 
        
                 ); 
        
               }); 
        
             });

sharma-anushka · 2026-08-18T06:26:45Z

Hi @akolson, in case of inactivity, I would love to take this up and do the changes ?

Replaces the mislabeled "invalid email" test (it submitted an empty form and only asserted on the static page header) with tests that actually exercise errors.email/emailErrorText, adds coverage for the request-failure banner, and swaps hardcoded string literals for the component's own $trs and the shared fieldRequired translation.

rtibblesbot

PR #6026 — all 5 prior findings resolved; 1 new suggestion on the test delta (inline).

CI pending at review time; manual QA did not run, so the UI is unverified.

Prior-finding status

RESOLVED — RequestNewActivationLink.vue:23 — email setter validates on every update, error timing
RESOLVED — RequestNewActivationLink.vue:70 — fieldRequiredMessage duplicated commonStrings.fieldRequired
RESOLVED — RequestNewActivationLink.vue:97 — 'Please enter a valid email' diverged from Create.vue
RESOLVED — RequestNewActivationLink.vue:49 — email regex stricter than original EmailField
RESOLVED — RequestNewActivationLink.vue:106 — bare form { ... } style selector

@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

rtibblesbot · 2026-08-18T08:02:11Z

+      name: RequestNewActivationLink.$trs.submitButton,
+    });
+
+    await user.type(emailInput, 'not-an-email');


suggestion: The validation tests don't assert the submit was blocked. generateFormMixin's field setter validates on every v-model update (shared/mixins.js:406-413), so the message is already rendered by the end of user.type — the click is incidental and the assertion passes regardless of what requestActivationLink does. Both this test and the empty-email one would stay green if submit dispatched with invalid data.

Adding expect(sendActivationLink).not.toHaveBeenCalled() closes the gap; renderComponent already accepts an injectable mock, so it just needs hoisting into the test. forgotPassword.spec.js:45-56 is the existing pattern.

@LightCreator1007, this seems like a valid add that would make the test more robust. Thoughts?

probably worth adding the expect(sendActivationLink).not.toHaveBeenCalled() as suggested to close the gap.

LightCreator1007 added 2 commits July 3, 2026 01:48

refactor: removed vuetify from request new activation link page

03897c4

fixes

04827f1

[pre-commit.ci lite] apply automatic fixes

790052e

rtibbles requested a review from rtibblesbot July 3, 2026 04:18

rtibblesbot reviewed Jul 3, 2026

View reviewed changes

Merge branch 'learningequality:unstable' into refactor/remove-vuetify…

6c76e37

…-req-activation-page

rtibblesbot reviewed Jul 3, 2026

View reviewed changes

LightCreator1007 and others added 2 commits July 4, 2026 00:56

[pre-commit.ci lite] apply automatic fixes

cd5a893

rtibblesbot reviewed Jul 3, 2026

View reviewed changes

rtibbles assigned akolson Aug 11, 2026

akolson requested changes Aug 17, 2026

View reviewed changes

rtibblesbot reviewed Aug 18, 2026

View reviewed changes

	describe('requestNewActivationLink', () => {
	it('should show validation error when submitting with invalid email', async () => {
	const user = userEvent.setup();
	renderComponent();

	const submitButton = screen.getByRole('button', { name: /submit/i });
	await user.click(submitButton);

	await waitFor(() => {
	expect(screen.getByText(/activation failed/i)).toBeInTheDocument();
	});
	});

	it('should submit when email is valid', async () => {
	const user = userEvent.setup();
	renderComponent();
	const sendActivationLink = jest.spyOn(testStore, 'dispatch');

	const emailInput = screen.getByLabelText(/email/i);
	const submitButton = screen.getByRole('button', { name: /submit/i });

	await user.type(emailInput, 'test@test.com');
	await user.click(submitButton);

	await waitFor(() => {
	expect(sendActivationLink).toHaveBeenCalledWith(
	'account/sendActivationLink',
	'test@test.com',
	);
	});
	});

Uh oh!

Uh oh!

Conversation

LightCreator1007 commented Jul 2, 2026

Summary

References

Reviewer guidance

AI usage

Uh oh!

learning-equality-bot Bot commented Jul 2, 2026

Uh oh!

learning-equality-bot Bot commented Jul 3, 2026

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rtibblesbot Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rtibblesbot Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

rtibblesbot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟡 Waiting for changes

Uh oh!

akolson left a comment

Choose a reason for hiding this comment

Uh oh!

sharma-anushka commented Aug 18, 2026

Uh oh!

rtibblesbot left a comment

Choose a reason for hiding this comment

Uh oh!

rtibblesbot Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

akolson Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

akolson Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rtibblesbot Jul 3, 2026 •

edited

Loading

rtibblesbot Jul 3, 2026 •

edited

Loading

rtibblesbot commented Jul 3, 2026 •

edited

Loading