From 8a04fa4b9c8f50ae5f7ac65d1e47e7bf4a2f5524 Mon Sep 17 00:00:00 2001 From: Alex Kanunnikov Date: Fri, 26 Jun 2026 09:57:12 +0000 Subject: [PATCH] Fix blank profile after registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The profile page rendered empty right after sign-up: no first/last name, gender unselected, and "Год рождения: NaN" / invalid-date. Root cause is a navigation-vs-task race in registration-form. The shared loginTask called router.transitionTo('index') the moment the session authenticated — in the middle of registrationTask. That navigation tears down the registration-form component, and ember-concurrency auto-cancels the still running registrationTask before patchUserInfo() and the refreshing loadCurrentUser() complete. So userData.userModel kept the empty pre-patch stub (loadCurrentUser otherwise only runs in application.beforeModel at boot), and a missing bornYear made fromLatestUserDto emit the literal string "NaN". Fixes: - registration-form: remove the redirect from loginTask and move it to the end of registrationTask, after patchUserInfo + loadCurrentUser. The redirect now fires whenever the session is authenticated, even if the profile PATCH fails, so a fully-registered user is never trapped on the form (re-submitting would re-run registerUser and fail with "user already exists"); they can finish their profile on the profile page, which patches each field on input. - network: fromLatestUserDto now yields an empty birthday string instead of "NaN" when bornYear is missing/invalid, and drops the vestigial Date round-trip. Tests: - network: birthday parsed from bornYear; missing bornYear -> '' (fails on old). - registration-form: redirect happens only after the profile is patched and reloaded (fails on old ordering); and the user is still redirected when the profile patch fails after auth. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/registration-form/index.gts | 19 +- frontend/app/services/network.ts | 14 +- .../registration-form/component-test.gjs | 168 ++++++++++++++---- frontend/tests/unit/services/network-test.js | 30 ++++ 4 files changed, 186 insertions(+), 45 deletions(-) diff --git a/frontend/app/components/registration-form/index.gts b/frontend/app/components/registration-form/index.gts index de76c10be..9e311073c 100644 --- a/frontend/app/components/registration-form/index.gts +++ b/frontend/app/components/registration-form/index.gts @@ -118,9 +118,10 @@ export default class RegistrationFormComponent extends Component { await this.loginTask.cancelAll(); } - if (this.session.isAuthenticated) { - this.router.transitionTo('index'); - } + // Registration deliberately omits the post-login redirect: registrationTask + // still has to PATCH the profile and reload the user, and navigating away + // here would tear down this component and cancel that work, leaving the + // profile blank. registrationTask redirects once it is done. }); // --- Own getters --- @@ -232,8 +233,13 @@ export default class RegistrationFormComponent extends Component { this.network.loadCloudUrl(), ]); } catch (e) { + // The account exists and the session is authenticated; only the profile + // save failed. Fall through to the redirect rather than trapping the user + // here — a re-submit would re-run registerUser and fail with "user already + // exists". They can finish the profile on the profile page. const error = e as Error & { errors?: string[] }; const key = error.errors?.pop() ?? error.message; + console.error('Failed to save profile after registration:', error); if (this.intl.exists(`msg.validation.${key}`)) { this.errorMessage = this.intl.t(`msg.validation.${key}`); } else { @@ -242,7 +248,12 @@ export default class RegistrationFormComponent extends Component { ? this.intl.t(ERRORS_MAP[key as keyof typeof ERRORS_MAP]) : key; } - await this.registrationTask.cancelAll(); + } + + // Redirect whether or not the PATCH succeeded; this tears down the component + // and cancels the task, so it must be the last step. + if (this.session.isAuthenticated) { + this.router.transitionTo('index'); } }); diff --git a/frontend/app/services/network.ts b/frontend/app/services/network.ts index deb34f7d8..bb2fbd24a 100644 --- a/frontend/app/services/network.ts +++ b/frontend/app/services/network.ts @@ -29,17 +29,19 @@ export interface LatestUserDTO { function fromLatestUserDto(user: LatestUserDTO): UserDTO { const [firstName = '', lastName = ''] = (user.name || '').split(' '); - const bDate = new Date(); - - bDate.setFullYear(user.bornYear); + // `birthday` is just the four-digit year; guard a missing/invalid bornYear so + // the field renders empty instead of "NaN". + const bornYear = Number(user.bornYear); + const birthday = + Number.isInteger(bornYear) && bornYear > 0 ? String(bornYear) : ''; return { - firstName: firstName || '', - lastName: lastName || '', + firstName, + lastName, avatar: user.avatar, email: user.email, gender: user.gender, - birthday: bDate.getFullYear().toString(), + birthday, id: user.id as string, }; } diff --git a/frontend/tests/integration/components/registration-form/component-test.gjs b/frontend/tests/integration/components/registration-form/component-test.gjs index 8a9e0319f..f0e48d80a 100644 --- a/frontend/tests/integration/components/registration-form/component-test.gjs +++ b/frontend/tests/integration/components/registration-form/component-test.gjs @@ -11,6 +11,17 @@ function getDate(num) { return date.getFullYear() + num; } +async function fillAndSubmit() { + await fillIn('[name="firstName"]', 'b'); + await fillIn('[name="email"]', 'c@name.com'); + await fillIn('[name="password"]', 'Test1234'); + await fillIn('[name="repeatPassword"]', 'Test1234'); + await fillIn('[name="birthday"]', '1991'); + await click('[name="agreement"]'); + await click('[id="male"]'); + await click('[data-test-submit-form]'); +} + module('Integration | Component | registration-form', function (hooks) { setupRenderingTest(hooks); setupIntl(hooks, 'en-us'); @@ -25,8 +36,6 @@ module('Integration | Component | registration-form', function (hooks) { }); test('it send register request if all fields filled', async function (assert) { - assert.expect(6); - // eslint-disable-next-line ember/no-classic-classes const MockFirebaseAuthenticator = EmberObject.extend({ registerUser() { @@ -34,22 +43,16 @@ module('Integration | Component | registration-form', function (hooks) { }, }); - let loadCurrentUserCallCount = 0; - let patchUserInfoCalled = false; - class MockNetwork extends Service { loadCurrentUser() { - loadCurrentUserCallCount++; - if (patchUserInfoCalled) { - assert.ok(true, 'loadCurrentUser called after patchUserInfo to refresh profile data'); - } + assert.step('loadCurrentUser'); return Promise.resolve(); } loadCloudUrl() { return Promise.resolve(); } patchUserInfo(fields) { - patchUserInfoCalled = true; + assert.step('patchUserInfo'); assert.ok(fields, 'patchUserInfo called with user fields'); return Promise.resolve(fields); } @@ -57,10 +60,8 @@ module('Integration | Component | registration-form', function (hooks) { class MockSession extends Service { isAuthenticated = false; - authenticate(type, login, password) { - assert.ok(type, 'authenticate called with type'); - assert.ok(login, 'authenticate called with login'); - assert.ok(password, 'authenticate called with password'); + authenticate() { + assert.step('authenticate'); return Promise.resolve(); } } @@ -70,25 +71,126 @@ module('Integration | Component | registration-form', function (hooks) { this.owner.register('service:network', MockNetwork); await render(); - await fillIn('[name="firstName"]', 'b'); - await fillIn('[name="email"]', 'c@name.com'); - await fillIn('[name="password"]', 'Test1234'); - await fillIn('[name="repeatPassword"]', 'Test1234'); - await fillIn('[name="birthday"]', '1991'); - await click('[name="agreement"]'); - await click('[id="male"]'); - await click('[data-test-submit-form]'); - - assert.strictEqual(loadCurrentUserCallCount, 2, 'loadCurrentUser called twice: once during login, once after patchUserInfo'); + await fillAndSubmit(); + + // Login loads the user, then the profile is patched and the user reloaded. + assert.verifySteps([ + 'authenticate', + 'loadCurrentUser', + 'patchUserInfo', + 'loadCurrentUser', + ]); }); - test('it able to handle registration error', async function (assert) { - assert.expect(2); + test('redirects to index only after the profile is patched and reloaded', async function (assert) { + // eslint-disable-next-line ember/no-classic-classes + const MockFirebaseAuthenticator = EmberObject.extend({ + registerUser() { + return Promise.resolve(); + }, + }); + + class MockNetwork extends Service { + loadCurrentUser() { + assert.step('loadCurrentUser'); + return Promise.resolve(); + } + loadCloudUrl() { + return Promise.resolve(); + } + patchUserInfo(fields) { + assert.step('patchUserInfo'); + return Promise.resolve(fields); + } + } + class MockSession extends Service { + isAuthenticated = false; + authenticate() { + // Mirror production: the session becomes authenticated after login. + this.isAuthenticated = true; + return Promise.resolve(); + } + } + + this.owner.register('authenticator:firebase', MockFirebaseAuthenticator); + this.owner.register('service:session', MockSession); + this.owner.register('service:network', MockNetwork); + + // Spy on the real router's transitionTo so the template's s keep + // rendering while we record when the redirect happens. + this.owner.lookup('service:router').transitionTo = (route) => { + assert.step(`transitionTo:${route}`); + }; + + await render(); + await fillAndSubmit(); + + // The redirect must come last — after the profile is patched and reloaded. + // Redirecting earlier cancels the in-flight task and leaves the profile + // blank (the bug this fixes). + assert.verifySteps([ + 'loadCurrentUser', + 'patchUserInfo', + 'loadCurrentUser', + 'transitionTo:index', + ]); + }); + + test('still redirects into the app when the profile patch fails after auth', async function (assert) { // eslint-disable-next-line ember/no-classic-classes const MockFirebaseAuthenticator = EmberObject.extend({ registerUser() { - assert.ok(true, 'registerUser was called'); + return Promise.resolve(); + }, + }); + + class MockNetwork extends Service { + loadCurrentUser() { + return Promise.resolve(); + } + loadCloudUrl() { + return Promise.resolve(); + } + patchUserInfo() { + assert.step('patchUserInfo'); + // The account is already created/authenticated; only the profile save + // fails (e.g. a transient backend error). + return Promise.reject( + Object.assign(new Error('save failed'), { errors: ['save failed'] }), + ); + } + } + + class MockSession extends Service { + isAuthenticated = false; + authenticate() { + this.isAuthenticated = true; + return Promise.resolve(); + } + } + + this.owner.register('authenticator:firebase', MockFirebaseAuthenticator); + this.owner.register('service:session', MockSession); + this.owner.register('service:network', MockNetwork); + + this.owner.lookup('service:router').transitionTo = (route) => { + assert.step(`transitionTo:${route}`); + }; + + await render(); + await fillAndSubmit(); + + // The registered+authenticated user is sent into the app instead of being + // trapped on the form, even though the profile save failed. + assert.verifySteps(['patchUserInfo', 'transitionTo:index']); + }); + + test('it able to handle registration error', async function (assert) { + // eslint-disable-next-line ember/no-classic-classes + const MockFirebaseAuthenticator = EmberObject.extend({ + registerUser() { + assert.step('registerUser'); return Promise.reject(new Error('foo')); }, }); @@ -117,14 +219,10 @@ module('Integration | Component | registration-form', function (hooks) { this.owner.register('service:network', MockNetwork); await render(); - await fillIn('[name="firstName"]', 'b'); - await fillIn('[name="email"]', 'c@name.com'); - await fillIn('[name="password"]', 'Test1234'); - await fillIn('[name="repeatPassword"]', 'Test1234'); - await fillIn('[name="birthday"]', '1991'); - await click('[name="agreement"]'); - await click('[id="male"]'); - await click('[data-test-submit-form]'); + await fillAndSubmit(); + + // registerUser fails, so login/patch never run. + assert.verifySteps(['registerUser']); assert.dom('[data-test-form-error]').hasText('foo'); }); diff --git a/frontend/tests/unit/services/network-test.js b/frontend/tests/unit/services/network-test.js index 517a0668c..b2e9813d1 100644 --- a/frontend/tests/unit/services/network-test.js +++ b/frontend/tests/unit/services/network-test.js @@ -116,7 +116,37 @@ module('Unit | Service | network', function (hooks) { assert.strictEqual(userData.userModel.email, 'test@example.com', 'email is set'); assert.strictEqual(userData.userModel.avatar, '3', 'avatar is set'); assert.strictEqual(userData.userModel.gender, 'MALE', 'gender is set'); + assert.strictEqual(userData.userModel.birthday, '1990', 'birthday parsed from bornYear'); assert.strictEqual(userData.userModel.id, '42', 'id is set'); assert.strictEqual(userData.userModel.initials, 'TU', 'initials computed correctly'); }); + + test('loadCurrentUser leaves birthday empty when bornYear is missing', async function (assert) { + window.server.get('users/current', () => ({ + data: [ + { + id: '43', + name: 'No Year', + email: 'noyear@example.com', + gender: 'FEMALE', + active: true, + avatar: '1', + roles: ['ROLE_USER'], + }, + ], + errors: [], + meta: [], + })); + + const network = this.owner.lookup('service:network'); + const userData = this.owner.lookup('service:user-data'); + + await network.loadCurrentUser(); + + assert.strictEqual( + userData.userModel.birthday, + '', + 'birthday is empty (not "NaN") when bornYear is absent', + ); + }); });