Fix Users page not updating fields correctly (#1105)

* fix: 🐛 Normalize update responses to track removed fields

✅ Closes: ICU-4189

addons/api/addon/models/user.js

@@ -40,7 +40,7 @@ export default class UserModel extends GeneratedUserModel {
    * There are many options, so we fallback in order of precedence.
    * @type {string}
    */
-  @computed('email,full_name,login_name,displayName')
+  @computed('email', 'full_name', 'login_name')
   get accountName() {
     const { email, full_name, login_name } = this;
     return email || full_name || login_name;

addons/api/addon/serializers/application.js

@@ -137,7 +137,7 @@ export default class ApplicationSerializer extends RESTSerializer {
   }
 
   /**
-   * In our API, singluar resources are _unrooted_, whereas Ember Data expects
+   * In our API, singular resources are _unrooted_, whereas Ember Data expects
    * them to be rooted under their model name, e.g. `{modelName: {…}}`.
    * This method makes this transformation to accommodate Ember Data.
    *
@@ -268,4 +268,46 @@ export default class ApplicationSerializer extends RESTSerializer {
     });
     return hash;
   }
+
+  /**
+   * If the API returns an updated response with removed fields, we need to
+   * explicitly set any read only field that is on the model but wasn't
+   * returned in the response to `null`.
+   * This will let ember data know the field was removed, otherwise
+   * it won't touch the removed field and will use the old value.
+   *
+   * @override
+   * @param {object} store
+   * @param {object} primaryModelClass
+   * @param {object} payload
+   * @param {?string} id
+   * @param {string} requestType
+   * @return {object}
+   */
+  normalizeUpdateRecordResponse(
+    store,
+    primaryModelClass,
+    payload,
+    id,
+    requestType
+  ) {
+    primaryModelClass.attributes.forEach((attribute) => {
+      const {
+        name,
+        options: { readOnly },
+      } = attribute;
+
+      if (readOnly && !Object.hasOwn(payload, name)) {
+        payload[name] = null;
+      }
+    });
+
+    return super.normalizeUpdateRecordResponse(
+      store,
+      primaryModelClass,
+      payload,
+      id,
+      requestType
+    );
+  }
 }

addons/api/tests/unit/models/user-test.js

@@ -115,4 +115,39 @@ module('Unit | Model | user', function (hooks) {
       'User has empty account_ids by default'
     );
   });
+
+  test('it defaults to null for readonly attributes when response does not have fields', async function (assert) {
+    assert.expect(3);
+    this.server.post('/v1/users/123abc:remove-accounts', (schema, request) => {
+      const body = JSON.parse(request.requestBody);
+      assert.deepEqual(body, {
+        account_ids: ['3'],
+        version: 1,
+      });
+      return { id: '123abc', name: 'newName' };
+    });
+    const store = this.owner.lookup('service:store');
+    store.push({
+      data: {
+        id: '123abc',
+        type: 'user',
+        attributes: {
+          name: 'user',
+          description: 'Description',
+          account_ids: ['3'],
+          version: 1,
+          login_name: 'zed',
+          scope: {
+            scope_id: 'o_1',
+            type: 'scope',
+          },
+        },
+      },
+    });
+    const model = store.peekRecord('user', '123abc');
+    await model.removeAccount('3');
+
+    assert.equal(model.login_name, null);
+    assert.equal(model.name, 'newName');
+  });
 });

addons/api/tests/unit/serializers/application-test.js

@@ -211,4 +211,41 @@ module('Unit | Serializer | application', function (hooks) {
       },
     });
   });
+
+  test('it normalizes removed fields correctly', function (assert) {
+    assert.expect(1);
+    const store = this.owner.lookup('service:store');
+    const serializer = store.serializerFor('user');
+    const user = store.createRecord('user').constructor;
+    const payload = {
+      id: '1',
+      name: 'user 1',
+      scope: { id: 'o_123' },
+    };
+    const normalized = serializer.normalizeUpdateRecordResponse(
+      store,
+      user,
+      payload
+    );
+    assert.deepEqual(normalized, {
+      included: [],
+      data: {
+        id: '1',
+        type: 'user',
+        attributes: {
+          authorized_actions: [],
+          name: 'user 1',
+          scope: { scope_id: 'o_123' },
+          account_ids: [],
+          authorized_collection_actions: null,
+          created_time: null,
+          email: null,
+          full_name: null,
+          login_name: null,
+          updated_time: null,
+        },
+        relationships: {},
+      },
+    });
+  });
 });

ui/admin/app/routes/scopes/scope/users/user/accounts.js

@@ -33,7 +33,7 @@ export default class ScopesScopeUsersUserAccountsRoute extends Route {
   // =actions
 
   /**
-   * Remove a account from the current role and redirect to accounts index.
+   * Remove an account from the current role and redirect to accounts index.
    * @param {UserModel} user
    * @param {AccountModel} account
    */

ui/admin/app/routes/scopes/scope/users/user/add-accounts.js

@@ -15,7 +15,7 @@ export default class ScopesScopeUsersUserAddAccountsRoute extends Route {
   // =methods
 
   /**
-   * Emtpy out any previously loaded acconuts.
+   * Empty out any previously loaded accounts.
    */
   beforeModel() {
     this.store.unloadAll('account');