Up roles defaults #7965
Up roles defaults #7965
Conversation
|
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
diocas
force-pushed
the
up_roles_defaults
branch
from
2ac7321
to
1d05622
Compare
|
@kulmann I think I solved the tests' issues... Ready for review. |
diocas
force-pushed
the
up_roles_defaults
branch
from
1d05622
to
530f00d
Compare
diocas
force-pushed
the
up_roles_defaults
branch
from
530f00d
to
11c4e9c
Compare
|
Made some changes in the tests. Let me know if it's not ok. Maybe we could extend the tests as well. |
|
Kudos, SonarCloud Quality Gate passed! |
| ...(canEditFile ? [linkRoleEditorFile] : []) | ||
| ].filter((r) => r.folder === isFolder) | ||
| } | ||
|
|
||
| static getByBitmask(bitmask: number, isFolder: boolean): ShareRole { | ||
| return [...this.all, linkRoleEditorFile, linkRoleInternalFile, linkRoleInternalFolder] // Always return all roles | ||
| .find((r) => r.folder === isFolder && r.bitmask(false) === bitmask) | ||
| return this.list(isFolder, true, true, true, false).find((r) => r.bitmask(false) === bitmask) |
There was a problem hiding this comment.
This means 2 finds, but the list is small.. Let me know if you prefer this more optimized.
There was a problem hiding this comment.
that's fine. I prefer the readability over the performance in this case (like you said, only few items in the list).
| ...(canEditFile ? [linkRoleEditorFile] : []) | ||
| ].filter((r) => r.folder === isFolder) | ||
| } | ||
|
|
||
| static getByBitmask(bitmask: number, isFolder: boolean): ShareRole { | ||
| return [...this.all, linkRoleEditorFile, linkRoleInternalFile, linkRoleInternalFolder] // Always return all roles | ||
| .find((r) => r.folder === isFolder && r.bitmask(false) === bitmask) | ||
| return this.list(isFolder, true, true, true, false).find((r) => r.bitmask(false) === bitmask) |
There was a problem hiding this comment.
that's fine. I prefer the readability over the performance in this case (like you said, only few items in the list).
Follow up to what we discussed in our last meeting (some weeks ago).
This is not final yet (the tests are failing, and I need to check why, as the default behaviour should stay the same...) and I would appreciate feedback.
The Contributor role is now optional, since we do not support it in CERBox.
It also makes the re-sharing permission optional, as allowing re-sharing doesn't mean we want to make it the default.
We discussed this and you were ok with the change, but now I see that oCIS is not honouring the change... Example: I have re-sharing default to false, in the share dialog I set the Viewer role, the request sends the proper permission code (1), together with the role name, but oCIS replies with the sharing permission (1 + 16). Is it using the role name to define the permissions?
Also, the implementation might need some suggestions...