Only update changed data (#7538)

Only patch user data if changes are detected
owncloud · Aug 31, 2022 · 273a5f4 · 273a5f4
1 parent 7487249
commit 273a5f4
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 44 deletions.
diff --git a/changelog/unreleased/enhancemnet-user-management-edit-quota b/changelog/unreleased/enhancemnet-user-management-edit-quota
@@ -4,5 +4,6 @@ We've added the possibility to change user's quota in the user management app.
 
 https://github.com/owncloud/web/pull/7182
 https://github.com/owncloud/web/pull/7530
+https://github.com/owncloud/web/pull/7538
 https://github.com/owncloud/web/issues/7059
 
diff --git a/packages/web-app-user-management/src/components/Users/SideBar/EditPanel.vue b/packages/web-app-user-management/src/components/Users/SideBar/EditPanel.vue
@@ -52,8 +52,8 @@
     </div>
     <compare-save-dialog
       class="edit-compare-save-dialog"
-      :original-object="originalObjectUser"
-      :compare-object="compareObjectUser"
+      :original-object="user"
+      :compare-object="editUser"
       :confirm-button-disabled="invalidFormData"
       @revert="revertChanges"
       @confirm="$emit('confirm', editUser)"
@@ -100,16 +100,6 @@ export default {
     }
   },
   computed: {
-    userRole() {
-      return this.user ? this.userRoles[this.user.id] : null
-    },
-
-    originalObjectUser() {
-      return { ...this.user, passwordProfile: { password: '' } }
-    },
-    compareObjectUser() {
-      return { ...this.editUser }
-    },
     invalidFormData() {
       return Object.values(this.formData)
         .map((v) => !!v.valid)
@@ -122,7 +112,7 @@ export default {
   watch: {
     user: {
       handler: function () {
-        this.editUser = { ...cloneDeep(this.user), ...{ passwordProfile: { password: '' } } }
+        this.editUser = cloneDeep(this.user)
       },
       deep: true,
       immediate: true
@@ -132,7 +122,6 @@ export default {
     changeSelectedQuotaOption(option) {
       this.editUser.drive.quota.total = option.value
     },
-
     validateDisplayName() {
       this.formData.displayName.valid = false
 
@@ -145,7 +134,6 @@ export default {
       this.formData.displayName.valid = true
       return true
     },
-
     validateEmail() {
       this.formData.email.valid = false
 
@@ -158,9 +146,8 @@ export default {
       this.formData.email.valid = true
       return true
     },
-
     revertChanges() {
-      this.editUser = { ...cloneDeep(this.user), ...{ passwordProfile: { password: '' } } }
+      this.editUser = this.user
       Object.values(this.formData).forEach((formDataValue) => {
         formDataValue.valid = true
         formDataValue.errorMessage = ''

diff --git a/packages/web-app-user-management/src/views/Users.vue b/packages/web-app-user-management/src/views/Users.vue
@@ -76,6 +76,8 @@
 </template>
 
 <script lang="ts">
+import isEqual from 'lodash-es/isEqual'
+import omit from 'lodash-es/omit'
 import UsersList from '../components/Users/UsersList.vue'
 import CreateUserModal from '../components/Users/CreateUserModal.vue'
 import DeleteUserModal from '../components/Users/DeleteUserModal.vue'
@@ -169,6 +171,7 @@ export default defineComponent({
 
       users.value.forEach((user) => {
         user.memberOf = user.memberOf || []
+        user.passwordProfile = user.passwordProfile || { password: '' }
       })
 
       yield loadGroupsTask.perform()
@@ -409,9 +412,24 @@ export default defineComponent({
     },
     async editUser(editUser) {
       try {
-        await this.graphClient.users.editUser(editUser.id, editUser)
+        const actualUser = this.users.find((user) => user.id === editUser.id)
 
-        if (editUser.drive) {
+        const graphEditUserRawObjectExtractor = (user) => {
+          return omit(user, ['drive', 'role'])
+        }
+
+        if (
+          !isEqual(
+            graphEditUserRawObjectExtractor(actualUser),
+            graphEditUserRawObjectExtractor(editUser)
+          )
+        )
+          await this.graphClient.users.editUser(
+            editUser.id,
+            graphEditUserRawObjectExtractor(editUser)
+          )
+
+        if (!isEqual(actualUser.drive, editUser.drive)) {
           const updateDriveResponse = await this.graphClient.drives.updateDrive(
             editUser.drive.id,
             { quota: { total: editUser.drive.quota.total } },
@@ -427,25 +445,30 @@ export default defineComponent({
           }
         }
 
-        /**
-         * Setting api calls are just temporary and will be replaced with the graph api,
-         * as the backend supports it.
-         */
-        await axios.post(
-          '/api/v0/settings/assignments-add',
-          {
-            account_uuid: editUser.id,
-            role_id: editUser.role.id
-          },
-          {
-            headers: {
-              authorization: `Bearer ${this.accessToken}`
+        if (!isEqual(actualUser.role, editUser.role)) {
+          /**
+           * Setting api calls are just temporary and will be replaced with the graph api,
+           * as the backend supports it.
+           */
+          await axios.post(
+            '/api/v0/settings/assignments-add',
+            {
+              account_uuid: editUser.id,
+              role_id: editUser.role.id
+            },
+            {
+              headers: {
+                authorization: `Bearer ${this.accessToken}`
+              }
             }
-          }
-        )
+          )
+        }
 
-        const user = this.users.findIndex((user) => user.id === editUser.id)
-        this.$set(this.users, user, editUser)
+        this.$set(
+          this.users,
+          this.users.findIndex((user) => user.id === editUser.id),
+          editUser
+        )
         /**
          * The user object gets actually exchanged, therefore we update the selected users
          */

diff --git a/packages/web-app-user-management/tests/unit/components/Users/SideBar/EditPanel.spec.js b/packages/web-app-user-management/tests/unit/components/Users/SideBar/EditPanel.spec.js
@@ -82,7 +82,13 @@ function getWrapper({ propsData = {} } = {}) {
       $gettextInterpolate: jest.fn()
     },
     propsData: {
-      user: { id: '1', displayName: 'jan', mail: '[email protected]', drive: { quota: {} } },
+      user: {
+        id: '1',
+        displayName: 'jan',
+        mail: '[email protected]',
+        passwordProfile: { password: '' },
+        drive: { quota: {} }
+      },
       roles: [{ id: '1', displayName: 'admin' }],
       ...propsData
     },

diff --git a/packages/web-app-user-management/tests/unit/views/Users.spec.js b/packages/web-app-user-management/tests/unit/views/Users.spec.js
@@ -47,14 +47,11 @@ describe('Users view', () => {
         id: '1',
         displayName: 'jan',
         role: { id: '1', displayName: 'admin' },
-        drive: { id: '1', quota: { total: 10000 } }
+        drive: { id: '1', quota: { total: 10000 } },
+        passwordProfile: { password: 'newpassword' }
       }
 
-      const wrapper = getMountedWrapper({
-        mocks: {
-          users: []
-        }
-      })
+      const wrapper = getMountedWrapper()
       const busStub = jest.spyOn(bus, 'publish')
       const setStub = jest.spyOn(wrapper.vm, '$set')
       const updateSpaceFieldStub = jest.spyOn(wrapper.vm, 'UPDATE_SPACE_FIELD')
@@ -288,7 +285,7 @@ function getMountedWrapper({
       return {
         selectedUsers: [
           {
-            id: 1,
+            id: '1',
             memberOf: []
           }
         ],