Fix favourite notification without status #10043
Conversation
| @@ -14,10 +14,12 @@ def call(account, status) | |||
|
|
|||
| return favourite unless favourite.nil? | |||
|
|
|||
| favourite = Favourite.create!(account: account, status: status) | |||
| Favourite.transaction do | |||
There was a problem hiding this comment.
In NotifyService, we have a check if the activity still exists, I believe it serves the same purpose as wrapping it in a transaction. It should be noted that putting more code into a transaction is not great for performance. ReblogService uses an async worker for NotifyService, and for consistency it might make sense to do the same here instead of using a transaction.
There was a problem hiding this comment.
Sorry for not answering for a long time.
I tried to do the same thing with async worker several times, but it didn't work.
I think this issue is a timing issue as shown in the image below.
I believe this issue can only be resolved by either:
- Combine favorite creation process and notification creation process into one transaction to prevent users from deleting favorites until notification is created
- Add a foreign key constraint on the database side, and fail to insert notification to deleted favorites
And the latter method requires a (potentially large) migration to the notifications table, which can take a long time on a large instance.
Therefore, I think that the former method has no choice but to solve.
Please let me know if there is anything I missed.
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
bump |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
/unstale |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Fix this.
close imas#63 and maybe fix #2865