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

DEV: Normalize reviewable actions #122

Merged
merged 2 commits into from Feb 20, 2024
Merged

Conversation

Drenmi
Copy link
Contributor

@Drenmi Drenmi commented Feb 19, 2024

What is this change?

Several pieces of feedback suggests that the review actions for posts flagged by Akismet are confusing. Since there is conceptually little difference between a post marked as spam by a human and a computer, this PR brings the options more in line with what we have for posts marked as spam in core.

Before

akismet-spam-before

After

akismet-spam-after

@Drenmi Drenmi force-pushed the dev/normalize-reviewable-actions branch from 730328f to b6e7e16 Compare February 19, 2024 06:29
@Drenmi Drenmi marked this pull request as ready for review February 19, 2024 06:39
@@ -4,4 +4,8 @@

class ReviewableAkismetPostSerializer < ReviewableSerializer
payload_attributes :post_cooked, :external_error

def created_from_flag?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only use for this is to determine whether we display the context question:

Is this X Y?

I think it's not a stretch to say that Akismet is flagging posts. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

created_from_flag sounds weird? When are reviewables not created from flagging?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are some settings, IIRC. E.g. where new user first post needs review.

@@ -10,31 +10,24 @@ def self.action_aliases
def build_actions(actions, guardian, _args)
return [] unless pending?

agree =
agree_bundle =
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

nattsw
nattsw previously approved these changes Feb 19, 2024
Copy link
Contributor

@nattsw nattsw left a comment

Choose a reason for hiding this comment

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

Looks ok

It's a bit sad (not your doing) that we didn't create a plugin API on this Akismet plugin itself to extend reviewables. Ideally this plugin should not have knowledge of post voting.

@@ -46,8 +46,11 @@ en:
types:
reviewable_akismet_post:
title: "Akismet Flagged Post"
noun: "post"
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, where is noun used?

Copy link
Contributor Author

@Drenmi Drenmi Feb 19, 2024

Choose a reason for hiding this comment

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

It is used to construct the context question, e.g.:

Is this post spam?

@nattsw nattsw dismissed their stale review February 19, 2024 08:43

Unsure about something

@Drenmi Drenmi requested a review from nattsw February 19, 2024 10:06
@Drenmi Drenmi merged commit cc56c9c into main Feb 20, 2024
3 checks passed
@Drenmi Drenmi deleted the dev/normalize-reviewable-actions branch February 20, 2024 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants