Update: closed in favor of #332796: Add permissions to the update.module to hide warnings

Problem/Motivation

Currently the update notification ("Update available") is shown to all users with the permision "administer site configuration", while there is already a permission "administer software updates" which is not used for that.

This leads to the issue that users who are not allowed to update see this message. In many cases, this are customers who are then affraid as they don't understand what they can (not) do and what are the consequences.

There are already two different approaches for this problem:
#332796: Add permissions to the update.module to hide warnings
#2059375: Update notification messages as an option in settings

Both are ready for review. This is the third one and most lightweight. I'd like to suggest to simplify things and only show the message to people who are able to act upon that. Technically that means, the message should be shown to users with the permission "administer software updates" and no more to the more general "administer site configuration" who can see this in the status report.

Steps to reproduce

Use a Drupal project with available updates and "administer site configuration" permission, but without "administer software updates" permission and be confused ;)

Proposed resolution

No need for additional permissions or configuration, which complicates things and can lead to logical problems like eventhough the permission is set, also requires the "administer site configuration" to see the message and "administer software updates" to run the updates... too complex solutions for a quite simple problem.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Credits

Huge THANKS to @voleger who did a great job in the both other issues to create the patches!

Alternative approaches

There are three different approaches to this problem, to be discussed which one to use finally. Afterwards both others should be closed as duplicate:
#332796: Add permissions to the update.module to hide warnings
#2059375: Update notification messages as an option in settings
#3239970: Show update notifications only with permission "administer software updates" (THIS)

Issue fork drupal-3239970

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Anybody created an issue. See original summary.

Anybody’s picture

Anybody
Primary language German
Location Porta Westfalica
CreditAttribution: Anybody at DROWL.de commented
Issue summary: View changes

Anybody opened merge request !1276

Anybody’s picture

Anybody
Primary language German
Location Porta Westfalica
CreditAttribution: Anybody at DROWL.de commented
Issue summary: View changes
dww’s picture

dww
we/he/they
CreditAttribution: dww at TEN7 commented
Status: Active » Needs review

Thanks for helping to move this forward!

I think this is my favorite solution to the problem this family of issues aims to solve. This needs review. Hope to be able to do so in the next few days.

bkosborne’s picture

bkosborne
Primary language English
Location New Jersey, USA
CreditAttribution: bkosborne commented

I'm happy with any of the approaches. This one seems to be the easiest. Thank you for moving it forward.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for GOLEMS GABB, Drupal Ukraine Community commented
Status: Needs review » Reviewed & tested by the community

Yes, this one is the easiest approach from the three proposed solutions.
Tested manually on Drupal 9.2.4 installation and it works as expected.

Anybody’s picture

Anybody
Primary language German
Location Porta Westfalica
CreditAttribution: Anybody at DROWL.de commented

Thank you very much! Looking forward to @dww's final review and the next steps into Core! Whao! ;D

Huge thanks (and credits) to @voleger who did a lot of work in the related issues!

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
CreditAttribution: benjifisher at FRUITION commented
Status: Reviewed & tested by the community » Needs review

Usability review

We discussed this issue at #3239070: Drupal Usability Meeting 2021-10-01. That issue has a link to a recording of the meeting.

I apologize for not commenting sooner. I see that there has been activity on this issue since that meeting.

We also reviewed the related issues listed in the issue description. The usability group did not have a strong consensus, but a majority of us preferred the solution in #332796: Add permissions to the update.module to hide warnings. I left a similar comment there.

Drupal has to accommodate different use cases. Consider

  • A small site with an on-call administrator (superuser) and a few content editors
  • A large site that is actively maintained and has many content editors

Both sites will give the administrators, not the content editors, permission to administer updates. The first site will want the content editors to see the warnings, so that they can tell the part-time administrator that the site needs an update. The second site will not want to distract its content editors with these warnings, and certainly does not want all of them messaging the administrator when an update becomes available.

Adding a new permission, as in #332796, lets each site show the warnings to the appropriate users. Making the warnings depend on the existing permission does not give such flexibility.

I recommend closing this issue as a duplicate, but for now I will just bump it back to NR. As I said, the usability group did not have a strong consensus, so I want to encourage further discussion of the options.

Anybody’s picture

Anybody
Primary language German
Location Porta Westfalica
CreditAttribution: Anybody at DROWL.de commented

Thank you very much @benjifisher. I'm OK with that decision! :)

