Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

Role select ui #89

Merged
merged 19 commits into from
Aug 21, 2020
Merged

Role select ui #89

merged 19 commits into from
Aug 21, 2020

Conversation

kulmann
Copy link
Member

@kulmann kulmann commented Aug 19, 2020

Split from #54

Todo:

@kulmann kulmann changed the base branch from add-permissions-for-roles to master August 19, 2020 16:08
@kulmann
Copy link
Member Author

kulmann commented Aug 21, 2020

rebased on #90, let's see what happens

Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

some changes needed, as discussed

this.getUsersCurrentRole()
} else {
this.showMessage({
title: 'Failed to change role.',
Copy link
Contributor

Choose a reason for hiding this comment

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

gettext

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

})

if (response.status === 201) {
this.getUsersCurrentRole()
Copy link
Contributor

Choose a reason for hiding this comment

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

use response body for populating

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

commit('SET_ROLES', roles || [])
} else {
dispatch('showMessage', {
title: 'Failed to fetch roles.',
Copy link
Contributor

Choose a reason for hiding this comment

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

gettext

Copy link
Member Author

Choose a reason for hiding this comment

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

didn't fix. apparently it's not that easy to access $gettext in vuex store, since it has no access to the Vue instance. Raised this ticket instead, since we are using $gettext in a wrong way: owncloud/web#3984

ui/tests/acceptance/features/accounts.feature Show resolved Hide resolved
@@ -11,3 +11,11 @@ Then('user {string} should be displayed in the accounts list on the WebUI', asyn
const userListed = await client.page.accountsPage().isUserListed(username)
return assert.strictEqual(userListed, username)
})

When('the users changes the role of user {string} to {string} using the WebUI', function (username, role) {
Copy link
Contributor

Choose a reason for hiding this comment

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

"the user" ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed this typo

Previously we were fetching the role assignments again. Not necessary,
as the request for setting a role already returns the resulting
assignment object as a response.
Apparently the step definition for the page reload gets reused from phoenix.
@ownclouders
Copy link

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- ui/helpers/utils.js  2
- ui/helpers/auth.js  3
         

Complexity decreasing per file
==============================
+ ui/store/index.js  -1
         

Clones added
============
- ui/client/settings/index.js  84
         

See the complete overview on Codacy

@sonarcloud
Copy link

sonarcloud bot commented Aug 21, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@kulmann kulmann requested a review from PVince81 August 21, 2020 14:34
Copy link
Contributor

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@kulmann kulmann merged commit 1803a5b into master Aug 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants