#38673 closed enhancement (fixed)
Introduce helper function for DISALLOW_FILE_MODS checks
Reported by: | MaximeCulea | Owned by: | swissspidy |
---|---|---|---|
Milestone: | 4.8 | Priority: | normal |
Severity: | normal | Version: | 4.7 |
Component: | Upgrade/Install | Keywords: | needs-patch |
Focuses: | Cc: |
Description
Hi guys,
Before submitting a real issue and the according patch, I would like to know how you guys feel about the following situation.
As an agency, I do not allow my clients to update core, plugins, so on. So, I set the DISALLOW_FILE_MODS
to true.
But by doing this, no one on site is anymore able to install a new language, translate i18n strings with Locostranslate, for ie, as they are based, and with good reasons, on the DISALLOW_FILE_MODS
var. This is due of a maybe file creation.
For now, I described the current and "correct" way of using and doing it. But as far as WordPress evolved since v4.0.0, the available language dropdown and other features are limited as we could say "in the same basket".
My question is, why we couldn't create a new var which affect these languages concern DISALLOW_LANGUAGE_MODS
?
Talking about the core, it would have an effect on :
wp_download_language_pack
wp_can_install_language_pack
Waiting for you responses ! :raised_hands:
Attachments (8)
Change History (45)
This ticket was mentioned in Slack in #core-i18n by maximeculea. View the logs.
8 years ago
#5
@
8 years ago
- Keywords dev-feedback 2nd-opinion removed
Related: #38664
If we'd go with a wp_disallow_file_mods()
it should probably get a $context
argument.
#6
@
8 years ago
I like this way to handle this ! Thx for the feedback @ocean90 @swissspidy
Moreover, I agree with @ocean90 as we need to pass a
context var to define where it has been called.
If everyone agrees on this I can supply the patch soon ;)
#7
@
8 years ago
Hah, I was actually going to suggest passing the context to the function and the filter as well.
@MaximeCulea Go for it :-)
#8
@
8 years ago
- Keywords has-patch dev-feedback added; needs-patch removed
Please find the according patch in attached files : patch-38673.diff.
By applying this patch, people can now filter the DISALLOW_FILE_MODS
constant. So, to conclude on the way to add languages, or whatever, while DISALLOW_FILE_MODS
is activated, it is only needed to filter wp_disallow_file_mods()
like this :
<?php add_action( 'disallow_file_mods_download_language_pack', '__return_false' );
@swissspidy, I am now why wondering about doing this .. because we are like trespassing security behaviour of php constants which could not be redefined, isn't it ?
#9
@
8 years ago
- Keywords needs-refresh added; dev-feedback removed
- Milestone changed from Awaiting Review to Future Release
patch-38673.diff needs to follow the PHP Documentation Standards.
I am now why wondering about doing this .. because we are like trespassing security behaviour of php constants which could not be redefined, isn't it ?
I don't see a problem with that. Constants are not the ideal way to change these things so it makes sense to open it up.
#10
@
8 years ago
I followed the doc to reformat the filter and add some phpdoc.
Is the following patch-38673-1.diff good now ? @swissspidy
#12
@
8 years ago
- Keywords needs-refresh added
It seems that your last patch miss some lines of the wp_disallow_file_mods
function.
#13
@
8 years ago
Thx @Mista-Flo, had to reinstall git locally cause it crashed on my laptop. As the patch is easy, I just edit it with sublime, but now it is the full one : patch-38673-4.diff !
This ticket was mentioned in Slack in #core-multisite by ocean90. View the logs.
8 years ago
#18
@
8 years ago
- Component changed from I18N to Filesystem API
- Milestone changed from Future Release to 4.8
#20
@
8 years ago
Related to this is #39677.
The original goal from this ticket can be achieved via either, but in the end I think both tickets are valid as they provide different kinds of flexibility.
#21
@
8 years ago
@flixos90 yeap !
You are playing with roles while I am just refactoring way to check existing roles.
Two tickets providing, as you say, two different ways of flexibility :)
#22
@
8 years ago
Moving the file mods check to a function is a good step nonetheless. It's not just because of the translations.
#23
@
8 years ago
- Summary changed from Handle languages as 'DISALLOW_FILE_MODS' is activated to Introduce helper function for DISALLOW_FILE_MODS checks
#28
@
8 years ago
Just ran into this, and the function name seems incorrect.
How about wp_is_file_mod_allowed()
instead?
(wp_disallow_file_mods()
reads (to me) like I would use it to disallow file modifications.)
#29
@
8 years ago
@johnjamesjacoby I totally agree with you on the name.
But as other constants through helpers, I wanted to keep the same (old) pattern as @swissspidy suggested it for naming the method.
#30
@
8 years ago
- Keywords needs-patch added; has-patch commit removed
- Resolution fixed deleted
- Status changed from closed to reopened
Agreed with j³. The function name should use some form of _is_
terminology.
#32
@
8 years ago
Here we are : patch-38673-5.diff.
I have renamed the func name from "wp_disallow_file_mods" to "wp_is_file_mod_allowed" and added some refactoring/documentation.
This ticket was mentioned in Slack in #core by jeffpaul. View the logs.
7 years ago
#34
@
7 years ago
@MaximeCulea wp_is_file_mod_allowed()
has the opposite meaning of wp_disallow_file_mods()
so all the checks would need to be negated, i.e. wp_is_file_mod_allowed() === ! wp_disallow_file_mods()
Hey there,
Thanks for your report!
There are indeed some valid reasons these functions rely on
DISALLOW_FILE_MODS
. But before we add another constant, we should rather think about introducing a newwp_disallow_file_mods()
function that checks this function and is filterable. Example:See #37436 for some patches that partly implemented this already.
This way, one can easily allow file mods under certain circumstances using the filter.