Skip to content

Add ability to hide content from front page#10024

Merged
ahukkanen merged 2 commits intodecidim:developfrom
i-need-another-coffee:ale/frontend-moderation
Dec 23, 2022
Merged

Add ability to hide content from front page#10024
ahukkanen merged 2 commits intodecidim:developfrom
i-need-another-coffee:ale/frontend-moderation

Conversation

@alecslupu
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu commented Nov 3, 2022 •
edited
Loading

🎩 What? Why?

This PR adds a checkbox in the report form that will allow the admin to directly hide the content of an user.

This PR depends on : #9852

Ref: SPAM04

📌 Related Issues

Link your PR to an issue

Testing

  1. Checkout the branch
  2. Visit any content page
  3. Click on report
  4. Use the checkbox to hide content
  5. Observe behavior

📷 Screenshots

Please add screenshots of the changes you're proposing
image

♥️ Thank you!

@Crashillo
Copy link
Copy Markdown
Contributor

Crashillo commented Nov 7, 2022 •
edited
Loading

@alecslupu keep in mind the changes you made to frontend files (mainly) can be overwritten with the redesign. In this case, flag modal changes will lose, due to the refactor we're doing in #9852

@alecslupu alecslupu force-pushed the ale/frontend-moderation branch from b053cee to 20a391b Compare November 8, 2022 22:30
@alecslupu alecslupu self-assigned this Nov 9, 2022
@alecslupu alecslupu force-pushed the ale/frontend-moderation branch 3 times, most recently from c936da6 to 0c227f3 Compare November 23, 2022 20:54
@alecslupu alecslupu marked this pull request as ready for review November 23, 2022 22:56
@andreslucena andreslucena mentioned this pull request Nov 25, 2022
Merged
@alecslupu alecslupu force-pushed the ale/frontend-moderation branch from 0c227f3 to 5d147bf Compare December 11, 2022 18:39
andreslucena
andreslucena previously requested changes Dec 14, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

I had some general suggestions to make, but after seeing the changes from #9852, I think that we should expedite that PR (as its longer, had more work already and its pretty close to be merge). After that one is merge I think we can retake that, just applying it to the new design.

Just skimming in the code, I think that this will mean a big change (like the current FlagModalCell isn't used anymore, now it's on ReportButtonCell, https://github.com/decidim/decidim/blob/feature/redesign-action-buttons/decidim-core/app/cells/decidim/report_button/flag_modal.erb)

Sorry for not seeing this until its too late 😞

@alecslupu
Copy link
Copy Markdown
Contributor Author

alecslupu commented Dec 14, 2022

I had some general suggestions to make, but after seeing the changes from #9852, I think that we should expedite that PR (as its longer, had more work already and its pretty close to be merge). After that one is merge I think we can retake that, just applying it to the new design.

Just skimming in the code, I think that this will mean a big change (like the current FlagModalCell isn't used anymore, now it's on ReportButtonCell, https://github.com/decidim/decidim/blob/feature/redesign-action-buttons/decidim-core/app/cells/decidim/report_button/flag_modal.erb)

Sorry for not seeing this until its too late disappointed

@andreslucena don't worry. the changes are not that big, and once you merge #9852 i can take this and change it. Just want to make sure that the new Report Button is being used by the old interface as well.

@alecslupu alecslupu force-pushed the ale/frontend-moderation branch from 989b1f6 to 57f1295 Compare December 14, 2022 18:09
@alecslupu alecslupu marked this pull request as draft December 14, 2022 18:10
@alecslupu alecslupu force-pushed the ale/frontend-moderation branch 4 times, most recently from 11e4370 to bfb9f8e Compare December 20, 2022 11:55
@alecslupu alecslupu marked this pull request as ready for review December 20, 2022 12:58
@alecslupu
Copy link
Copy Markdown
Contributor Author

alecslupu commented Dec 20, 2022

@andreslucena #9852 has been merged. I have updated the branch, and can confirm that it works with old design and re design. Currently is waiting your input.

ahukkanen
ahukkanen requested changes Dec 21, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

Works great! Tested this for normal admins, process admins and process moderators and all those roles can see the hide control. Alongside this, also eagerly waiting for #10111. 🎉

Just a question, is it by design that process valuators can also see the hiding controls? I understand that this might be by design but it just seemed a bit weird to me.

Also, added one comment to improve the specs and one comment to improve the code.

@alecslupu alecslupu force-pushed the ale/frontend-moderation branch from bfb9f8e to 7b1c732 Compare December 21, 2022 14:37
@alecslupu
Copy link
Copy Markdown
Contributor Author

alecslupu commented Dec 21, 2022

Just a question, is it by design that process valuators can also see the hiding controls? I understand that this might be by design but it just seemed a bit weird to me.

I have logged in as a valuator on my local environment, and i can successfully see the global moderation section, which is displaying the places where i have access to. for that particular reportable resource i can also use the hide functionality.

If is something to be changed regarding whether a valuator could or could not hide a resource, i would say is not in the scope of this PR.

@decidim/maintainers I think all the required changes have been made, therefore, please review.

@alecslupu alecslupu requested a review from ahukkanen December 21, 2022 14:50
@ahukkanen
Copy link
Copy Markdown
Contributor

ahukkanen commented Dec 22, 2022

I have logged in as a valuator on my local environment, and i can successfully see the global moderation section, which is displaying the places where i have access to. for that particular reportable resource i can also use the hide functionality.

Yes this is exactly what I meant, are valuators supposed to be able to moderate content?

@decidim/product Could you shed some light whether valuators should be able to moderate content within the participatory space?

@carolromero
Copy link
Copy Markdown
Member

carolromero commented Dec 22, 2022

Could you shed some light whether valuators should be able to moderate content within the participatory space?

@ahukkanen I confirm that they shouldn't, and it should be treated as a bug.

@alecslupu
Copy link
Copy Markdown
Contributor Author

alecslupu commented Dec 22, 2022 via email

@ahukkanen
Copy link
Copy Markdown
Contributor

ahukkanen commented Dec 22, 2022

Should it treated within this task? It seems this could be an entire new issue.

I agree, we should report it as a bug as I believe it also applies to the current version.

Just wanted to make sure that this was not intentional.

This was referenced Dec 23, 2022
Closed
Closed
@alecslupu
Copy link
Copy Markdown
Contributor Author

alecslupu commented Dec 23, 2022 •
edited
Loading

Hello
The following issues have been created:
#10186 ( addresses the admin section )
#10187 ( addresses the frontend moderation tools )

I have added them as individual tickets, as they have a different timeline to be fixed.

@ahukkanen ahukkanen mentioned this pull request Dec 23, 2022
Closed
ahukkanen
ahukkanen approved these changes Dec 23, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

👍

@ahukkanen ahukkanen dismissed andreslucena’s stale review December 23, 2022 08:41

It seems the comments were already resolved from this review.

@ahukkanen ahukkanen merged commit 166d602 into decidim:develop Dec 23, 2022
@alecslupu alecslupu mentioned this pull request Mar 20, 2023
Merged
@andreslucena andreslucena mentioned this pull request Mar 23, 2023
Closed
@alecslupu alecslupu deleted the ale/frontend-moderation branch August 3, 2023 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ahukkanen ahukkanen ahukkanen approved these changes

@andreslucena andreslucena Awaiting requested review from andreslucena

+1 more reviewer

@Crashillo Crashillo Crashillo left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@alecslupu alecslupu

Labels

module: core

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add direct action to hide this content

5 participants

@alecslupu @Crashillo @ahukkanen @carolromero @andreslucena