Fix explosion when updating a role (and false positive test)

Fixes FOO-896
flag=granular_permissions_wiki_pages

There was a bug involving updating roles in which the updated
role did not get the group roles added to it. This caused an
explosion in the UI when the expected group role no longer
existed in the newly-updated role. This was made worse because
the selenium test for the permissions page did not enable the
feature flag and so was blind to this error.

This fixes the bug and also makes the selenium test with the FF
on (it's soon to be removed anyway) sensitive to this case.

Test plan:
* Have a custom role
* Go into the permissions page and edit the role by updating
  its name
* That should work and not explode once the name change happens

Change-Id: Ic601772a9decf2cb1431afd90923b0ad66f72ffb
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/246077
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ahmad Amireh <ahmad@instructure.com>
QA-Review: Ahmad Amireh <ahmad@instructure.com>
Product-Review: Ahmad Amireh <ahmad@instructure.com>
This commit is contained in:
Charley Kline 2020-08-26 10:20:18 -05:00
parent f165153249
commit 64971ea1f6
3 changed files with 9 additions and 2 deletions

View File

@ -120,8 +120,12 @@ function groupPermissionEnabled(boolArray) {
/*
* Takes a role and creates a new permission for any groups of granular permissions
* in that role, based on the permission group name.
*
* Mutates its argument (role)
*/
export function groupGranularPermissionsInRole(role) {
if (!role?.permissions) return // some JS tests don't bother to fill this in
const groups = {}
Object.values(role.permissions).forEach(permission => {
// Fix up boolean enabled values to the enabled state

View File

@ -107,8 +107,10 @@ const roles = handleActions(
groupGranularPermissionsInRole(roleToAdd)
return roleSortedInsert(state, roleToAdd)
},
[actionTypes.UPDATE_ROLE]: (state, action) =>
state.map(r => (r.id === action.payload.id ? {...r, ...action.payload} : r)),
[actionTypes.UPDATE_ROLE]: (state, action) => {
groupGranularPermissionsInRole(action.payload)
return state.map(r => (r.id === action.payload.id ? {...r, ...action.payload} : r))
},
[actionTypes.DELETE_ROLE_SUCCESS]: (state, action) =>
state.filter(role => action.payload.id !== role.id)
},

View File

@ -23,6 +23,7 @@ describe "permissions index" do
before :once do
@account = Account.default
@account.enable_feature!(:granular_permissions_wiki_pages)
@subaccount = Account.create!(name: "subaccount", parent_account_id: @account.id)
account_admin_user
end