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

Apply permissions system to comments #8035

Merged
merged 14 commits into from Jul 21, 2021

Conversation

entantoencuanto
Copy link
Contributor

@entantoencuanto entantoencuanto commented May 21, 2021

🎩 What? Why?

This PR extends authorizations permissions system to comments on all commentable resources:

  • For the commentable resources linked to components some of the logic has been extracted to a CommentableWithComponent concern and the comment action has been declared in their resource manifests.
  • There are 2 other commentable resources not linked to components: Decidim::Initiative and Decidim::Consultations::Question. For them:
    • The PR creates the controller to manage permissions from admin for initiatives (on questions already exists)
    • Adds an authorization modal for resources not linked to components
    • Adapts ActionAuthorizationHelper to accept a permissions_holder argument which is used on resources without component and to return the previously defined modal path when component is not present
    • Defines a component method on initiatives and question models which returns a blank response to establish that they are resources not related to any component.
    • Overrides current_participatory_space_manifest used in some controllers of initiatives to return the participatory space manifest instead of a resource manifest
    • Allow definition of permissions_class_name in resources manifests as it is done in the manifests of the participatory spaces manifests.
  • Adds tests

📌 Related Issues

Link your PR to an issue

Testing

From admin visit accountability, blogs, budgets, debates, meetings and sortitions components of a participatory space. The permissions icon should appear and the permissions for comment action should be available in the permissions modal.
The same from initiatives and questions of a consultation indexes.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

📷 Screenshots

Screen Shot 2021-07-08 at 16 18 31

Screen Shot 2021-07-08 at 16 18 46

Screen Shot 2021-07-08 at 16 18 55

Screen Shot 2021-07-08 at 16 19 06

Screen Shot 2021-07-08 at 16 19 21

Screen Shot 2021-07-08 at 16 19 28

♥️ Thank you!

@entantoencuanto entantoencuanto force-pushed the 7726-apply_permissions_system_to_comments branch from 9082c75 to 00c94be Compare May 21, 2021 19:17
@entantoencuanto entantoencuanto force-pushed the 7726-apply_permissions_system_to_comments branch from 00c94be to d35c234 Compare June 4, 2021 19:41
@andreslucena
Copy link
Member

Hi @entantoencuanto
This is already finished? I say because this is a Draft but the linked issue (#7226) is in the "QA Testing" column.
Also, could you please provide a description? For instance, in which components this is implemented? Could you also add some screenshots? Thanks

@entantoencuanto entantoencuanto force-pushed the 7726-apply_permissions_system_to_comments branch from d35c234 to 61f9120 Compare July 1, 2021 16:01
@entantoencuanto entantoencuanto force-pushed the 7726-apply_permissions_system_to_comments branch 5 times, most recently from 92f1350 to d59da0b Compare July 7, 2021 20:52
@entantoencuanto entantoencuanto marked this pull request as ready for review July 8, 2021 11:46
@entantoencuanto entantoencuanto force-pushed the 7726-apply_permissions_system_to_comments branch from e38d823 to 9924e4f Compare July 8, 2021 16:51
@entantoencuanto
Copy link
Contributor Author

Now it's ready, @andreslucena, I've added the 2 remaining resources and the PR description

@andreslucena andreslucena added this to Pending review from Product in Maintainers via automation Jul 15, 2021
@andreslucena andreslucena moved this from Pending review from Product to To Review by Core in Maintainers Jul 20, 2021
@leio10 leio10 added the type: feature PRs or issues that implement a new feature label Jul 21, 2021
Copy link
Contributor

@leio10 leio10 left a comment

Choose a reason for hiding this comment

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

LGTM!

@leio10 leio10 merged commit c8bdbb0 into develop Jul 21, 2021
Maintainers automation moved this from To Review by Core to Done Jul 21, 2021
@leio10 leio10 deleted the 7726-apply_permissions_system_to_comments branch July 21, 2021 09:09
@leio10 leio10 mentioned this pull request Jul 21, 2021
12 tasks
entantoencuanto added a commit that referenced this pull request Jul 26, 2021
* develop: (32 commits)
  Remove obsolete rake webpack task (#8237)
  Active storage migrations service (#7902)
  Fix content type delegation to blank attachments (#8230)
  Evote bug fixing (#8220)
  Fix the proposal data migration for proposals without authors or organization (#8015)
  Bump addressable version because security issues (#8229)
  Online meetings iframe visibility with time (#8097)
  Meetings iframe and iframe URL (#8096)
  Remove flaky test on meetings (#8226)
  Fix broken tests after problematic PRs (#8224)
  Apply permissions system to comments (#8035)
  Set current_component as commentable when commentable is a participatory space (#8189)
  Fix don't require inactive authorization handlers (#8122)
  Improve metrics calculations performance (#8215)
  Fix performance issue in notification settings page (#8155)
  Active storage migration (#7598)
  Update manual installation guide in documentation (#8217)
  Load JS configuration in elections focus mode layout (#8213)
  Fix user activity pagination when there are hidden items (#8202)
  Make it possible to define SCSS settings overrides from modules (#8198)
  ...
roxanaopr pushed a commit to tremend-cofe/decidim that referenced this pull request Jul 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Apply permissions system to comments
3 participants