codidact / qpixel

Q&A-based community knowledge-sharing software
https://codidact.com
GNU Affero General Public License v3.0
378 stars 69 forks source link

Don't allow edits or suggested edits to change moderator-only tags #1299

Open cellio opened 5 months ago

cellio commented 5 months ago

meta:289786

A non-moderator with the Edit ability (or post owner) who tries to edit a post to modify moderator-only tags gets an error message about the tag restriction. A user proposing an edit to add one, on the other hand, can make that tag change -- but then non-moderators who would otherwise be able to approve the edit cannot, so the suggested edit is in limbo until a moderator handles it. In a related bug, a direct edit to remove a moderator-only tag succeeds and leaves no history on the post.

Can we intercept changes to restricted tags (in either direction) when the edit or suggested edit is submitted and complain? We could, optionally, add a line to the error message directing the user to flag for changes to mod-only tags.

cellio commented 1 month ago

I thought this would be easy (we already have the logic for direct edits; how hard could it be to apply it to suggested edits too?), but I got lost in the post controller and model, sigh.

trichoplax commented 1 month ago

Being in a state of incomplete understanding might be a good opportunity to think about how things might be made easier for future contributors.

I got lost in the post controller and model

It's easy to think that Collab should be reserved for well formed questions, and vague statements like this should go to a discussion in chat (Discord currently). However, I suspect that there is benefit to asking a question on Collab along the lines of "I'm not sure what I'm stuck on, but here's what I've tried". Then an "answer" doesn't need to be a full solution to the overall task, but might just be a hint or a next step.

I personally find it difficult to tell whether my various confusions come from a general lack of understanding of Rails, or a lack of understanding of how QPixel specifically is doing something. I tend to want to fully understand first, and then write a self answered question on Collab, because otherwise how will I know how to phrase the question? I know that the imperfect wording I would have used before understanding would probably be a better match for what future contributors might search for though, so maybe I should get into a habit of posting questions early, when I'm not really sure what I want to ask.

I suppose I'm both suggesting Collab for you, and also asking whether you think that the approach I suggest is an acceptable use of Collab.

cellio commented 1 month ago

Good idea. Here it is: https://collab.codidact.org/posts/291665 .

trichoplax commented 2 weeks ago

In looking into your Collab question, I haven't found an answer but testing did show up a potentially larger problem so I'll mention this first. Would you like to expand the scope of this issue or have a separate issue (probably to be fixed before this one is worked on)? If relevant to that decision, this affects edits rather than suggested edits.

Attempting to remove a mod-only tag succeeds with no history

In my development environment, I took the following steps:

  1. As an admin, make a tag moderator only and add it to a post.
  2. As a user, try to edit my own post to remove the moderator only tag from that post.
  3. As that user, see a red warning message indicating I cannot remove a moderator only tag.
  4. Notice that the post no longer has the moderator only tag.
  5. Go to the post history.
  6. Notice that the latest edit is the one adding the moderator only tag, and there is no edit showing the removal of the moderator only tag.

I don't know if this is a general problem or just a quirk of my development environment, so I've added a tag called "moderator-only" to the dev environment https://a.dev.codidact.org as a test. If you want to use your admin powers there to make it a real moderator only tag, then I'll try to edit to test this and see if we get the same outcome.


Further local development environment testing

I took the following steps for the post that lost its moderator-only tag in the previous testing:

  1. As the same user, make an edit to the text.
  2. Before saving, notice that the moderator only tag is present in the relevant edit field.
  3. After saving, notice that the moderator only tag is present again.
  4. In the edit history, notice that the latest edit shows the moderator only tag being added (by the user).
cellio commented 2 weeks ago

I've changed that tag to mod-only on the dev server.

trichoplax commented 2 weeks ago

I've now carried out the first set of steps on the dev server and I see the same outcome.

If you click Edit below the post, you can see that in the editor page tags section the "moderator-only" tag is still present, even though it isn't visible on the post.

cellio commented 2 weeks ago

Thanks for testing. I've updated the description to incorporate this bug. (I still don't understand how this part of the code works, though, so I can't fix it myself.)

trichoplax commented 2 weeks ago

I initially phrased this incorrectly and later edited my first comment today to fix it. I thought I'd edited before you read it but not sure now.

I was mistaken when I referred to this being a suggested edit problem. It actually happens when I edit my own post (which doesn't require the Edit Posts ability). Does this just require an edit to the issue wording, or would you want this to be a separate issue? Asking because your update to the issue wording refers to a "suggested edit" (my initial mistake) rather than an "attempted edit".

cellio commented 2 weeks ago

I was mistaken when I referred to this being a suggested edit problem. It actually happens when I edit my own post (which doesn't require the Edit Posts ability). Does this just require an edit to the issue wording, or would you want this to be a separate issue? Asking because your update to the issue wording refers to a "suggested edit" (my initial mistake) rather than an "attempted edit".

I've edited the description to (I hope) account for what you said. I don't know if this is easier to tackle as one bug or two, but we might as well keep the information together and if people want to carve off pieces to fix separately, that's fine. I don't understand the edit/suggested-edit code well enough to have an opinion.

trichoplax commented 2 weeks ago

The edited description looks good now.

Letting whoever works on this decide whether to split it sounds good too.