Feature: Media Library configure the view #14700
Conversation
jhoward1994
added
source: core:upload
Codecov ReportBase: 60.22% // Head: 60.32% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #14700 +/- ##
==========================================
+ Coverage 60.22% 60.32% +0.10%
==========================================
Files 1355 1368 +13
Lines 33048 33282 +234
Branches 6324 6357 +33
==========================================
+ Hits 19903 20079 +176
- Misses 11297 11356 +59
+ Partials 1848 1847 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
jhoward1994
force-pushed
the
feature/media-library-ctv
branch
3 times, most recently
from
7091f80
to
4afa486
Compare
|
Hi @petersg83 @Marc-Roig @nathan-pichon This PR contains the backend changes to the upload package, adding configure the view with a I have another PR targeting this branch for the frontend changes. Once both FE and BE changes are approved I will merge everything here and use this branch for QA :) |
jhoward1994
force-pushed
the
feature/media-library-ctv
branch
from
4afa486
to
3090f3a
Compare
test: e2e chore: naming
jhoward1994
force-pushed
the
feature/media-library-ctv
branch
from
3090f3a
to
c8c1678
Compare
test(upload): useConfig hook
…feature/media-library-ctv-front
…feature/media-library-ctv-front
…feature/media-library-ctv-front
| if (userAbility.cannot(ACTIONS.configureView, FILE_MODEL_UID)) { | ||
| return ctx.forbidden(); | ||
| } | ||
|
|
There was a problem hiding this comment.
I was looking at the content-manager as a reference and I am seeing each plugin deals with permissions a little bit different. For instance:
-
content-manager has a permissions services, where it registers all the actions & has functions like canConfigureContentType. But in upload we have a constants file.
-
Super minor thing, we say getViewConfiguration here and findViewConfiguration in there.
The part that I understand the less is how actions are registered in the upload plugin. And if there is a "standard"way of doing it in the repo.
There was a problem hiding this comment.
Thanks for the review
- Do you suggest we move to the permission checker service approach in the upload plugin? imo we use the
userAbilityin quite a simple way in theuploadplugin so think calling.cannotdirectly is clear enough - Good point I have renamed it to
findViewConfiguration - In terms of how the actions are registered you can see in
packages/core/admin/server/services/role.jswe have added the new permissionplugin::upload.configure-view. L285. This is how I understand the functionality:- Every time strapi boots the function
resetSuperAdminPermissionsis run, granting super admins all permissions createRolesIfNoneExistis responsible for assigning permissions toeditorandauthorroles. This is only run on strapis first boot. Therefore the new CTV permission will not automatically be added to editors and authors but can be added through the Admin UI
- Every time strapi boots the function
There was a problem hiding this comment.
Do you suggest we move to the permission checker service approach in the upload plugin? imo we use the userAbility in quite a simple way in the upload plugin so think calling .cannot directly is clear enough
Agree, I think it is super clean like you have now. So I would leave it as it is!
Thanks a lot for the actions explanation 🙇
|
Hi @joshuaellis & @petersg83. I've addressed your feedback, let me know what you think 🙂 |
| requests.admin = await createAuthRequest({ strapi }); | ||
| requests.restricted = await createAuthRequest({ strapi, userInfo: restrictedUser }); |
There was a problem hiding this comment.
very clean way to do different auth requests 💡
| test('Successfully update the ML view configuration', async () => { | ||
| const config = { | ||
| pageSize: 100, | ||
| sort: sample(ALLOWED_SORT_STRINGS), |
There was a problem hiding this comment.
minor thing, maybe you could use ALLOWED_SORT_STRINGS.slice(1) so it gets a different value from the default one.
There was a problem hiding this comment.
LGTM, I left a minor comment in a test file.
When I change the entries per page and click the <- Back button it is correctly changed, but if I use the browser back button I see the previous value and I have to refresh (maybe because the entries per page are stored in the URL params)
jhoward1994
added
the
flag: documentation
jhoward1994
removed
the
flag: documentation
What does it do?
Adds configure the view to the media library
Why is it needed?
In order to set the default page size and sort order for the Media Library across an entire Strapi project
How to test it?
packages/core/upload/server/routes/view-configuration.jspackages/core/upload/tests/admin/configure-view.test.e2e.jspackages/core/upload/admin/src/pages/AppRelated issue(s)/PR(s)
Frontend changes will be merged into this branch from #14697