#39677 closed enhancement (fixed)
Introduce capabilities for managing translations
Reported by: | flixos90 | Owned by: | flixos90 |
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Role/Capability | Keywords: | has-unit-tests, has-patch, granular-capabilities, commit |
Focuses: | Cc: |
Description
We currently have capabilities for managing core updates, plugin installation/updates and theme installation/updates. However we lack capabilities for managing translation installation/updates, so let's add them. This is especially critical as it's currently not possible to allow a user to only install/update translations while not allowing them to install/update anything else.
I suggest introducing install_translations
and update_translations
. I think they should be added as primitive caps to the administrator
role (would need to happen in WP version update function), just how the other above caps work. We would then also need to add them to map_meta_cap()
, where it could currently go into the same group as the other caps I mentioned above. However, let's also have an eye on #38673, as the changes on that ticket might require us to handle the map_meta_cap()
stuff differently.
About usage of these capabilities, there are several areas in core where they could be used:
wp-admin/options.php
line 185wp-admin/options-general.php
line 147wp-admin/network/settings.php
line 66 and 345wp-admin/network/site-new.php
line 68 and 236wp-admin/update-core.php
line 617 and 732
Regarding wp_can_install_language_pack()
, I think the current_user_can( 'install_translations' )
check should not go into that function as that function is also used during setup where it should be accessible to anyone.
Attachments (5)
Change History (25)
#1
@
3 years ago
- Keywords has-patch has-unit-tests 2nd-opinion added
- Milestone changed from Awaiting Review to Future Release
#2
@
3 years ago
+1 on this, we have same usage, disallow plugin/themes/core updates in production, but we want to allow language pack updates.
#3
@
3 years ago
- Keywords needs-patch needs-unit-tests added; has-patch has-unit-tests 2nd-opinion removed
- Milestone changed from Future Release to 4.8
- Owner set to flixos90
- Status changed from new to assigned
As discussed with @swissspidy at WC Torino, let's plan this for 4.8.
The current patch should be updated to use meta capabilities instead of introducing new primitive capabilities (I don't know why I did this). We also decided to use the term "languages" instead of "translations" for the capability names.
Let's introduce general capabilities as a first step (install_languages
and update_languages
). We can iterate later and introduce more granular ones (such as current_user_can( 'install_language', $locale )
).
#4
@
3 years ago
- Keywords has-patch has-unit-tests dev-feedback needs-dev-note added; needs-patch needs-unit-tests removed
39677.2.diff is an updated patch with changes as described above. It introduces meta capabilities install_languages
and update_languages
.
Since new capabilities should be documented in a dev note, I'm adding the respective keyword.
This enhancement will furthermore fix another part of #37616 by the removal of two is_super_admin()
checks.
#5
@
3 years ago
- Keywords needs-patch added; has-patch dev-feedback removed
wp_can_install_language_pack()
should be part of thecurrent_user_can( 'install_languages' )
check IMO.- Mapping
update_languages
/install_languages
only toupdate_core
is not the same as before because it's currentlyupdate_core
orupdate_plugins
orupdate_themes
.
#6
@
3 years ago
- Keywords has-patch added; needs-patch removed
39677.3.diff takes @ocean90's notes into account.
Improvements over the previous patch:
wp_can_install_language_pack()
is now included in themap_meta_cap()
clause for the two new capabilities. This ensures that no-one has the capability in case the filesystem does not allow such changes. Something to be particularly careful about is that the clause may requirewp-admin/includes/translation-install.php
now. I don't think this will cause problems, but it's a change to be aware of. This theoretically makes some require statements in other files unnecessary, but I left them in there for clarity at this point.- In order to "map" the capability correctly, we cannot use
map_meta_cap()
efficiently because it always checks whether all capabilities are met. Therefore I used theuser_has_cap
filter to grant anyone theinstall_languages
capability that has at least one out of theupdate_core
,install_plugins
andinstall_themes
capabilities, basically as if this was a regular capability that was part of a the administrator role in the DB.map_meta_cap()
is used for the more detailed handling of the capability and to mapupdate_languages
toinstall_languages
. - All related function calls have been adjusted to solely rely on
current_user_can( 'install/update_languages' )
now.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
3 years ago
#8
@
3 years ago
- Milestone changed from 4.8 to 4.8.1
While this is close to ready, it still needs a further review. Let's come back to it soon, but let's not rush it into 4.8.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
3 years ago
#13
@
3 years ago
- Keywords changed from has-unit-tests, needs-dev-note, has-patch, granular-capabilities to has-unit-tests needs-dev-note has-patch granular-capabilities
- Owner changed from flixos90 to swissspidy
- Status changed from assigned to reviewing
#14
follow-up:
↓ 15
@
3 years ago
I'm happy with this change, and it mostly looks very good. Changes I've made in 39677.4.diff:
- In
site-new.php
, ensure a chosen existing language can still get set for a site when the user cannot install languages. - Remove
@todo
notes which are now done since [29630]. - Add a
wp_
prefix towp_maybe_grant_install_languages_cap()
.
Remaining question: Should the three cap checks at the top of update-core.php
(line 22) be altered to include update_languages
? Is this screen usable if a user only has that cap?
#15
in reply to:
↑ 14
@
3 years ago
- Keywords commit added
Replying to johnbillion:
Remaining question: Should the three cap checks at the top of
update-core.php
(line 22) be altered to includeupdate_languages
? Is this screen usable if a user only has that cap?
I tested it, with only having the update_languages
capability (out of the four updating caps), and it works perfectly fine. Good point bringing that up. In 39677.5.diff I added the necessary check to the top of update-core.php
and also to menu.php
so that the menu item shows up correctly.
A sidenote: Looking at the wp-admin/network/menu.php
file, I noticed in that file only the update_core
capability is checked. Let's ignore this here and deal with it in a separate ticket post-merge. I opened #41538 for this.
39677.diff is a patch with what I have in mind. I wonder if
*_translations
is the best name for the capabilities, as*_languages
might also be suitable.