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

Granular level topic gating #9512

Merged
merged 30 commits into from
Oct 17, 2024
Merged

Conversation

mzparacha
Copy link
Contributor

@mzparacha mzparacha commented Oct 10, 2024

Link to Issue

Closes: #9063

Description of Changes

  • Added topic-level permission checks for gated topics
  • Added Migration for existing gated topics to have full topic level permissions i.e UPVOTE_AND_COMMENT_AND_POST
  • Added frontend/API changes to allow community admins to restrict certain actions within a topic
  • Added logic to handle edge cases where multiple groups refer to the same topic with different permission levels
  • Some UI elements were not present in designs, to show relevant data I added some custom UI.

"How We Fixed It"

N/A

Test Plan

General steps

  • pnpm migrate-db
  • Ensure no regressions

Test topic gating with non-group member accounts (i.e test original gating logic, works as expected)

  1. Visit any existing community that has some gated topics
  2. Join that community with a different user having a new address
  3. Make sure you are not part of a gated group in that community
  4. Now try to perform gated actions ("Create thread", "Create comment", "Create thread reaction", "Create comment reaction") in a gated topic and verify you get blocked by Topic is gated messages via a tooltip or on form controls

Test topic gating with existing topics in existing communities with group member accounts (i.e test original gating logic with full topic level permissions, works as expected)

  1. Visit any existing community that has some gated topics
  2. Join that community with an account that can be part of a gated group
  3. Now try to perform gated actions ("Create thread", "Create comment", "Create thread reaction", "Create comment reaction") and verify you are able to perform all gated actions within that gated topic

Test topic gating with new topics in new communities

  1. Create a new community (referring to community admin as ACC-1)
  2. Join that community with a different account as a community member (referring to this as ACC-2)
  3. With ACC-1 create a group, add ACC-2, and gate a topic with some permission level. i.e UPVOTE, UPVOTE_AND_COMMENT or UPVOTE_AND_COMMENT_AND_POST
  4. Now with ACC-2 try to perform gated actions within that topic
  5. If the topic permission level is UPVOTE, verify you are only able to upvote and not able to create a thread or comment.
  6. If the topic permission level is UPVOTE_AND_COMMENT, verify you are only able to upvote and comment on that thread and not able to create a thread.
  7. If the topic permission level is UPVOTE_AND_COMMENT_AND_POST, verify you can upvote, comment, and create a thread on that topic.
  8. For steps 5, 6, and 7, verify you see the Topic members are only allowed to ${topicPermissionLevel} tooltip in the block action CTA element
  9. For steps 5 and 6, verify you see a topic-level permission banner when creating a thread via the thread form in that gated topic.
image
  1. Verify you get the same tooltip message from 5, 6, and 7 on blocked action CTA element on all pages where thread/comment cards are rendered i.e /:communityId/overview, /:communityId/discussions, /:communityId/discussions/:topic, /dashboard/for-you, /dashboard/global.
  2. Verify that you are able to bypass 5, 6, 7, 8, and 9 with a community admin account.

Test permissions precedence with the same topics in different groups having different permission levels

  1. Create a new community (referring to community admin as ACC-1)
  2. Join that community with a different account as a community member (referring to this as ACC-2)
  3. With ACC-1 create a group (referring to this as GROUP-1), add ACC-2, and gate a topic (referring to this as TOPIC-1) with the UPVOTE permission level
  4. With ACC-1 create another group (referring to this as GROUP-2), add ACC-2, and gate TOPIC-1 with the UPVOTE_AND_COMMENT_AND_POST permission level
  5. Now with ACC-2 try to create a thread in TOPIC-1, verify that it works (i.e GROUP-2 permission levels for TOPIC-1 takes precedence over GROUP-1 permissions levels for the same TOPIC-1).
  6. With ACC-2 try to create a comment in the thread just created in TOPIC-1, verify that it works.
  7. With ACC-2 try to react to comments and thread for the thread just created in TOPIC-1, verify that it works.

Test permissions change on gated topics are reflected

  1. Create a group in any community where you are an admin and gate a topic with any permission level
  2. On the community groups /:communityId/members?tab=groups page, verify the group cards show correct permission levels with the gated topics
  3. Update the group and change topic permission levels, and again verify on the community groups page that the correctly updated permission levels are shown with gated topics.
  4. While performing steps 2 and 3, verify the permission levels are reflected with a member account trying to perform gated actions.

Deployment Plan

N/A

Other Considerations

N/A

@mzparacha mzparacha self-assigned this Oct 11, 2024
@mzparacha mzparacha marked this pull request as ready for review October 11, 2024 08:50
Copy link
Contributor

@masvelio masvelio left a comment

Choose a reason for hiding this comment

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

FE code looks good to me. Didn't do proper CA testing though

Copy link
Collaborator

@timolegros timolegros left a comment

Choose a reason for hiding this comment

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

I agree with Roger on this one. I think we can build this without creating a new enum. Additionally, I think a new table/model is also overkill. It seems like the only difference between the existing GroupPermissions and the new GroupTopicPermissions table is an additional topic_id foreign key...

I would also like to get @kurtisassad's eyes on this PR since he is the original author of the more recent granular permissions model.

Copy link
Contributor

@Rotorsoft Rotorsoft left a comment

Choose a reason for hiding this comment

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

LGTM, after addressing the comments

@mzparacha mzparacha merged commit 49aa4a9 into master Oct 17, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enabling granular topic level gating
4 participants