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

fix: Ensure actions happen against the correct status #373

Merged
merged 4 commits into from
Jan 26, 2024

Conversation

nikclayton
Copy link
Contributor

Previously when the user interacted with a status the operation (reblog, favourite, etc) travels through multiple layers of code, carrying with it the position of the item in the list that the user operated on.

At some point the status is retrieved from the list using its position so that the correct status ID can be used in the network operation.

If this happens while the list is also refreshing there's a possible race condition, and the original status' position may have changed in the list. Looking up the status by position to determine which status to perform the action on may cause the action to happen on the wrong status.

Fix this by passing the status' viewdata to any actions instead of its position. This includes all the information necessary to make the API call, so there is no chance of a race.

This is quite an involved change because there are three types of viewdata:

  • StatusViewData, used for regular timelines
  • NotificationViewData, used for notifications, may wrap a status that can be operated on
  • ConversationViewData, used for conversations, does wrap a status

The previous code treated them all differently, which is probably why it operated by position instead of type.

The high level fix is to:

  1. Create an interface, IStatusViewData, that contains the data exposed by any viewdata that contains a status.

  2. Implement the interface in StatusViewData, NotificationViewData, and ConversationViewData.

  3. Change the code that operates on viewdata (SFragment, StatusActionListener, etc) to be generic over anything that implements IStatusViewData.

  4. Change the code that handles actions to pass the viewdata instead of the position.

Fixes #370

Previously when the user interacted with a status the operation (reblog,
favourite, etc) travels through multiple layers of code, carrying with
it the position of the item in the list that the user operated on.

At some point the status is retrieved from the list using its position
so that the correct status ID can be used in the network operation.

If this happens while the list is also refreshing there's a possible
race condition, and the original status' position may have changed in
the list. Looking up the status by position to determine which status
to perform the action on may cause the action to happen on the wrong
status.

Fix this by passing the status' viewdata to any actions instead of
its position. This includes all the information necessary to make the
API call, so there is no chance of a race.

This is quite an involved change because there are three types of
viewdata:

- `StatusViewData`, used for regular timelines
- `NotificationViewData`, used for notifications, may wrap a status
  that can be operated on
- `ConversationViewData`, used for conversations, does wrap a status

The previous code treated them all differently, which is probably why
it operated by position instead of type.

The high level fix is to:

1. Create an interface, `IStatusViewData`, that contains the data
   exposed by any viewdata that contains a status.

2. Implement the interface in `StatusViewData`, `NotificationViewData`,
   and `ConversationViewData`.

3. Change the code that operates on viewdata (`SFragment`,
   `StatusActionListener`, etc) to be generic over anything that
   implements `IStatusViewData`.

4. Change the code that handles actions to pass the viewdata instead
   of the position.

Fixes pachli#370
…ong-post

# Conflicts:
#	app/lint-baseline.xml
@nikclayton nikclayton merged commit fc81e8b into pachli:main Jan 26, 2024
6 checks passed
@nikclayton nikclayton deleted the 370-post-actions-wrong-post branch January 26, 2024 11:15
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.

Post actions operating on the wrong post
1 participant