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

3924 mod archive thread #4338

Merged
merged 116 commits into from
Oct 24, 2023
Merged

3924 mod archive thread #4338

merged 116 commits into from
Oct 24, 2023

Conversation

Miaplacidus
Copy link
Contributor

@Miaplacidus Miaplacidus commented Jun 26, 2023

Provide ability for mods/admins to archive threads.

Link to Issue

Closes: #3924

Description of Changes

  • Allows mods/admins to archive threads.
  • Conditionally disables/hides elements based on archive status

Test Plan

  • inspect thread index
  • inspect view thread page

Deployment

Other Considerations

@Miaplacidus Miaplacidus marked this pull request as ready for review June 29, 2023 15:31
@Miaplacidus
Copy link
Contributor Author

  1. Not supposed to be a pointer.
  2. Good question! @jessmart1213 How should archived posts appear on the Overview page (I'm guessing the same way they show up on the discussions pages)? Should they appear at all?
  3. The actions that are blocked by a thread being archived were previously discussed. If anything has changed on design's side, @jessmart1213, let me know.
  4. Keeping the tags even when a thread is archived is intentional.
  5. Same answer as 3.
  6. We keep the comment count, even though you are not allowed to add comments.
  7. We never discussed if all of the options in the cards to the right of the thread on the view thread page(i.e. 'Link Proposal', 'Link Discussion', etc.) should be allowed. @jessmart1213 - any thoughts on this?
    8., 9. @zakhap @jessmart1213 How do we handle a thread initially marked as Spam, which is then archived?
  8. This is specified in the design.
  9. @zakhap I believe we discussed this and it's intentional. However, it is jarring. We can potentially leave the sorting and filtering off of the Archived page
  10. There's no filter being applied. Archive is not a topic.
  11. Please create a bug card for this. Thanks!

@zakhap
Copy link
Collaborator

zakhap commented Oct 20, 2023

  1. Not supposed to be a pointer.

👍

  1. Good question! @jessmart1213 How should archived posts appear on the Overview page (I'm guessing the same way they show up on the discussions pages)? Should they appear at all?

The query for populating the Overview Page should filter out content flagged as either Spam or Archive. This is definitely scope creep and can be created as a fast follow ticket. We should not add new UI here.

  1. Not sure, but should we block the pin option if thread is archived?

Yes! It doesn't make much sense to pin a thread to the archive.

  1. Keeping the tags even when a thread is archived is intentional.

👍 yep

  1. Should we block the subscribe option if thread is archived?

This should also be disabled 👍, like #3 above.

  1. We keep the comment count, even though you are not allowed to add comments.

👍

  1. We never discussed if all of the options in the cards to the right of the thread on the view thread page(i.e. 'Link Proposal', 'Link Discussion', etc.) should be allowed. @jessmart1213 - any thoughts on this?

It is fine for this functionality to stay active. This should be further discussed later within product intentions about what it means for a thread to be archived/locked/etc.

8., 9. @zakhap @jessmart1213 How do we handle a thread initially marked as Spam, which is then archived?

It should be marked as both.

  1. This is specified in the design.

👍 correct

I will respond to 9-14 shortly

@zakhap
Copy link
Collaborator

zakhap commented Oct 20, 2023

  1. The archived thread, which is also marked as spam, doesn't show up in the "Archived" section

I don't find this behavior to be particularly jarring. The only weird case is on the normal listing page that the user needs to click both spam+archive to see something tagged as both, but I don't think that's a problem. On the archive page, you can simply click "include spam" and it'll show up there.

  1. Do we need the "include posts flagged as spam" checkbox in the archived section?

Yes! It is fine as is here.

  1. When we apply any filter from the archived page, it redirects to the discussion page

Def a bug! We should be able to filter for archived threads without seeing all the threads in the topic as well.

  1. I think we should show this message in the archived page if there are no archived threads (we do a similar thing on the discussion page)

I agree. @Miaplacidus this is the only new ask here. Empty state is really important here as all communities will start with an empty Archive page and it will be a new button in their sidebar when the feature is launched.

  1. Unrelated to this PR: the "trending" tag doesn't showup for a thread in the details page

This is a separate bug and I will make a bug ticket for it now!

@Miaplacidus
Copy link
Contributor Author

In regards to the bug, @kurtisassad and I have been working to fix it, but it's extremely flakey and appears only under unusual circumstances. Namely, 1. you archive a number of threads in one community 2. you archive yet more threads in another community 3. maybe you see it or maybe you don't. @kurtisassad can't reproduce it at all. In addition, I saw this bug with the spam feature as well under the same circumstances. I'll create a bug ticket around this, but, for now, we're going to disregard the bug.

@Miaplacidus
Copy link
Contributor Author

@zakhap
So, in regards to pinned threads, I have a question/suggestion.
If a thread is currently pinned and an admin decides to archive it, I think we should remove the status of 'pinned' from it. Of course, this means that the admin will have to re-pin the thread, but I think that is the correct level of seriousness archiving should have. It's basically a "dead" thread once it's archived, so I don't think that's inappropriate.

Please let me know if you have any thoughts or objections! Thanks!

@Miaplacidus
Copy link
Contributor Author

@zakhap Also, we do have a message for when there are no archived threads. It is "There are currently no archived threads." I think there is a good argument for making that more generic, since it's not entirely accurate when you are filtering/sorting by newest, stage, etc. Something more like "There are no archived threads matching your filter." would fit all situations.

@Miaplacidus
Copy link
Contributor Author

Miaplacidus commented Oct 23, 2023

Per discussion with @zakhap, we've removed the status of 'pinned' from a thread when it is archived.

@Miaplacidus
Copy link
Contributor Author

@mzparacha
I've responded to and/or implemented your suggestions above; please note the responses from @zakhap and that some of your suggestions have been made into cards.
Please take a second look. If the new work is acceptable, please approve the PR. Thanks!

Copy link
Contributor

@mzparacha mzparacha left a comment

Choose a reason for hiding this comment

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

This looks great. Thanks for all the changes @Miaplacidus 🙏.

@Miaplacidus Miaplacidus merged commit 4349019 into master Oct 24, 2023
6 of 7 checks passed
@Miaplacidus Miaplacidus deleted the 3924-mod_archive_thread branch October 24, 2023 17:37
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.

Moderation: Inline Archiving UI
10 participants