dev/cloud-native/issues#18: Soften messages for read-only extensionsDir #12623
Conversation
|
Can one of the admins verify this patch? |
|
(Standard links)
|
|
test this please |
|
@tiotsop01 I'm fine with downgrading the error level - I'm just not sure what the Settings file means in the below context 'Writing to Settings file has been disabled by admin' Perhaps you mean 'Please check with your system admin if file permissions have been restricted deliberately'? @agh1 this was your wording in the first place |
There was a problem hiding this comment.
I think it's fine to downgrade a non-writable extensions directory to notice. When we picked warning, it's because it seemed to fit the PSR-3 standard of a warning: an exceptional occurrence that's not an error--an undesirable thing that is not necessarily wrong. However, if we see the situation of in-app extension downloads as a nice bonus rather than a core feature, I can see it matching the notice criteria.
On the other hand, those other directories really need to be writable. Also, the "writing to Settings file" text doesn't really describe what's going on. If you want to elaborate in the message, it might be better to have different messages keyed to each file that describe the problem. However, I think the labels are enough, and the main thing for the site admin is that they need to go make them writable regardless.
| @@ -453,16 +453,16 @@ public function checkDirsWritable() { | |||
| if (!empty($notWritable)) { | |||
| $messages[] = new CRM_Utils_Check_Message( | |||
| __FUNCTION__, | |||
| ts('The %1 is not writable. Please check your file permissions.', array( | |||
| ts('The %1 is not writable. Writing to Settings file has been disabled by admin. Please check your file permissions.', array( | |||
There was a problem hiding this comment.
I agree with @eileenmcnaughton that this text "writing to Settings file has been disabled by admin" is confusing. This warning can appear in several situations, but I don't think any of them involve writing to a settings file.
| )), | ||
| ts('Directory not writable', array( | ||
| 'count' => count($notWritable), | ||
| 'plural' => 'Directories not writable', | ||
| )), | ||
| \Psr\Log\LogLevel::ERROR, | ||
| \Psr\Log\LogLevel::NOTICE, |
There was a problem hiding this comment.
This should remain an error: it's a real problem if CiviCRM can't upload import files, images, or custom files. In nearly all cases, a site that can't write to these folders will appear "broken" to someone trying to do regular tasks like running imports or uploading a file to a custom field.
|
Just adding a link back to dev/cloud-native#18 for convenience. |
|
Closing this in favour of #13100 which has the agreed portion of this PR (which is the initial subject of it) |
Overview
Improve messaging when someone has a different policy for managing extensionsDir. Most of the messaging update has already been done. There is only one message ("Read-Only Extensions"). It still encourages web-writable policy, but it lowers the severity and presents it a choice ("if you want X, do Y").
Before
The message that is displayed is a
warningwhen the directory is "Read-Only" which implies something is wrong.After
The message that is displayed is a
noticewhen the directory is "Read-Only".noticesays it's merely out of the ordinaryComments
This a continuation of this PR #11895.
dev/cloud-native#18