timfitzzz
commented
3 years ago Investigated this a bit, and it turns out that this might be less of a bug and more of an undesired feature, and not the simplest thing to resolve.
The root cause of the problem is basically this:
- Annotations mutate clips in the app's Redux store.
- Subsequently, if an annotation is deleted, we attempt to reverse that mutation as appropriate.
- Annotation types tag, upvote, downvote, and veto are all cleanly reversible, since a user can only apply any of these types once, and removing their contribution can be done without any loss of fidelity.
- Annotation type Link, though, is established by the presence of a clip link in the message. This is overloaded, because a link can be interpreted both as a submission of that clip, and as a reference to it for the purposes of targeting one of the other Annotations. Thus the ambiguity arises: if a Link+Tag is removed, should the Link action be reversed, or only the Tag? It depends on whether the link is a submission or a reference; if it's a submission, it should be reversed -- but if it's just a reference, it should not. The main distinction between these two states is whether there is a prior submission of that clip by this user.
To solve this we need to either a) add complexity to the reversion process, or b) re-examine how annotations and links are handled.
The complexity-added solution: The reversion process could include examining a user's other annotations to determine whether another has the Link annotation type. If so, revert only the non-Link aspect of the message. (If the removed message produced only a Link annotation, though, we can assume the intent is to undo the Link). Downsides: reversion process now has to search for another Link annotation; it's a bit of a monkey-patch, especially considering the additional conditions. The conceptual change solution: Currently, the state of a clip is stored as mutated by all prior annotations. Perhaps an unmodified copy of the clip should be stored as well. That way, if necessary, all prior annotations can be re-applied to a clip (minus the removed ones) to produce the exact desired state.
For now, considering this project's timelines, I'm going to investigate the viability of the complexity-added solution. However, in the long run, the conceptual fix is (as usual) probably the one we should think about moving towards.
Action taken: a. messageRemove on a message containing a link and a tag where link was separately posted b. messageRemove on a message containing a link where link was separately posted
Expected results: a. tag is removed, cliip remains b. user is removed from clip postedBy, clip remains if another postedBy remains
Observed results: a. tag and clip are removed b. clip is removed