Problem/Motivation

Non admins cant see the list of feedback as the access check is using a non existing permission.

Steps to reproduce

Create a user associated with a role which has all Feedback permissions.
User can t access /admin/content/feedback_message

Proposed resolution

Add the 'view feedback message entities' to feedback.permissions.yml

CommentFileSizeAuthor
#10 3420616-10.patch717 bytesisampo

Issue fork feedback-3420616

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

Comments

quadbyte created an issue. See original summary.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

The permission is already present in the feedback.permissions.yml file.

view unpublished feedback message entities:
  title: 'View unpublished Feedback message entities'
quadbyte’s picture

quadbyte commented
Issue summary: View changes
quadbyte’s picture

quadbyte commented

Sorry I had wrong copy/paste in my initial post
The missing one from permissions yml : "view feedback message entities"
And it is in use in : FeedbackMessageHtmlRouteProvider

yanniboi’s picture

yanniboi commented

I can confirm this issue and I would suggest using the "administer feedback message entities" permission for the collection page.

yanniboi opened merge request !2

yanniboi’s picture

yanniboi commented
Status: Active » Needs review

I've created a PR to fix this. :)

yanniboi’s picture

yanniboi commented
Title: Module is using a non existing permission » Feedback collection route is using non existent permission
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Version: 3.0.0-alpha4 » 3.x-dev
isampo’s picture

isampo commented
Status: Needs review » Needs work
StatusFileSize
new 3420616-10.patch717 bytes

Accessing the listing using the `administer feedback message entities` permission works with the latest MR and is better already as now the collection can be accessed. Attached the change as a static patch for composer projects.

Anyhow it might need to be re-thought though as the current description for the permission is "Allow to access the administration form to configure Feedback message entities.", which kind of implies that is meant to be much deeper permission, and it's also defined as `admin_permission` in the FeedbackMessage entity.

A new `access feedback overview` permission could make sense, as that's the way the collection route permissions are usually defined. For example Taxonomy has collection_permission = "access taxonomy overview" in its Entity definition.

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

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented

While writing tests in #3480990: Add automated tests, I ran into this problem as well.

I daresay calling it access feedback message list would be even more clear.

FWIW, this wasn't proposed in the patch; but it also wasn't explicitly stated in any comments: I think a new permission to access the list of feedback message is better than replacing the administer feedback message entities permission, because administer feedback message entities is used in \Drupal\feedback\FeedbackMessageAccessControlHandler::checkFieldAccess() to control field access - and it is plausible for a site admin to want some users to see a list of feedback messages without letting them modify which fields are on feedback messages.

mparker17’s picture

mparker17
he/him
Primary language English
Location UTC-5
commented
Status: Needs work » Fixed

This looks good to me now. I've merged it. Thanks everyone!

  • mparker17 committed c74becdb on 3.x authored by yanniboi 
    Issue #3420616 by yanniboi, mparker17, isampo: Feedback collection route...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.