Simplify text describing updates in the site status report

Hi All!

I am working this issue at Drupalcon Nashville right now. Feel free to stop by room 103 if you want to connect. I'm in a yellow shirt.

Thanks!

Scratch that. I am moving onto something more frontend. This issue is open!

I'm gonna have a go at this one.

So have done the text mods detailed in the ticket. I have also split the long t() commands into single lines as per coding conventions and replaced the deprecated url function. The mods have been tested both as an admin and authenticated user with status report access.

This is just about there. Excellent. Patch applied cleanly. Text is replaced per specification, and method is replaced per specification. To meet coding standards empty line at 574 needs to be removed, and commas need to be placed after closing paren on lines 122, 130. Did not check email response.

1. +++ b/core/modules/update/update.install
   @@ -114,10 +114,20 @@ function _update_requirement_check($project, $type) {
   +            [':pending_updates' => \Drupal\Core\Url::fromRoute('update.report_update')->toString()]
   

   Drupal\Core\Url is already imported so you can directly use Url.

2. +++ b/core/modules/update/update.install
   @@ -114,10 +114,20 @@ function _update_requirement_check($project, $type) {
   +            [':available_updates' => \Drupal\Core\Url::fromRoute('update.status')->toString()]
   

   Same as above.

3. +++ b/core/modules/update/update.module
   @@ -567,12 +567,12 @@ function _update_message_text($msg_type, $msg_reason, $langcode = NULL) {
   +        [':update-report' => \Drupal\Core\Url::fromRoute('update.status')->toString() ],
   

   Same as above.

Thanks @borwickja and @msankhala - I have made those changes and rerun my previous manual tests.

+++ b/core/modules/update/update.module
@@ -567,12 +567,11 @@ function _update_message_text($msg_type, $msg_reason, $langcode = NULL) {
+      // \Drupal::url('update.status')

Remove commented code. Sorry missed this in first round of review.

@msankhala - thanks for that. Removed now.

Hi @benjifisher - thanks for that

- spaces before closing array bracket resolved
- both files already use Drupal\Core\Url
- #12 dealt with in #13 commit
- screenshots below show status report page in both scenarios
- will do email screenshot imminently

Updates available email screenshot

Working on this patch at Design4Drupal in Boston.

Rerolled the patch with these changes:
Change \Drupal::url to Url::fromRoute.
Put use Drupal\Core\Url; in alphabetical order.

Interdiff attached.

@esod Hi All the things are alredy solved on latest code. So the thing only need to do is
"Put use Drupal\Core\Url; in alphabetical order."
So i am creating a new patch for this.

Thanks for moving this forward, everyone.

A) I don't see what this issue has to do with 'Accessibility', and see no mention of why that tag is here in any comment. Removing the tag. If someone thinks this issue has accessibility implications, please explicitly mention them in the Problem/Motivation section of the summary.

B) Patch #26 doesn't make sense. That's definitely not the only change needed here. What happened to the rest of #19?

C) #19 needs a re-roll, and #26 isn't it. ;) Tagging for a genuine re-roll.

Thanks,
-Derek

Hi @dww,
1. Patch #19 doesn't apply with 9.1.x dist.
2. I went through core/modules/update/update.install and checked what was changed in #19.
3. "\Drupal::url" has been replaced by "Url::fromRoute", so there is no requirement of that patch.
4. Also in core/modules/update/update.module there is no requirement of the changes.

Hi @Saurabh_sgh, thanks for looking. Re: #30:

1. Agreed, #19 doesn't apply anywhere, anymore. ;)
2: No, that's not the only change in update.install. There's also the wording change, which is the original/main purpose of this issue.
3: Agreed, that change (which was out of scope for this issue, anyway) has already been fixed in the modern branches of core. Sadly, looks like most of the changes introduced in #19 were out of scope. Oh well.
4. Again, mostly agreed, but there are some specific wording changes that should be kept.

Here's an accurate re-roll/re-do of #19 based on the current state of core. It's wildly different from earlier patches, so an interdiff is basically meaningless.

I doubt this will be eligible for a backport all the way to 8.8.x (since it's changing strings). But on the off chance it's considered, also including a re-roll for that branch. The raw 2919559-31.patch applies cleanly to 9.1.x, 9.0.x and 8.9.x branches.

1. +++ b/core/modules/update/update.install
   @@ -128,10 +128,21 @@ function _update_requirement_check($project, $type) {
   +          '#markup' => t('Review and install <a href=":pending_updates">pending up
   +dates</a>.',
   

   Stray newline. Sorry.

2. +++ b/core/modules/update/update.install
   @@ -128,10 +128,21 @@ function _update_requirement_check($project, $type) {
   -        $requirement['description'][] = ['#prefix' => ' ', '#markup' => t('See the <a href=":available_updates">available updates</a> page for more information.', [':available_updates' => Url::fromRoute('update.status')->toString()])];
   +        $requirement['description'][] = [
   +          '#prefix' => ' ',
   +          '#markup' => t('See the <a href=":available_updates">available updates</a> page for more information.',
   +            [':available_updates' => Url::fromRoute('update.status')->toString()]
   +          ),
   +        ];
   

   Upon closer inspection, I think this is out-of-scope code style play. I'm going to undo it.

Also, I'm going to not bother with 8.8.x patches until this lands in a higher branch and a core committer requests a backport. ;)

I'm going to create the issue fork in order to review it using DrupalPod.

Thanks for reviving this issue! FYI: DrupalPod can work with patches, too. ;)

From my test using patch #32: the new message appears correctly in the Drupal status page:

I'm going to create the merge request.

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

Seems there were failures in the MR.

Not sure what needs to be done for translation updates

Also since this is a UX change tagging for usability review.

I'm removing the "Needs screenshots" tags as the Issue summary has been updated on #18 with the screenshots needed.

For item 2 in the Issue Summary. The text in update.install was changed in #3182405: Do not use verb "Install" for things other than turning on modules/themes and Url:fromRoute was added in #2731817: Replace all calls to the deprecated Drupal::url() function in Core. The text is now
See the <a href=":available_updates">available updates</a> page for more information and to update your software.
It was changed to remove 'install'.

Item #1. This is still valid. That text was added in 2006, #94154: update.module in core (formerly known as update_status), and has not been changed since.

This should have before and after screenshots, made after the MR is updated.

Per 46

I have already addressed the issue mentioned in #46 @smustgrave. Please review my latest commit in #47 in 2082 MR.
Let me know if I have missed anything.

Don't think you read the full comment :) It says to include screenshots

Before:

After:

Thanks @kostyashupenko!

Unfortunately this is still tagged for usability review.

The issue summary also doesn't describe the proposed solution. There is also a task regarding the update email notification, and it's not clear whether that should be changed as part of this issue or not.