search

themotte / rDrama

This code runs https://www.themotte.org. Forked from https://github.com/Aevann1/rDrama
GNU Affero General Public License v3.0
25 stars 30 forks source link

Sit down and figure out a better system for post/comment state. #359

Closed zorbathut closed 1 year ago

zorbathut commented 1 year ago

Right now it is unclear how many states a post or comment can be in. As near as I can tell there's:

This all ends up weird because there's two different ways that mods can remove posts; one of them, as near as I can tell, also cannot be undone by mods.

Remove the filter-removed-by-mod state and reconcile that with the main removed-by-mod. Make sure to port existing data over in the database update :V While you're in there, look for any other states posts have and, uh, do something with them? Feel free to poke me if you're unsure what the right decision is.

mjhouse commented 1 year ago

Hey @zorbathut, do you remember what properties on the classes or columns in the database are used for those flags, offhand? I found filter_state, and it looks like it has three string values (EDIT - looks like it's actually 5):

zorbathut commented 1 year ago

I do not, I'm afraid. There should be a fourth state somewhere, which is "normal and ignore future reports", but that might be a separate boolean.

I think my suspicion here is that "filter_state" should be renamed to "state", whatever the existing Removed boolean is should be removed, and that boolean should be turned into "state = removed". But this is guesswork without having looked at the code.

JulianRota commented 1 year ago

Yes, it's filter_state, and the currently allowed/expected values also includes ignored. Done in PRs #81 and #120 . It does implement Zorba's initially proposed workflow for post/comment filtering basically as described and integrate the reporting system into this workflow. It seems it does not fully integrate all the ways mods may remove posts, the ability for users to remove their own posts, and exactly who should be able to do what to a post/comment where after it has been removed.

mjhouse commented 1 year ago

Initially, just from poking around this morning, I'm thinking I need to put together a list of every possible filter/delete state and figure out where that's all handled now. I edited my comment above, but I'll repost it here for clarity- the filter_state values I've found in the codebase:

I'm not sure what the above values all mean. The states that I think I need to track down:

Once I've got all the possible filtering states tracked down, I think I'll replace all of them with a single Enum type that has good comments, if no one has any objections. SqlAlchemy and Postgresql can handle enums, and I think it will make it a lot more clear what possible states a submission can be in.

JulianRota commented 1 year ago

The filtered is the state that new posts and comments go into under certain criteria that basically means they're invisible to everyone but the original poster and mods until a mod approves it. I think the only actual new state we need is "removed by user".

I think what's needed for the transition is to convert everything that sets or uses those other flags to use this state field, and write a database migration that includes combining the "old" state field and those flags into a "new" state field. Including making sure we define in the code who can un-remove an item and UI to support that action.

If you know how to use SqlAlchemy and PG enums properly that's even better IMO.

mjhouse commented 1 year ago

...I think the only actual new state we need is "removed by user"

If this is accurate then that makes things a lot simpler. What other flags, specifically, should we combine with state? I haven't seen any booleans on the submission/comment classes that seem related.

zorbathut commented 1 year ago

Note that "removed by user" needs to be independent; we need to be able to track user-removed and system-imposed removals separately. I think users can unremove their posts, and we can't have that overwriting the mod decisions.

Also, in theory there are ways posts can get reported despite being in another state (user reports their own post :V) and so that also can't clobber state.

So maybe we're looking at three separate enums:

Userstate:

Modstate:

Reportstate:

All of these states should already exist somehow, but there's going to be some hoopjumping to convert old state into new state.

JulianRota commented 1 year ago

I don't think we need multiple enums, one should be fine.

We need UserRemoved, in which state only the user can see/restore (if anyone), and ModRemoved, in which only mods can see/restore. And also forbid creating reports and moving to report state for items in filtered state.

zorbathut commented 1 year ago

If it's ModRemoved, and the user decides they want to delete it, what do they do? Those flags really are independent - we can't refuse to let a user delete a comment if a mod has removed it. And we can't let a user delete and undelete a comment in order to clear the mod-removal flag.

JulianRota commented 1 year ago

Interesting point... How do the users see their mod-deleted posts to user-delete them? Are they still visible on their user-page, but only for themselves? If we do want to allow that, then I suppose it does need to be a separate flag. I guess it could just as well remain a boolean flag, unless we just really like PG Enums.

Thinking about the use case of user-deletions of mod-deleted items. Nice to allow just for the sake of letting users really control their data, delete something with personal info or that they were embarrassed of even from mods. But would it be an issue if a user making bad posts deleted them after the mods deleted them, so a mod acting on their newest bad post might not know that it's their 6th bad post instead of their first?

Now that I think about it more, what is the purpose of a user-deleted state? We want the post to be visible/restorable, but only for the users, not the mods, even though the DB admin will still be able to see them too? What if we hard-delete for user deletions?

zorbathut commented 1 year ago

Keep in mind that mods can see user-deleted posts still; they're deleted, but they're still in the DB. I'm divided on this, because yeah it is important that we be able to see negative behavior, but also I kinda wish users had a way to actually purge bad stuff. But this is a different issue to deal with later :V

I think my general feeling here is that the more data we store on the exact actions, the better. Honestly I'd like to have an Action Log with users and timestamps, but I don't want that enough to worry about it right now. But we're unlikely to run into trouble with a few extra bytes per post, and we simply cannot retrieve that data later if we throw it away now.

I think users can see mod-deleted posts, I may be wrong on that.

Booleans are fine for things that actually are booleans unless you think we're likely to extend it; I suspect a two-item enum is actually just a boolean (or a char) on the backend. Code for intent, not for optimization :)

mjhouse commented 1 year ago

@zorbathut @JulianRota Hey, I was hoping I would have time to do this, but things have been hectic the last few days and it doesn't look like it's going to stop any time soon. I've unassigned myself.

justcool393 commented 1 year ago

I think users can see mod-deleted posts, I may be wrong on that.

almost positive they can see their own removed posts. essentially it looks like the admin view but they can only see it on their own posts and comments

zorbathut commented 1 year ago

Added note from @justcool393: the User Shadowban state is currently applied to all comments. I'd actually say to just remove this check and make it so new shadowbanned user posts are automatically mod-removed.

justcool393 commented 1 year ago

ref #351 and #369

justcool393 commented 1 year ago

okay so there was previous discussion in discord about this and i think there are a few questions here.

broadly, what states make sense?

the states i'm thinking of are:

user shadowbans can be handled i guess by just autoremoving their comment. content nuker could set the filter state

user state:

(these states are separate)

one question i have is for moderation state do we want reports to be visible to mops on content that's been removed or reports ignored or something (even if collapsed)?

if so, we could actually let users report removed stuff if they really want to, but it just wouldn't show up in the mopqueues

zorbathut commented 1 year ago

I'm still generally in favor of keeping the states as orthogonal as possible, just so we're not trying to draw weird state transition graphs. I posted this above and I'll reproduce it:

Userstate:

Modstate:

Reportstate:

Just as an example of a thing that happens sometimes, take your state list above, imagine I'm reading comments, and I accidentally click "remove" on a post. Whoops! I click "approve" immediately again. But unknown to me, it was actually set to "ignored" before. So now I've unignored it; whoops!

Whereas with this system it won't end up unignored, it'll just be moved from Modstate Normal -> Removed -> Normal.

And yeah, I'm fine with people reporting things that are hidden. I think it's unclear if it should show up in the modqueue or not - I'd probably lean towards "sure, why not" - but I don't want to give an error on it, it's a thing you can do.

zorbathut commented 1 year ago

Doin' some tweaks on this since I am actually tackling it now.

Userstate:

Modstate:

Modstate_set_by is the ID of the last mod to change the state. We don't need this for userstate (the poster is the only person who can change the state) or reportstate (we keep the full list of reports anyway.)

Reportstate:

justcool393 commented 1 year ago

yeah this seems to be reddit's approach and works fine enough