Merge pull request #2923 from Brain-up/fix-empty-profile-after-registration

Fix blank profile after registration

Author: lifeart

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');
     }
   });
 

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,
   };
 }

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,42 +36,32 @@ 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() {
         return Promise.resolve();
       },
     });
 
-    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);
       }
     }
 
     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(<template><RegistrationForm /></template>);
-    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 <LinkTo>s keep
+    // rendering while we record when the redirect happens.
+    this.owner.lookup('service:router').transitionTo = (route) => {
+      assert.step(`transitionTo:${route}`);
+    };
+
+    await render(<template><RegistrationForm /></template>);
+    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(<template><RegistrationForm /></template>);
+    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(<template><RegistrationForm /></template>);
-    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');
   });
 

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',
+    );
+  });
 });