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.

Sign up for GitHub

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

Jump to bottom

[CORL-2948] emit moderation actions #4377

Merged
merged 9 commits into from
Nov 9, 2023
Merged

[CORL-2948] emit moderation actions #4377

merged 9 commits into from
Nov 9, 2023

Conversation

marcushaddon
Copy link
Contributor

@marcushaddon marcushaddon commented Oct 20, 2023 •
edited
Loading

What does this PR do?

This PR updates our comment moderation pipeline to distinguish between commentActions and moderationActions during the pipeline. It causes certain phases to conditionally emit a moderationAction field that sets the status and terminates the pipeline.

These changes will impact:

  • commenters
  • moderators
  • admins
  • developers

What changes to the GraphQL/Database Schema does this PR introduce?

none.

Does this PR introduce any new environment variables or feature flags?

No.

If any indexes were added, were they added to INDEXES.md?

n/a

How do I test this PR?

  1. On both dev, and then on this branch Create the following comments
    • non rejected comments
    • comment reported
    • comment reported, then rejected
    • comment reported, rejected, then approved
    • comment reported then approved
    • comment flagged for suspect word, approved
    • comment flagged for suspect word, rejected
    • comment flagged for suspect word
    • comment flagged for banned word, rejected
    • comment flagged for banned word, rejected, then approved
    • comment rejected in stream by mod
    • comment rejected via reject all comments
    • comment rejected via reject all comments on this site
    • comment rejected via spam ban
    • comment approved via external moderation
    • comment flagged via external moderation
    • comment rejected via external moderation
  2. Confirm that all queues contain the same corresponding comments after creating these on each branch.

Where any tests migrated to React Testing Library?

No.

How do we deploy this PR?

No special considerations should be needed.

@netlify
Copy link

netlify bot commented Oct 20, 2023 •
edited
Loading

✅ Deploy Preview for gallant-galileo-14878c canceled.

Name Link
🔨 Latest commit 6e9bb23
🔍 Latest deploy log https://app.netlify.com/sites/gallant-galileo-14878c/deploys/654be28b3a621f00089c649f

@marcushaddon marcushaddon marked this pull request as ready for review October 24, 2023 23:14
marcushaddon
marcushaddon commented Nov 7, 2023
View reviewed changes
server/src/core/server/services/comments/pipeline/helpers.ts Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aggregating commentActions instead of actions. moderationAction is a single object rather than an array, and isn't aggregated because Moderation Actions set a status on the comment, and so short circuit the pipeline.

marcushaddon
marcushaddon commented Nov 7, 2023
View reviewed changes
server/src/core/server/services/comments/pipeline/phases/detectLinks.ts Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renaming field

marcushaddon
marcushaddon commented Nov 7, 2023
View reviewed changes
server/src/core/server/services/comments/pipeline/phases/external.ts
Pick<PhaseResult, "actions" | "status" | "tags">
>;
Pick<PhaseResult, "status" | "tags">
> & { actions: PhaseResult["commentActions"] };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is because our public API for external moderation phases still accepts the actions field, but their type is now internally called commentActions

nick-funk
nick-funk approved these changes Nov 7, 2023
View reviewed changes
Copy link
Contributor

@nick-funk nick-funk left a comment

Choose a reason for hiding this comment

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

This make sense and thanks for adding in the new mod actions distinction! Solid refactor!

@marcushaddon
remove comment
6e9bb23 
also flag comment before rejecting for banned word
kabeaty
kabeaty reviewed Nov 8, 2023
View reviewed changes
Copy link
Contributor

@kabeaty kabeaty left a comment

Choose a reason for hiding this comment

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

The refactor looks good overall!

I did notice that when banned words are rejected, their moderation cards don't include the Banned word marker next to Details at the bottom of the card. The MarkersContainer component is checking in actionCounts flag reasons for whether to show that. Is there a way to update so this is still included in the moderation card in this case?

@marcushaddon marcushaddon added the 🚀 merge it! Pull requests that should be merged after status checks pass with a review label Nov 8, 2023
@marcushaddon marcushaddon added this pull request to the merge queue Nov 9, 2023
Merged via the queue into develop with commit b85002d Nov 9, 2023
@marcushaddon marcushaddon deleted the feat/CORL-2948-emit-moderation-actions branch November 9, 2023 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kabeaty kabeaty kabeaty approved these changes

@nick-funk nick-funk nick-funk approved these changes

Assignees
No one assigned
Labels
🚀 merge it! Pull requests that should be merged after status checks pass with a review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@marcushaddon @nick-funk @kabeaty