codidact
Suggested edit compares to the latest version, not the version that was edited
meta:291061
A suggested edit shows a diff. This diff compares the edit to the latest version of the post, which is always correct for an active suggested edit, but is not correct for one that has been resolved.
Example: this edit was proposed to revision 1 in this history, at 2024-03-13T00:28:17Z. That suggested edit was rejected, and then the post was edited half an hour later (rev 2) and twice more after that. The diff in the suggested edit compares the edit to the latest version. It should compare it to rev 1, where the edit was proposed.
An unclear diff like this can appear to put changes in a user's mouth, so to speak, and unclear history makes review and moderation more challenging.
Unfortunately, I don't think we can easily fix the already borked ones (given that they are rejected, it's not worth the effort required to dig up post history states at the time of rejection, but I'll see how feasible that is), but the fix itself is straightforward.
Note: this only applies to rejected suggested edits from what I can tell
I think it applies to all resolved suggested edits that are not the most recent edit. If a suggestion was accepted and then the post was further edited, viewing the suggested edit itself shows the diff with head. The post history shows what the changes were, but if you're looking at a suggested edit directly, like from your activity history, you get the confusing view.
When you say the fix is straightforward, going forward, but we can't fix the already-messed-up ones, do you mean that the fix involves recording some extra state at the time the suggestion is resolved? (And obviously we didn't do that in the past.)
I think there might be two schools of thought on this. On one hand, comparing to the revision that was edited is most faithful to the changes that were made, but comparing to the current revision has the advantage of showing you what changes will be made to the post as it currently stands.
The only other way to resolve that disparity would be to disallow edits while there's a pending suggestion on the post.
but we can't fix the already-messed-up ones
no longer applicable, @cellio, I've added a special maintenance task to retroactively fix up such edits (not ready for review yet as it needs some tweaking, but can be seen in the linked PR).
I think it applies to all resolved suggested edits that are not the most recent edit
I don't think it does - do you have a reproducible example for this one? The controller's approve method addresses this problem specifically - it's only the reject method that, until now, was lacking the logic needed.
I think there might be two schools of thought on this. On one hand, comparing to the revision that was edited is most faithful to the changes that were made, but comparing to the current revision has the advantage of showing you what changes will be made to the post as it currently stands.
This issue only applies to suggested edits that have been resolved (either applied or rejected).
Why would somebody be looking at an already-resolved suggestion? As a user, to understand what happened; as a moderator, while looking at patterns.
The only other way to resolve that disparity would be to disallow edits while there's a pending suggestion on the post.
That's what we do now. :-) When a suggestion is pending, the "edit" link is replaced with either "review" (if you can) or "suggestion pending".
I don't think it does - do you have a reproducible example for this one? The controller's
approvemethod addresses this problem specifically - it's only therejectmethod that, until now, was lacking the logic needed.
Oh, you're right -- not sure how to find a case in the wild, but I just tested that scenario locally and it's correct. Sorry for the faulty memory.
This issue only applies to suggested edits that have been resolved (either applied or rejected).
Ah, I misunderstood what we were looking at - thank you.
@Oaphi: does the fix only apply to new edits (works going forward), or did we expect old suggested edits like the one in the description to now behave differently?
@cellio only if / when we run the maintenance task - I presume we haven't done that yet given that the deployment was a part of emergency fixes and not a scheduled one. Until then the change is not retroactive