The same link "available updates" links to two different pages in the same paragraph when there is a problem checking available updates

Changed available updates link

@sdstyles is that going to ensure that if ($msg_type == 'core') { is going to give you the same results as:

  if ($report_link) {
    if (update_manager_access()) {

Might it be better to change the text so that the link text is differentiated:
<a href="@update-report">available updates report</a>
<a href="@available_updates">available updates</a>

It still seems like available updates could have duplicate links going to different places with that patch, but I could be wrong about that.

@xq1003 are you working on a solution too?

@mgifford I've created another patch to fix wrong path. I added a check to verify if user has access to manage updates.
Users with access have the link to Update page (admin/reports/updates/update), users without access can only see Reports page (admin/reports/updates).
These changes are applied for both Core and modules messages.
I thought to rename @update-report link and keep path to report page, but for second case (user will have to different links with same path).

@mgifford @sdstyles, I take a different approach, please correct me if I'm wrong.

Instead of linking user to admin/reports/updates/update for both links, maybe we can change the wording for the first link. Something like

<td>
  <div class="description">There was a problem checking <a href="/drupal/admin/reports/updates">list of available updates</a> for Drupal. See the <a href="/drupal/admin/reports/updates/update">available updates</a> page for more information and to install your missing updates.</div>
</td>

The "List" page contains more information than "Update" page, could be more useful.

@xq1003 I think you've got the better approach. If the two links go to different places, they need different text.

I think it needs a "the" in the text though:
There was a problem checking the <a href="/drupal/admin/reports/updates">list of available updates</a> for Drupal.

Now the Available updates page admin/reports/updates is indeed a list of available updates.

The Update page admin/reports/updates/update is where you can actually apply the updates to contributed modules.

Isn't this more accurate for the 2nd sentence:
Visit the <a href="/drupal/admin/reports/updates/update">update</a> page to apply updates to your contributed modules.

@mgifford I agree with first link, but for the second
Visit the <a href="/drupal/admin/reports/updates/update">update</a> page to apply updates to your contributed modules.
it's ok that this message to be displayed for "Drupal core update status" too? not only for "Module and theme update status"?

@mgifford a new patch. You can see in screenshots the message displayed for administrator, and the message when user has no access to make updates.

@mgifford @sdstyles , I agree with both of you on the firs link.

For the second link, maybe we can try something like

Visit the <a href="/drupal/admin/reports/updates/update">update</a> page for more information and to install your missing updates.

This way, it don't sounds too specific and can be apply to both core and contributed module.

In the patch in #11, a full administrator will see:
There was a problem checking the <a href="/admin/reports/updates">list of available updates</a> for Drupal. See the <a href="/admin/reports/updates/update">available updates</a> page for more information and to install your missing updates.

and if the user can't make updates they see:
There was a problem checking the <a href="/admin/reports/updates">list of available updates</a> for Drupal. Visit the <a href="/admin/reports/updates">update</a> page to see updates to your contributed modules.

It's not as bad as it was, but having two different pieces of text link to the same URL is also not very good. Why would we repeat the link in the first place?

I think this would make more sense to a full administrator:
New software upgrades are available from the Drupal.org repository. A list of projects with upgrades is available from the <a href="/admin/reports/updates">available updates</a> page. To apply upgrades from within the administration interface go to the <a href="/admin/reports/updates/update">available updates</a> page to apply the latest release.

and if the user can't make updates they see:
New software upgrades are available from the Drupal.org repository. A list of projects with upgrades is available from the <a href="/admin/reports/updates">available updates</a> page. Contact your Drupal administrator about applying these upgrades.

Actually, why return anything at all in the second sentence if the user doesn’t have the authority to address updates.

This shouldn't be all that complicated an issue. Trying to keep it super general probably isn't helping us here.

Sentence 1 is informing users that there is a list of updates available (Different links with different text.). Sentence 2 simply tells administrators where they can go to apply updates within the admin interface (Just one link).

I like the direction of #13... The 2nd "if they can't" paragraph I think is good. But the proposed "if they can" pgae goes back to the original problem being reported here, that there are two links in the paragraph both called "available updates", that go to two different places, which is (right?) not good for accessibility.

How about something like this:

New software upgrades are available from the Drupal.org repository. You can view a list of projects with upgrades on the <a href="/admin/reports/updates">available updates report</a> page, or you can <a href="/admin/reports/updates/update">apply updates using the user interface</a>.

I'm not sure how those suggestions would work given that this message displays when the update status can't be determined?

The current behavior is wrong in a lot of ways. The first link is displayed when the calling code specifically requests that it not be displayed (and since the calling code sometimes puts this text in emails, including a link with a non-absolute URL would be bad). The second link duplicates the first (for low-level admins) or makes no sense at all (for high-level admins) - we shouldn't be telling people to go somewhere to "install your missing updates" when we don't know of any updates that are actually missing.

So I think we should just remove the bad links and leave the one good one behind.

The text would be the same for either type of user in this case (see attached screenshot), and the single "available updates" link points to admin/reports/updates only.

Also tagging this for backport - especially if it's an accessibility issue it's worth changing this admin-facing text to fix it.

I noticed that this patch more or less makes it go back to how it was in Drupal 6. This is called "progress" :)

Which made me wonder why it was changed in Drupal 7. Git history points to #639434: Update error messages but if you look at the discussion there, it looks like it was just a mistake. Bojhan (in comment #3) was actually proposing to change the entire text to:

There was a problem checking [available updates] for Drupal.

which is along the lines of the text in my above patch (but even shorter).

But the committed patch in that issue didn't do anything to keep the second sentence from being appended to the first... hence it is still appended.

We could actually just go with Bojhan's text here instead. I actually thought about that independently while writing the above patch, but figured it might be a little too short.

I think short is good. Redundant is bad. @David_Rothstein you made a lot of good points here.

The simple approach from @Bojhan seems fine to me. I wonder if "There was a problem checking [available updates] for Drupal.org." would be a bit clearer for folks who just know that they are using Drupal and might be confused how that could be possible.

We'd just have one sentence & one link. Fits well with the approach we're aiming for in D8 in general.

And we'll just nix the whole section within:

   if ($report_link) {
    if (update_manager_access()) {}}

@jhodgdon I think I left in the wrong link in #13, thanks for catching that. What do you think of simplifying it further to be just one sentence & link as suggested in #15 & #17?

Go for it!

@David_Rothstein, @Bojhan's text which includes a link, as I read it, but think you had the right approach of removing it.
There was a problem checking [available updates] for Drupal.

I also think that there really is no reason to differentiate between Core & modules & themes here. It's not like an available update for a module/theme is going to be any more/less important than a Core update necessarily. The logic really doesn't add much value. If there's a problem checking for updates, there's a problem checking for updates.

Breaking up the two sections to alert users to problems checking for updates and providing a link to administrators made more sense to me the longer I looked at it so I brought over the $skip_update_manager.

In this patch there will be one sentence for everyone who sees the report & two sentences (and a link) for administrators.

Looks reasonable to me, mostly... a couple of small things:

1. +++ b/core/modules/update/update.module
   @@ -516,12 +517,11 @@ function _update_message_text($msg_type, $msg_reason, $report_link = FALSE, $lan
   -      if ($msg_type == 'core') {
   -        $text = t('There was a problem checking <a href="@update-report">available updates</a> for Drupal.', array('@update-report' => \Drupal::url('update.status')), array('langcode' => $langcode));
   -      }
   -      else {
   -        $text = t('There was a problem checking <a href="@update-report">available updates</a> for your modules or themes.', array('@update-report' => \Drupal::url('update.status')), array('langcode' => $langcode));
   -      }
   +      $text = t('There was a problem checking available updates for Drupal.', array(), array('langcode' => $langcode));
   +
   

   It looks like we probably do not need the $msg_type local variable any more? Not sure, but worth checking on...

2. +++ b/core/modules/update/update.module
   @@ -516,12 +517,11 @@ function _update_message_text($msg_type, $msg_reason, $report_link = FALSE, $lan
   +      $text = t('There was a problem checking available updates for Drupal.', array(), array('langcode' => $langcode));
   

   Why do we need "for Drupal" here at all, if this is going to be for both core and contrib?

I don't understand why we'd remove the link to admin/reports/updates? That's the page that gives you a bit more info about what the problem checking available updates actually was, so I'd think we want people to be able to go there...

Also if I'm reading the patch right, it will remove the link in all cases (not just when there are problems checking updates). That definitely doesn't look right to me. For administrators who don't have access to the Update Manager - or for entire sites that don't use the Update Manager at all - admin/reports/updates is the page they go to in order to see all the details about what modules and themes actually need updating.

I also think that there really is no reason to differentiate between Core & modules & themes here.... If there's a problem checking for updates, there's a problem checking for updates.
.....
Why do we need "for Drupal" here at all, if this is going to be for both core and contrib?

I like that combination a lot :) Just saying "There was a problem checking available updates" and leaving it at that sounds perfect to me (as long as there is a link).

Needs work then...

Fixing for comments in #21.2. Regarding point 1, the variable $msg_type is actually a parameter, not a local variable and is used in other case blocks.

Regarding #22, I added the earlier message as pointed out that there must be some way to go to the updates page from here, even if the update check failed here.

I applied the patch on an older version so that I can see the available updates messages. This was accomplished by the command git reset HEAD~500 --hard
As expected, I could see the available updates message. Screenshot attached(originalIssue.png)
After applying the patch in comment#24 using the command git apply -v available-updates-link-2545520-24.patch the message changed. Text "Drupal" has been removed from the message."There was a problem checking available updates". Attaching a screenshot after applying the patch (afterApplyingPatch.png)
Just to double check I reverted the patch using git reset HEAD~1 --hard and the original message was visible.

So... I thought in the discussion above, that people thought we *should* have a link still to both the original updates report, as well as the page that lets you actually perform updates?

It seems like with this patch, if you don't have your site set up so people can do updates from the UI, you don't have any alternative. You still need to see the available updates report so you can see what is out of date, right?

I am working on it this issue from the Barcelona DrupalCon. After words with my mentor, jp.stacey, we need some more works on it according to the commented by jhodgdon.

After reading over this issue a few times, I wanted to clarify the direction before making a patch.

It sounds like, to @jhodgdon and @David_Rothstein's points, we want to include a link to the update report for people who can't do updates via the UI, but still fix the issue of 'update' linking to two different destinations. How about:

There was a problem checking for updates.

And then for administrators to see:

See the available updates page for more information and to install your missing updates.

@mgifford, @jhodgdon - thoughts?

For accessibility, link text should tell you where you are going. "a problem" doesn't tell you where you are going, so it is not great link text. So, the first sentence needs a rewrite so the link text can be something like "updates report" or similar.

What's wrong with the simple solution discussed above?

Just have a single sentence, with the following text:

"There was a problem checking available updates."

with the link pointing to admin/reports/updates.

(Linking to admin/reports/updates/update on top of that seems unnecessary since if there was a problem checking updates there shouldn't be an actual update to perform, right? And in any case, the text above this paragraph - i..e the "Unknown release date" text in the screenshot in the issue summary - links there already.)

I looked over the discussion above, and you're right! I think there was consensus for this simple solution. Sorry for the misdirection in my previous comment. Please ignore #27 and go with #35.

As @David_Rothstein said, I removed "for Drupal" in the sentence "There was a problem checking available updates." and I removed the other sentence "See the available updates...".

Also, I saw this in update.module (line 444):

function update_mail($key, &$message, $params) {
  ...
  foreach ($params as $msg_type => $msg_reason) {
    $message['body'][] = _update_message_text($msg_type, $msg_reason, $langcode);
  }
  $message['body'][] = t('See the available updates page for more information:', array(), array('langcode' => $langcode)) . "\n" . \Drupal::url('update.status', [], ['absolute' => TRUE, 'language' => $language]);
  ...
}

Does the sentence "See the available updates page..." have to be removed as well?

Tested and verified on an older version (8.1.7) so that I can see the available updates message. After applying the patch #38 change_words_so_that-2545520-38_0.patch the message changed to "There are updates available for your version of Drupal. To ensure the proper functioning of your site, you should update as soon as possible."
Screenshots attached.

1. Those screenshots (before vs. after) show that the patch is removing the text in all scenarios, even where there is no duplicate link text.

   Earlier patches such as #15 only removed it in the "there was a problem checking available updates" scenario specifically discussed in this issue.

   Perhaps this makes sense but I'm not sure. ("Out of date" and "available updates" do link to the same place, so I guess there's an argument to be made that the latter is unnecessary here too.) Needs discussion at any rate.

   Also if you're going ahead with that then the whole code comment visible at the top of the patch file (about "format the two translated strings together") no longer makes sense and needs to be changed.

2. Also, I saw this in update.module (line 444):

   function update_mail($key, &$message, $params) {
     ...
     foreach ($params as $msg_type => $msg_reason) {
       $message['body'][] = _update_message_text($msg_type, $msg_reason, $langcode);
     }
     $message['body'][] = t('See the available updates page for more information:', array(), array('langcode' => $langcode)) . "\n" . \Drupal::url('update.status', [], ['absolute' => TRUE, 'language' => $language]);
     ...
   }
   

   Does the sentence "See the available updates page..." have to be removed as well?

   Good question. Seems like it could be removed also (for consistency), but only in the case where it's a duplicate link that already appears elsewhere in the email. Which again seems like it would be limited to the "there was a problem checking available updates" case.

I just tested this patch and changes looks good as per the expectation in the description.

There are unanswered items from the review in #43.

In Drupal-8.3.x-dev (which is what this issue's currently marked as) I get the following:

Drupal core update status	Out of date (version 8.2.5 available)
There are updates available for your version of Drupal. To ensure the proper functioning of your site, you should update as soon as possible. See the available updates page for more information and to install your missing updates.

And:

Module and theme update status	Out of date
There are updates available for one or more of your modules or themes. To ensure the proper functioning of your site, you should update as soon as possible. See the available updates page for more information and to install your missing updates.

Has this issue been corrected elsewhere? Because there's now only the one available updates link showing. If so, does it just need to be backported to Drupal 7?

@omorrill I don't think the duplicate link ever happened in that case - only in the "There was a problem checking available updates" scenario shown in the issue summary and discussed above.

So I guess in practice this actually doesn't occur much for anyone -- it may be most common in practice for developers running a local Git checkout, which doesn't have a release date in the .info.yml files (Drupal 8) or .info files (Drupal 7).

Restoring accidentally deleted tags.

Thanks, @David_Rothstein for helping me clear that up. I've removed the duplicate line so that it's now just the following:

There was a problem checking available updates.

And "available updates" is a link.

I also added the logic you had in your #15 patch to apply only on the "There was a problem checking..." item. I added that same logic to the line in the email body.

I believe that covers what was talked about in #43.

Tested patch #51 locally, works fine. Would be nice to scope the failure and resubmit it.

Update.install did not like that $skip_update_manager was undefined. I believe I've solved that problem--check interdiff.

Can we simplify it to this?

There was a problem checking available updates for Drupal. See the available updates page for more information.

Thus, simply removing the link from the first sentence?

Apologies, it should be this patch.

Sorry for making a mess of this thread. I now understand what owenpm3 was trying to achieve. This patch should implement that. Basically, we don't distinct /update and update/update. We always link to /update. By removing the link to this page from the first part of the sentence, I believe we fixed the issue.

Applied the patch and it solves the issue.
Before patch:
After patch:

Applied the patch and checked code and all looks good to me.

@steverossnyc do you have concerns with marking it RTBC when it goes green?

Uhhh... How am I supposed to test something with the latest version and get an Update message for a newer version? I don't get it ;-) Any clue is welcome!

@djouuuuh it is RTBC now, so no need to test it. Fine to re-test it of course.

Would be comparing:
https://simplytest.me/project/drupal/8.4.x

sith:
https://simplytest.me/project/drupal/8.4.x?patch[]=https://www.drupal.or...

@mgifford Ohhhh you just made my day showing me it was actually possible to apply a patch on a fresh Drupal sandbox with Simplytest.me... I'm so happy I want to cry :-D

Perfect. Happy to help @djouuuuh.

This patch results in different text than previously discussed. Why wasn't this done like shown in #45?

It also removes the "and to install your missing updates" phrase entirely, even when there are missing updates. While I'm not sure that phrase is critically important, I don't see an obvious reason to remove it here either... Again, I think what we need to do in this issue is as discussed in #43 - only modify text for the "there was a problem checking available updates" scenario.

@BarisW proposed it in #58. Simply removing the link from the first sentence resolves the accessibility problem without loosing meaning for the user.

This just takes the last half of the patch in #60, so it will include the "and to install your missing updates" phrase and the logic behind it.

Taking a look as part of Global Sprint Weekend (Boston)...

Following creation of a sandbox site w/ @mgifford 's patch (via simplytest.me), verifying that the first instance of the "available updates" hyperlink has been removed (i.e., navigating to Admin > Reports > Status report). Remaining "available updates" hyperlink resolves as expected.

But the problem with that was already discussed earlier in the issue, wasn't it? It removes the most useful link (admin/reports/updates) and leaves behind the one that makes much less sense in this context (admin/reports/updates/update).

Also while we're already changing the translated string, it would be nice to improve the wording as the other patches did... it's not strictly related to this issue but it's how it was always supposed to be and it avoids more broken translations (and more work for translators) to change it again in the future.

I see that Mike's patch misses an important part of my patch, namely changing the link in the second sentence to the status overview page.

Sorry @BarisW & @David_Rothstein.. Hopefully someone can revise this and we can get this simple patch RTBC'd for a final time.

I'll revise the patch! :)

I see that Mike's patch misses an important part of my patch, namely changing the link in the second sentence to the status overview page.

But then the link will be less useful in the case where there actually are new updates to install.

I really think this issue should go back to the earlier way it was doing it.

Verified on 8.4.x-dev branch without applying the latest patch on it. The issue is already fixed on 8.4.x-dev branch.
- Navigate to /admin/reports/status
- Check the 'Drupal Core Update Status'
- The first hyperlink for the "available updates" has been removed.
- Its appearing like this "There is a security update available for your version of Drupal. To ensure the security of your server, you should update immediately! See the available updates page for more information and to install your missing updates."
Screenshots attached for reference.

@tameeshb please re-assign if you have time to work on a new patch.

In opposition to #77 I still can see the issue on 8.4.x branch

So, based on #70 I´ve created a new patch, hopefully it won´t fail applying.

I have applied the patch from #80 and confirm that it removes confusing link from the first sentence in status reports (screenshots from #80 apply)

This is essentially the same patch as #70 so it has the same problems already mentioned above.

Also, I'm pretty sure Drupal 8 got rid of array() syntax in the interim, so it shouldn't be re-introduced.

We will work on issue at DCM2017

It will take some time to go through the comments, but I think this still counts as a novice task. It might be good for a couple of people to look at together.

Please pay attention to comments from @David_Rothstein.

As a first step, please update the issue summary. The "Proposed Resolution" section should clearly show the existing and proposed markup. At least, then people will know where to look instead of having to hunt though the comments to figure it out. We can always update the summary again if needed.

I am looking at this issue at Baltimore DrupalCon with Jlint in Room 301.

Working on a new patch.

Hey, we've (a11y.matters and edorsini) updated this issue during the DrupalCon Sprint Room 301. First patch attempt!

Moved the link location in the second sentence to the text "missing updates".

See the available updates page for more information and to install your missing updates.

It seems to be more logical to place the hyperlink for /updates/update on the "missing updates" text. It seems to remove the confusion.

I mentioned this to @a11y.matters, @edorsini, and @micheljp that the issue summary needs to be updated per @benjifisher in comment #87.

I reviewed the patch #88. All fine. The solution is elegant as it does not change the text itself, just the link is placed at a much more meaningful place. This is a quick win, so it should be make it into core right away.

1. The screenshots added in #88 should be added or replace the images of the screenshots in the issue summary.
2. I'm not sure if the proposed resolution changes match the expected format of what @benjifisher mentioned in #85. Maybe add the HTML markup changes as well (not the PHP) wrapped in code tags in the issue summary.
3. We should explain why we adding the link to "missing updates" is the more elegant approach. I am not an expert in language so that would be very useful for me.

@mradcliffe, ok I'll update the issue summary now.

I updated the issue summary to make it much more clear what this issue will fix.

Going to ask yoroy to look at the text change here. I wonder whether we should say 'pending updates' instead of 'missing updates'?

Referring to 'available updates' twice, but arriving at two different pages still seems slightly confusing to me. Mentioned in #72, "it would be nice to improve the wording", I've made a minor attempt at that.

Wording after patch in #88

<div class="description">There was a problem checking <a href="/admin/reports/updates">available updates</a> for Drupal. See the available updates page for more information and to install your <a href="/admin/reports/updates/update">missing updates</a>.</div>

Proposed Wording

<div class="description">There was a problem checking <a href="/admin/reports/updates">available updates</a> for Drupal. See the update page for more information and to install <a href="/admin/reports/updates/update">manual updates</a>.</div>

One thing that helps to review the patch is to provide an interdiff.

I have attached an interdiff between 88 to 94 to make it easier to see the changes. This should have also been done for the patch in #88, but I forgot to mention it.

I think the issue is going to be Needs Review until yoroy's review.

Removing Novice tag for now as there isn't anything to do at the moment.

Thanks for the interdiff mradcliffe.

For yoroy's review, a couple other reasons for the suggested word change:
There was a problem checking available updates for Drupal. See the update page for more information and to install manual updates.

• When you click the link 'available updates', the end-user is taken to a page titled Available Updates.
  
• When you click the link 'manual updates', the end-user is taken to a page titled Update.
  

Thanks for working on this. Two different destinations for identically worded links is indeed not good.

"Pending" is better than "Missing": with non-security related updates you might not be missing them, you just have no real reason yet to update. I think we can remove some friction by not explicitly calling out the names of the destination pages.

What about:

There was a problem checking for [available updates]. Review and [install pending updates].

I was going to suggest *Known* pending updates are listed on the updates page. but this sentence is shown without the preceding one as well and then it makes less sense to make "known" explicit. You link to another page because indeed there's more relevant information to be found there, probably not necessary to explicitly state that.

Verified and tested on 8.4.x-dev, checked with the latest patch change-links-2545520-94.patch in #101 comment.
Here are the test observations:
Before applying the patch:
- The message shows for available updates on /admin/reports/status
"There is a security update available for your version of Drupal. To ensure the security of your server, you should update immediately! See the available updates page for more information and to install missing updates."
- The hyperlink present only on "Available updates"

After applying patch in #101:
- The message appears as:
"There is a security update available for your version of Drupal. To ensure the security of your server, you should update immediately! See the update page for more information and to install manual updates."
- The hyperlink present only on "Manual updates"
- When user click the link 'manual updates', the end-user is taken to a page titled Update.

Screenshots attached

@NikitaJain thanks for testing and the screenshots. I would like some feedback on my comments in #107 first though, so setting back to needs review for that.

@Yoroy, I agree to your suggestion - it's short, actionable and clear. I'm gonna create a patch with this wording.

"There was a problem checking for [available updates]. Review and [install pending updates]."

Created a patch for comment #106 and #110.

Before

After

It looks like a testbot failure, now resolved. Back to NR.

There was a problem with another part of Drupal with "PHP 5.6 & MySQL 5.5 21,922 pass" which was not related to the patch #111. I retested and now it works - all green.

I updated the issue summary so the attached images in #110 appear there instead of older screenshots, and updated the resolution to reflect the latest approved changes.

Also, since these are string/localization changes, we should not back port to 8.3.x. Translators on localize.drupal.org will need to update their translations for 8.4.x.

Fixed link urls for images. Whoops.

-        $text = t('There was a problem checking <a href=":update-report">available updates</a> for Drupal.', [':update-report' => \Drupal::url('update.status')], ['langcode' => $langcode]);
+        $text = t('There was a problem checking <a href=":update-report">available updates</a>.', [':update-report' => \Drupal::url('update.status')], ['langcode' => $langcode]);
       }
       else {
         $text = t('There was a problem checking <a href=":update-report">available updates</a> for your modules or themes.', [':update-report' => \Drupal::url('update.status')], ['langcode' => $langcode]);

Why is the first sentence being shortened but not the second? That makes things inconsistent.

-      $requirement['description'][] = ['#prefix' => ' ', '#markup' => t('See the <a href=":available_updates">available updates</a> page for more information and to install your missing updates.', [':available_updates' => \Drupal::url('update.report_update')])];
+      $requirement['description'][] = ['#prefix' => ' ', '#markup' => t('Review and <a href=":available_updates">install pending updates</a>.', [':available_updates' => \Drupal::url('update.report_update')])];

By wordsmithing this, you wind up with a fix that is harder to backport (due to broken translations, etc).

Instead of wordsmithing it, why not just remove it (as previously discussed above)?

Again, (1) the text is simply wrong under the conditions being discussed in this issue, (2) it's equally wrong before and after the above text changes (there are no missing/pending updates of the given type to install), and (3) it was never supposed to display in this situation in the first place (doing so was an accidental change introduced in Drupal 7).

Apologies, we should definitely go for the simplest possible fix here. I was going from what's outlined in the issue summary, which does not mention what's *wrong* with the current text. Looks like the issue summary needs an update then.

#118 +1, I agree. Every user has a different approach for this and a different understanding of this.

Reviewing in the UX meeting, we did not have time to read the 119 preceding comments, sorry. So this may be repeating points.

1. +++ b/core/modules/update/update.install
   @@ -113,7 +113,7 @@ function _update_requirement_check($project, $type) {
   +      $requirement['description'][] = ['#prefix' => ' ', '#markup' => t('Review and <a href=":available_updates">install pending updates</a>.', [':available_updates' => \Drupal::url('update.report_update')])];
   

   I don't think you can *install* the updates, so saying in the link you can is definitely misleading.

2. +++ b/core/modules/update/update.install
   @@ -113,7 +113,7 @@ 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' => \Drupal::url('update.status')])];
   

   Would this need to be updated as well?

3. +++ b/core/modules/update/update.module
   @@ -550,7 +550,7 @@ function _update_message_text($msg_type, $msg_reason, $langcode = NULL) {
   -        $text = t('There was a problem checking <a href=":update-report">available updates</a> for Drupal.', [':update-report' => \Drupal::url('update.status')], ['langcode' => $langcode]);
   +        $text = t('There was a problem checking <a href=":update-report">available updates</a>.', [':update-report' => \Drupal::url('update.status')], ['langcode' => $langcode]);
   

   So "Drupal" clarified what type of update check was a problem (see the module and theme case below). By removing this part, the sentence is simpler, but the clarification is missing.

To hopefully wrap this issue up, here's a patch that goes back to the earlier-discussed approach of just removing the second sentence (the one that contains the duplicate link) when it serves no purpose being there.

There are a number of possible scenarios where the duplicate links were appearing, so here are screenshots of how all of them behave with the new patch:

1. UPDATE_UNKNOWN error
   (occurs during scenarios such as lack of data for this release from update.drupal.org)

   The Status Report now looks like this:

   

   The "available updates" link goes to the Available Updates page, which makes sense:

   

   The link that was removed went to the Update Manager page, which is not useful here anyway:

   

2. UPDATE_NOT_FETCHED error
   (occurs during scenarios such as a failure to contact update.drupal.org)

   The Status Report now looks like this:

   

   The "available updates" link goes to the Available Updates page, which makes sense:

   

   The link that was removed went to the Update Manager page, which is not useful here anyway:

   

3. UPDATE_NOT_CHECKED error
   (occurs in the case of a dev release with no datestamp; in real life, this is mostly encountered when using a Git checkout of Drupal core; I am not sure if it's even realistically possible to see it with a contrib module but I faked it for the screenshots below)

   The Status Report now looks like this:

   

   The "available updates" link goes to the Available Updates page, which makes sense and provides additional clarification:

   

   The link that was removed went to the Update Manager page, which in this case does offer an available "update" (so in theory could be useful), but it's very likely to be a downgrade in practice, so I don't think directing them here was particularly good anyway:

   

4. UPDATE_FETCH_PENDING error
   (occurs when the .info[.yml] file has been recently updated and update status consequently needs to be refreshed due to the new/updated version of the module)

   The Status Report now looks like this:

   

   In this case, the mere act of visiting the update pages will perform the necessary update status recheck. Thus when you click through the link to get there, the data will have been updated, and depending on what it finds, maybe there is or isn't an update that needs to be performed. The screnshots below are for a case where everything was, in fact, up to date.

   The "available updates" link in the above screenshot goes to the Available Updates page:

   

   The link that was removed went to the Update Manager page:

   

   Since we don't know what we're going to find until after the link is clicked, the behavior in this patch (going to the Available Updates page) is probably still best since it provides the most detail. (It might be nice if we simply ran the refresh on the Status Report page as well; therefore we'd never even see this status on that page. But that's a separate issue.)

The bottom line is that in all cases above, the new behavior should make more sense than the old behavior, while the duplicate links are also gone.

Meanwhile, in the case where there are actually updates to install, the patch changes nothing, and the second sentence is still appended, because that's the case where it actually does make sense and which it was designed for:

Updated the issue summary to reflect the above patch.

Note that in the patch:

@@ -136,6 +138,7 @@ function _update_requirement_check($project, $type) {
     case UPDATE_UNKNOWN:
     case UPDATE_NOT_CHECKED:
     case UPDATE_NOT_FETCHED:
+    case UPDATE_FETCH_PENDING:
       $requirement_label = isset($project['reason']) ? $project['reason'] : t('Can not determine status');
       $requirement['severity'] = REQUIREMENT_WARNING;
       break;

This part is sort of an independent bugfix, but it's necessary in order for the above screenshots in the UPDATE_FETCH_PENDING case to be at all sensible.

The patch in #122 is deliberately as simple as possible, and it does not break any translations. This means I left out a couple things discussed previously in this issue:

1. I left out any changes to remove "for Drupal" or "for your modules or themes" from the "There was a problem checking available updates" sentence. I think that's still worth considering, and I don't agree with this comment from #120:
   

   So "Drupal" clarified what type of update check was a problem (see the module and theme case below). By removing this part, the sentence is simpler, but the clarification is missing

   because as can be seen from the above screenshots, that clarification is already provided in the adjacent table columns.

   But, that change breaks translations and is not strictly related to this issue, so it's better to have a separate issue for it afterwards.

2. Replacement of "See the available updates page for more information and to install your missing updates" with "Review and install pending updates" (as proposed in #106). I think that's definitely worth considering as improved wording in the case where there are actually are pending updates such that this sentence needs to be displayed - but again, that should be a separate issue.

@yoroy added the "Needs issue summary update" tag in #118:

I was going from what's outlined in the issue summary, which does not mention what's *wrong* with the current text.

@David_Rothstein updated the summary (and the title) in #123, with a further tweak in #125. The summary now explains the problem, so I think we can remove the tag.

I am adding the "Needs followup" tag based on the two comments in #124.

To help with the review and to make it clear that this patch really is "as simple as possible", I have attached a do-not-test patch:

1. I rewrapped some comment lines to make it clear what had been added.
2. I ignored indentation changes, creating the patch with git diff -w -U4 8.5.x

IMO the patch (from #122) is RTBC, but the issue summary says "Waiting on usability review from yoroy", so I will hold off.

I created a single follow-up issue for the points mentioned in #124: #2919559: Simplify text describing updates in the site status report. I am removing the tag "Needs followup".

I am marking the patch from #122 RTBC despite the request for a review from @yoroy, who has said that he does not want to be a bottleneck for all Usability issues.

Again, the patch I attached in #126 is only to help with review. It should not be committed.

This test failure looks like the fault of #2919773: Fail to download drupal/coder because github.com/klausi/coder has gone missing or #2919786: drupal/coder in composer.lock broken resulting in broken composer install ... probably those are duplicates.

We committers can use a word diff or whitespace diff with the best of them, so we don't really need a -do-not-test patch for a five-line change. What will actually help us the most is ensuring that the correct patch to apply is attached as the final patch on the issue and displayed correctly in the issue summary. This is especially important in such a long issue where there's a chance of reviewing the wrong patch othewise. RTBC issues also automatically retest that patch, so if it's not there the correct patch will not be retested.

I'm reuploading @David_Rothstein's patch from #122 to address this.

Thanks!

Saving issue credit and adding a contributor who collaborated on an earlier patch with another sprinter.

Alright, I read over #122 and I agree that this change makes sense in all scenarios. We're now adding one message in all scenarios except those covered by those four update statuses, and then a different message in those four scenarios.

Committed to 8.5.x. Since this doesn't actually change strings, but instead just changes the situations under which existing strings are displayed, it's safe to backport, and I think we should do so since it's also an accessibility bugfix. We're in commit freeze right now for 8.4.x for the patch release tomorrow, so leaving RTBC against 8.4.x.

Once this is cherry-picked to 8.4.x we can move it to "Patch (to be ported)" for the D7 issue.

Thanks to everyone who's helped review and test this. I think the contributor list is longer than the patch. :)

The patch failed to apply to 8.5.x, since it has already been committed to that branch. I am not sure why the testbot tried that, since the issue is now set to 8.4.x, but let's try setting it back to RTBC.

And now cherry-picked to 8.4.x as well. Thanks everyone for your tireless review and testing on this issue!

Setting "Patch (to be ported)" for D7. A separate child issue should be created for the change in D7, and then this can be marked back to fixed in 8.4.x once the issue is referenced.

@xjm, thanks for the instructions. I did not know that the policy was to create a child issue for the D7 port.

I created the child issue #2920487: The same link "available updates" links to two different pages in the same paragraph when there is a problem checking available updates. Is it a problem that the two issues have the same title? I am not sure if someone else should review that or if I should change the version and status of this issue myself. I guess I will do that in a few days unless someone else does it first.

Oops, forgot to set the version back when changing from "Patch to be ported" to "Fixed".