Make utf8mb4 status check language more for the general consumer #15946
Conversation
|
(Standard links)
|
|
@seamuslee001 Sounds like a good improvement. Nitpick: can you use double-quotes for the string, to avoid escaping the single quote? I forget how the string extraction works, but I think it's best avoided (and readability of the code). |
seamuslee001
force-pushed
the
status_check_improve
branch
from
24300b6 to
cabf97e
Compare
|
ok have done so now @mlutfy |
|
I think including the relevant configuration here is helpful for some - so they can send this message to the sysadmin and the sysadmin can see exactly what needs to be tweaked without doing further research into CiviCRM. I'd say these technical details should be hidden behind a "technical details" button though - I just wasn't sure how to do that when I added the message.. |
CRM/Utils/Check/Component/Env.php
Outdated
| @@ -898,7 +898,7 @@ public function checkMysqlUtf8mb4() { | |||
| else { | |||
| $messages[] = new CRM_Utils_Check_Message( | |||
| __FUNCTION__, | |||
| ts('Future versions of CiviCRM may require MySQL utf8mb4 support. It is recommended, though not yet required, to configure your MySQL server for utf8mb4 support. You will need the following MySQL server configuration: innodb_large_prefix=true innodb_file_format=barracuda innodb_file_per_table=true'), | |||
| ts("Future versions of CiviCRM may require MySQL to support utf8mb4 encoding. It is recommended, though not yet required, Please discuss with your server administrator about configuring your MySQL server for utf8mb4. CiviCRM's recommended configurations are in the <a href='%1' title='System Administrator Guide'>System Administrator Guide</a>", [1 => CRM_Utils_System::docURL2("sysadmin/requirements/#mysql-configuration", TRUE)]), | |||
There was a problem hiding this comment.
grammar seems wrong: a comma followed by capital P.
|
@seamuslee001 I would actually be in favour of not displaying the "utf8mb4 support" pop-up alert to any users at all as it's not a message that any user can quickly action. As my guide, I believe user alerts need to be actionable by the user that sees them. As an alternative, can the message just be shown on the CiviCRM status page only? |
|
Please see also the gitlab I raised 6 months ago and is relevant here, https://lab.civicrm.org/dev/core/issues/979 |
|
This PR is related to #13425 |
@mfb Just to clarify: is this in support of the change, or do you think we should go further? Personally, if someone emails me with that error message, I would be pretty happy, since I could quickly reply with a recommended solution without doing further digging. (i.e. rather than displaying a generic message, such as "in the future your CiviCRM will stop working", and then a consulting will charge $500 to do diagnostics, scare the user into paying another $2000 to fix it, and meanwhile the user will migrate to another CRM) :-) |
|
Yes, I'm asking why remove the detailed instructions on how to resolve the issue? I realize not everyone is a sysadmin or DBA, but they can copy/paste the warning message to the relevant person to have it worked on. It does seem good to have a more general explanation - "Your database server does not use the configuration recommended by CiviCRM" - with details hidden behind a "technical details" button. |
|
@mfb why couldn't they just as easily copy and past the link the docs, these status checks are shown to non technical folks, I feel like others even drupal probably do it that way where they just have a link to the page on their website with more detail explanation of the types of configs you should have in place |
|
Ah hmm, yeah I think the error message as proposed by Seamus is preferable, and the docs are a better place to describe the fix (which might change in time, and we won't want to update civicrm core every time) Seamus: could you fix the typo? ("It is recommended, though not yet required," => period, instead of comma?) |
|
I've seen constant confusion about this in chat.civicrm.org so I wanted to be clear in the warning message, but sure it is there in the docs so linking there works fine - as long as they read down to the last bullet point :) |
seamuslee001
force-pushed
the
status_check_improve
branch
from
cabf97e to
3e9f9a5
Compare
|
ok have fixed the grammar and also change the title of the check to MySQL emoji support (utf8mb4) so its a bit more human as well |
Fix grammar and change title of check to be a bit more human related
seamuslee001
force-pushed
the
status_check_improve
branch
from
3e9f9a5 to
5aebe76
Compare
|
Looks good to me. Last call? |
Overview
This alters the language on the utf8mb4 status check so that it is a bit more general in nature and possibly a bit more applicable to differing audiences
Before
Language very technical
After
Language less technical
ping @jusfreeman @eileenmcnaughton @mfb