Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add custom capabilities for story post type #3507

Merged
merged 10 commits into from
Aug 7, 2020
Merged

Conversation

swissspidy
Copy link
Collaborator

Summary

Adds custom capabilities for story post type to allow for more granular control.

Relevant Technical Choices

  • Adds capabilities to default user roles upon plugin activation and during database upgrade.

To-do

User-facing changes

N/A

Testing Instructions


Fixes #3262

@swissspidy swissspidy added Type: Enhancement New feature or improvement of an existing feature Group: WordPress Changes related to WordPress or Gutenberg integration Pod: WP & Infra labels Jul 28, 2020
@google-cla google-cla bot added the cla: yes label Jul 28, 2020
@github-actions

This comment has been minimized.

@codecov
Copy link

codecov bot commented Jul 28, 2020

Codecov Report

Merging #3507 into main will decrease coverage by 0.29%.
The diff coverage is 93.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3507      +/-   ##
==========================================
- Coverage   83.53%   83.24%   -0.30%     
==========================================
  Files         778      778              
  Lines       13459    13487      +28     
==========================================
- Hits        11243    11227      -16     
- Misses       2216     2260      +44     
Flag Coverage Δ
#karmatests 67.50% <ø> (+0.02%) ⬆️
#unittests 66.81% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
includes/REST_API/Fonts_Controller.php 45.97% <0.00%> (ø)
includes/Story_Post_Type.php 79.53% <96.00%> (+2.16%) ⬆️
includes/Dashboard.php 23.58% <100.00%> (-35.85%) ⬇️
includes/Database_Upgrader.php 93.67% <100.00%> (+0.24%) ⬆️
includes/REST_API/Embed_Controller.php 82.02% <100.00%> (ø)
includes/REST_API/Link_Controller.php 93.42% <100.00%> (ø)
includes/Traits/Assets.php 72.41% <0.00%> (-27.59%) ⬇️
...ibrary/panes/media/common/paginatedMediaGallery.js 88.63% <0.00%> (+2.27%) ⬆️
...s/src/edit-story/elements/media/useAverageColor.js 92.85% <0.00%> (+14.28%) ⬆️

Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the places that reference edit_posts will also need to be changed. see https://github.com/google/web-stories-wp/search?q=edit_posts&type=

@swissspidy
Copy link
Collaborator Author

@spacedmonkey Good point regarding updating occurrences of edit_posts 👍 Will add that.

Besides that, I think I should add some E2E tests here to verify the behavior for the various user roles... WDYT?

@spacedmonkey
Copy link
Contributor

Besides that, I think I should add some E2E tests here to verify the behavior for the various user roles... WDYT?

Should all tests run in different user roles like how gutenberg e2e works? Some tests could be skip for some roles.

@swissspidy
Copy link
Collaborator Author

Should all tests run in different user roles like how gutenberg e2e works? Some tests could be skip for some roles.

That's no longer accurate. Gutenberg used to run all tests twice, once for authors, once for admins. No other roles. However, that got broken once, and author tests were removed entirely in WordPress/gutenberg#23588. They want to have more targeted author tests in the future.

There's no point in running the whole test suite N times for N roles, and then skipping the majority of tests because the role doesn't have many capabilities anyway.

More targeted tests are more useful, tests/e2e/specs/editor/authorUser.js is an example of that.

Similarly, we could have a contributorUser.js test that verifies that users cannot access most of the plugin. (Just thinking out loud here)

NB, one cannot easily programmatically skip tests with Jest anyway, see jestjs/jest#7245

@spacedmonkey
Copy link
Contributor

So the last two things,

  • adding on action when adding roles.
  • Change references of edit_posts

Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice an issue.

includes/namespace.php Outdated Show resolved Hide resolved
includes/namespace.php Outdated Show resolved Hide resolved
swissspidy and others added 2 commits August 7, 2020 11:49
Co-authored-by: Jonny Harris <spacedmonkey@users.noreply.github.com>
Copy link
Contributor

@spacedmonkey spacedmonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@swissspidy swissspidy merged commit 590d776 into main Aug 7, 2020
@swissspidy swissspidy deleted the add/3262-story-caps branch August 7, 2020 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Group: WordPress Changes related to WordPress or Gutenberg integration Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

More fine-grained capabilities for stories
2 participants