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

Moderation Pipeline will never run external phases when PREMOD is set #4630

Closed
bastiankistner opened this issue Jun 5, 2024 · 5 comments
Closed
Labels

Comments

@bastiankistner
Copy link
Contributor

Expected behavior:
External moderation phases are not skipped when pre-moderation phase is enabled.

Actual behavior:
When pre-moderation is enabled in admin settings, external moderation phases will never run as the statusPreModerate.ts will set a status and therefore the pipeline will abort early.


I wonder if it really makes sense to run the statusPreModerate phase before external phases. When premod is disabled and an external phase is unavailable (e.g. due to network issues or errors), this could potentially result in every comment being published immediately.

However, if external phases would run before the premod phase, they would have the opportunity to set a status and if not, the premod phase would still ensure that moderation is required.

The only necessary change would be to change the order of the very last phases. Instead of running external phases last, they should run before any phase that returns a status.

@bastiankistner
Copy link
Contributor Author

I've prepared a draft PR that contains a suggestion for the pipeilne order: https://github.com/coralproject/talk/pull/4631/files

@losowsky
Copy link
Member

losowsky commented Jun 6, 2024

Thanks! We'll take a look at this soon.

@losowsky
Copy link
Member

losowsky commented Jul 2, 2024

Just to let you know, this is still in our backlog to look at. More soon!

@nick-funk
Copy link
Contributor

I've taken a look into this and I think it would be safe to move the external mod phases to run prior to the pre-mod phases. I've opened a PR here: #4641

We still have to vet it with QA testing, but I think we can likely make this happen.

@losowsky
Copy link
Member

losowsky commented Aug 7, 2024

PR has now been merged and released: #4641

@losowsky losowsky closed this as completed Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants