Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only update changed data #7538

Merged
merged 6 commits into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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

Original file line number Diff line number Diff line change
Expand Up @@ -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)"
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -132,7 +122,6 @@ export default {
changeSelectedQuotaOption(option) {
this.editUser.drive.quota.total = option.value
},

validateDisplayName() {
this.formData.displayName.valid = false

Expand All @@ -145,7 +134,6 @@ export default {
this.formData.displayName.valid = true
return true
},

validateEmail() {
this.formData.email.valid = false

Expand All @@ -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 = ''
Expand Down
61 changes: 42 additions & 19 deletions packages/web-app-user-management/src/views/Users.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -169,6 +171,7 @@ export default defineComponent({

users.value.forEach((user) => {
user.memberOf = user.memberOf || []
user.passwordProfile = user.passwordProfile || { password: '' }
})

yield loadGroupsTask.perform()
Expand Down Expand Up @@ -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) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ehhhhhhhhhhhhhhhhhh

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 } },
Expand All @@ -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),
kulmann marked this conversation as resolved.
Show resolved Hide resolved
editUser
)
/**
* The user object gets actually exchanged, therefore we update the selected users
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down
11 changes: 4 additions & 7 deletions packages/web-app-user-management/tests/unit/views/Users.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -288,7 +285,7 @@ function getMountedWrapper({
return {
selectedUsers: [
{
id: 1,
id: '1',
memberOf: []
}
],
Expand Down