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

Backfill Tapbacks/Reactions & BlueBubbles Fix for Removing a Reaction #209

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

joshuafhiggins
Copy link
Contributor

@joshuafhiggins joshuafhiggins commented Apr 9, 2024

Resolves #185

Creates reaction events and sends them to the Matrix server and database after all other messages are sent. The other messages are required to be sent before reactions because reaction events need the event ID of the original message in order to be sent, which is only acquired by sending the target message first. Sending events to the Matrix server was moved to its own function because it now needs to be done twice. There is a case where reaction events fail to be created because we are unable to lookup the event ID of the link in a rich link (as rich links have double the reactions as normal messages, one for the favicon and the other for the link, see #183).

Note: This has only been tested for BlueBubbles, if someone could test this with the other connecters it would be much appreciated. There seemed to be a concern that removed reactions would need to be handled differently but BB doesn't keep track of removed reactions when querying because the reaction no longer exists (it does but prefixes it with a dash).

This PR also fixes removing a reaction for BB. Previously, the connector would send and handle the removal of reactions correctly but would add a reaction of an empty string to messages when the connector would get the updated message event. This is because the string passed in for TapbackFromName strictly matched the string and didn't handle the "-" that BB would prefix the reaction.

Copy link
Collaborator

@dltacube dltacube left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You said it only works with bluebubbles and should be tested w/ other integrations but that might be hard but I feel like this might not interfere with them. If it does we can always gate it behind a feature flag like we do with other things.

I'm gonna see if I can't test this tomorrow on my own machine to see if I can't produce more useful feedback.

historysync.go Show resolved Hide resolved
historysync.go Show resolved Hide resolved
@joshuafhiggins
Copy link
Contributor Author

joshuafhiggins commented Apr 10, 2024

cf1d86f Fixes redacting tapbacks for all of the BlueBubbles connector, previously "removing" would react with an empty string as if it were an emoji as it updated on the Matrix side. It was handled properly everywhere else though. If its better, this can be moved to its own PR.

@joshuafhiggins joshuafhiggins changed the title Backfill Tapbacks/Reactions Backfill Tapbacks/Reactions & BlueBubbles Fix for Removing a Reaction Apr 10, 2024
@rollingonchrome
Copy link

rollingonchrome commented Apr 12, 2024

After backfilling, chats appear to be sorted by the date of the last Tapback (unexpected) rather than the date of the last event, be it a message or a Tapback (expected).

For example, a chat with messages from as recent as 4/12/24 but only earlier Tapbacks from 1/1/24 is shown in the Inbox/Archive with a date of 1/1/24 and is missorted as a result.

@trek-boldly-go
Copy link
Collaborator

Thanks for testing @rollingonchrome

@joshuafhiggins I have noticed the way matrix orders messages is based on the order they come in, not based on sent date. You'll notice the backfill code in BB gets the messages with the newest on top, but I had to write a reverse list function to reverse the message order, that way they display correctly in the UI. A bit silly, so maybe you can find a better way to do it.

@joshuafhiggins
Copy link
Contributor Author

Thanks for testing @rollingonchrome

@joshuafhiggins I have noticed the way matrix orders messages is based on the order they come in, not based on sent date. You'll notice the backfill code in BB gets the messages with the newest on top, but I had to write a reverse list function to reverse the message order, that way they display correctly in the UI. A bit silly, so maybe you can find a better way to do it.

These commits should now fix it: we save the last massage chronologically for the end of the backfill to submit.

@rollingonchrome
Copy link

These commits should now fix it: we save the last message chronologically for the end of the backfill to submit.

@joshuafhiggins – the bridge is now working as expected with the BlueBubbles connector. Tapback backfill is working and chats are sorted as expected. Thanks for the quick update.

historysync.go Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add backfill for "Tapback" reactions
5 participants