Problem/Motivation

The descriptions on the permissions in Content Moderation don't really add much, apart from visual clutter.

Proposed resolution

Remove the descriptions.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#14 2890380-14.patch847 bytestimmillwood
#12 2890380-12.patch841 bytesamateescu
#9 2890380-9.patch955 bytestimmillwood
#9 interdiff-2890380-9.txt597 bytestimmillwood
#2 2890380-2.patch882 bytestimmillwood
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

timmillwood created an issue. See original summary.

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented
Assigned: timmillwood » Unassigned
Status: Active » Needs review
FileSize
2890380-2.patch882 bytes

Initial patch

acrosman’s picture

acrosman
Primary language English
Location South Carolina, US
CreditAttribution: acrosman at Cyberwoven commented

I often find it helpful when permissions have the dependency relationships stated (e.g. 'Also requires "View any unpublished content" permission)').

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dixon_’s picture

dixon_
he/him
Primary language English
Location France
CreditAttribution: dixon_ commented
Status: Needs review » Reviewed & tested by the community

Looks good.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Needs review

I'm fine with the current direction of the patch, but should we discuss #3?

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented

Displaying permission dependencies isn't something we do anywhere else in core, but we could leave it there if there's agreement.

acrosman’s picture

acrosman
Primary language English
Location South Carolina, US
CreditAttribution: acrosman at Cyberwoven commented

I can't really think of other places in core where one permissions requires another (although I don't have them memorized by a long stretch). My instinct is to say maybe that should be fixed another way, but I don't want to open a can of worms on a simple issue.

I guess my question is how else is someone new to Content Moderation supposed to figure out why setting "view latest version" didn't actually do anything?

If I'm the only one who feels this way please feel free to move on without me.

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented
FileSize
interdiff-2890380-9.txt597 bytes
2890380-9.patch955 bytes

I think it's fine to leave the requirement in.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Usability

The updated patch looks good to me.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Needs work

Hm, needs a reroll though.

amateescu’s picture

amateescu CreditAttribution: amateescu for Pfizer, Inc. commented
Status: Needs work » Reviewed & tested by the community
FileSize
2890380-12.patch841 bytes

Rerolled for #2900320: Remove workflow type checkWorkflowAcess() & "view content moderation" permission, which removed one of the permissions in that file.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Needs work

Needs another re-roll

timmillwood’s picture

timmillwood
Primary language English
Location 🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK
CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented
Status: Needs work » Needs review
FileSize
2890380-14.patch847 bytes

Here's another re-roll.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Needs review » Reviewed & tested by the community

Thanks @timmillwood.

  • catch committed d64ec77 on 8.5.x 
    Issue #2890380 by timmillwood, amateescu, acrosman: Remove descriptions...

  • catch committed 18d0c49 on 8.4.x 
    Issue #2890380 by timmillwood, amateescu, acrosman: Remove descriptions...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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