#39677 closed enhancement (fixed)
Introduce capabilities for managing translations
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| 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 185
- wp-admin/options-general.php line 147
- wp-admin/network/settings.php line 66 and 345
- wp-admin/network/site-new.php line 68 and 236
- wp-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
@flixos90
10 months ago
- Keywords has-patch has-unit-tests 2nd-opinion added
- Milestone changed from Awaiting Review to Future Release
#2
@Mista-Flo
10 months ago
+1 on this, we have same usage, disallow plugin/themes/core updates in production, but we want to allow language pack updates.
#3
@flixos90
8 months 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
@flixos90
7 months 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
@ocean90
7 months ago
- Keywords needs-patch added; has-patch dev-feedback removed
- wp_can_install_language_pack() should be part of the current_user_can( 'install_languages' ) check IMO.
- Mapping update_languages/install_languages only to update_core is not the same as before because it's currently update_core or update_plugins or update_themes.
#6
@flixos90
7 months 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 the map_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 require wp-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 the user_has_cap filter to grant anyone the install_languages capability that has at least one out of the update_core, install_plugins and install_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 map update_languages to install_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.
7 months ago
#8
@flixos90
6 months 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.
4 months ago
#10
@jbpaul17
4 months ago
- Milestone changed from 4.8.1 to 4.9
Punting to 4.9 per today's bug scrub.
#12
@flixos90
4 months ago
- Keywords granular-capabilities added
All of these are part of a larger goal.
#13
@flixos90
4 months 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
@johnbillion
4 months ago
#14
follow-up:
↓ 15
@johnbillion
4 months 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 to wp_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
@flixos90
4 months 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 include update_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.
#16
@johnbillion
4 months ago
I'm liking the look of this. Let's go ahead and get it into trunk.
This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.
4 months ago
#19
@flixos90
3 months ago
- Resolution set to fixed
- Status changed from reviewing to closed
In 41268:
#20
@jeremyfelt
5 weeks ago
- Keywords needs-dev-note removed
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.