We also discussed

Drupal has to accommodate different use cases. Consider

A small site with an on-call administrator (superuser) and a few content editors
A large site that is actively maintained and has many content editors

internally before creating the issue and I think it's these points vs. "Permission clutter".

Is there anyone else from the core team to review this or #332796: Add permissions to the update.module to hide warnings concept is ready for RTBC with your reply? If further feedback from core maintainers is required, I'd leave this option open for that. If we can RTBC #332796: Add permissions to the update.module to hide warnings please close this as duplicate.

thomas.frobieter’s picture

thomas.frobieter
Primary language German
Location EU
CreditAttribution: thomas.frobieter commented

+1 for #332796 sounds like a perfect solution to me, addressing all needs.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Anybody’s picture

Anybody
Primary language German
Location Porta Westfalica
CreditAttribution: Anybody at DROWL.de commented
Status: Needs review » Needs work

Needs decision and reroll.

ankithashetty made their first commit to this issue’s fork.

ankithashetty’s picture

ankithashetty
Location Karnataka, India
CreditAttribution: ankithashetty as a volunteer and at Specbee for Drupal India Association commented

Just rebased the existing MR, thanks!

Anybody’s picture

Anybody
Primary language German
Location Porta Westfalica
CreditAttribution: Anybody at DROWL.de commented
Status: Needs work » Needs review

Thank you! :) Then needs decision and review.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for GOLEMS GABB, Drupal Ukraine Community commented
Status: Needs review » Needs work

A message appeared when it was not expected. Needs work

DieterHolvoet’s picture

DieterHolvoet CreditAttribution: DieterHolvoet at Minsky commented
Status: Needs work » Needs review

A message appeared when it was not expected.

Can you be more specific? I just tried it out, and the message did not appear for a user without that permission.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for GOLEMS GABB, Drupal Ukraine Community commented

I mean that there is still some test that requires updates. Updated MR

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for GOLEMS GABB, Drupal Ukraine Community commented

Any chance to review MR?

Anybody’s picture

Anybody
Primary language German
Location Porta Westfalica
CreditAttribution: Anybody at DROWL.de commented
Status: Needs review » Reviewed & tested by the community

Yes, the MR is fine! Thanks a lot once again @voleger, still hoping for this to be accepted based on the discussions above see #10. Still think this is the smartest solution to the problem.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for GOLEMS GABB, Drupal Ukraine Community commented

@Anybody can you change the target branch in the MR? I want to rebase MR based on 9.4.x branch

Anybody’s picture

Anybody
Primary language German
Location Porta Westfalica
CreditAttribution: Anybody at DROWL.de commented

@voleger, sorry no, didn't use that yet.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
CreditAttribution: voleger as a volunteer and at GOLEMS GABB, Drupal Ukraine Community for GOLEMS GABB, Drupal Ukraine Community commented

Please follow the edit page link (https://git.drupalcode.org/project/drupal/-/merge_requests/1276/edit), and at the top of the form select 9.4.x branch in the branch select element.

dww’s picture

dww
we/he/they
CreditAttribution: dww at TEN7 commented
Issue tags: +Needs product manager review

I still haven't closely reviewed this, so I'm not sure if it's RTBC or not. 😉 As a subsystem maintainer for update.module, I personally prefer this approach over #332796: Add permissions to the update.module to hide warnings, but I'm on the fence. However, the UX team rightfully is concerned that you couldn't maintain the current behavior of warning a set of users who can't do anything about it. I believe we need a Drupal product manager to sort this out and decide which approach we should take. Probably for minimum disruption and maximum flexibility, we should close this and focus on #332796 instead. TBD.

Thanks!
-Derek

dww’s picture

dww
we/he/they
CreditAttribution: dww at TEN7 commented
Issue summary: View changes
Status: Reviewed & tested by the community » Closed (won't fix)
Issue tags: -Needs product manager review
Related issues: +#332796: Add permissions to the update.module to hide warnings

Sorry for the delays on this! Upon further discussion / reflection, I've changed my mind and agree that #332796: Add permissions to the update.module to hide warnings is indeed the right solution for this problem. Huge apologies to folks who worked on trying to make this happen in this issue. Thankfully, all of you who contributed patches here have already participated meaningfully at #332796, and you're all going to be credited with fixing that issue (once it lands). So there's no credit to transfer. Closing this one as "won't fix" so we can focus on #332796.

Thanks!
-Derek