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

[CORL-3120]: Add initialStatus to comment and use for pre-mod labels in moderate cards #4589

Merged
merged 7 commits into from
Apr 3, 2024

Conversation

kabeaty
Copy link
Contributor

@kabeaty kabeaty commented Mar 29, 2024

What does this PR do?

These changes add an initialStatus field to Comment. This helps have a clearer history from the initial creation of the comment into what its status history is. A status field is added to comment revisions, so we also have status per revision now. Then we use the first revision's status to populate initialStatus.

These changes will impact:

  • commenters
  • moderators
  • admins
  • developers

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

This adds an initialStatus field to the Comment. It also adds status to CommentRevision.

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?

You can set a user to pre-mod. Create comments as that user. See that they have the pre-mod label in moderate cards, in moderation queues and in user history drawer. Approve/reject these comments. See that they still have the pre-mod label based on their initial pre-mod status.

Were any tests migrated to React Testing Library?

How do we deploy this PR?

Copy link

netlify bot commented Mar 29, 2024

Deploy Preview for gallant-galileo-14878c canceled.

Name Link
🔨 Latest commit e137d0b
🔍 Latest deploy log https://app.netlify.com/sites/gallant-galileo-14878c/deploys/660c49ebfe13140008a28d48

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.

Curious what your thoughts are with respect to storing this in the CommentRevision instead of an initialStatus field on the comment.

@@ -42,6 +42,13 @@ const markers: Array<
</Localized>
)) ||
null,
(c) =>
(c.status !== "PREMOD" && c.initialStatus === "PREMOD" && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have string type enums that could be imported here instead of using raw string comparisons?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, updated to use that in all places status is referenced in this component...thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

awesome, thank you!

/**
* initialStatus is the initial Comment Status.
*/
initialStatus: GQLCOMMENT_STATUS;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts on storing/exposing this in the comment revisions? We follow a history based structure everywhere else in the server, might be cool to have the status per-revision?

Copy link
Contributor Author

@kabeaty kabeaty Apr 2, 2024

Choose a reason for hiding this comment

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

Yeah, I like this idea so that we have access to it for each revision this way. Updated to then use the first revision status to populate initialStatus for comments then.

Copy link
Contributor

Choose a reason for hiding this comment

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

amazing, thank you!

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.

I'm approving because technically status is also a persisted computed state that is calculated from the mod actions... so this intialStatus makes sense in that regard too...

Coral is so weird... I'll leave it up to you which style you prefer cause Coral seems to do both...

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.

Updates look great, thank you once again! Approved!

@tessalt tessalt added this to the 9.0.2 milestone Apr 3, 2024
@tessalt tessalt added this pull request to the merge queue Apr 3, 2024
Merged via the queue into develop with commit 15e8416 Apr 3, 2024
6 checks passed
@tessalt tessalt deleted the fix/CORL-3120-premod-labels-user-drawer branch April 3, 2024 16:18
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.

None yet

3 participants