Use UniqueId as a parameter for timeline APIs

[jmartinesp]

I'm opening this issue so we can discuss about what we want to do with the Timeline API and UniqueId as a replacement for event/transaction ids.

Context

After #3942 a couple of new Timeline::edit_by_id and Timeline::redact_by_id functions were added to the codebase, which take a TimelineEventItemId (either an EventId or a TransactionId) as the event's identifier since we can't pass back to the FFI layer the original Rust item and we need to use something to search for the expected item in the timeline.

It seems like this is not the API we want, we'd rather use UniqueId instead for the timeline APIs since it's a unique identifier for each timeline item and that way we don't have to track if the event has an id or a transaction id if it's local.

Proposal

What we want to do, as far as I can tell, is:

• Have most/all Timeline functions that modify events in the timeline to use UniqueId instead.
• Use this unique id to search for the items in the timeline.
• Remove the duplicate _by_id functions.

Possible problems

• What happens to functions that already accept only EventId or only TransactionId parameters, like Timeline::replied_to_info_from_event_id? Should we also make them use UniqueId and return an error if the items is not local/remote as expected?
• Some drafts-related code in the mobile EX clients rely on event or transaction ids to work, and although most of them can be moved to use unique ids instead, there are some cases, like editing or replying to an event, where this event may not come from the timeline and not have a unique id. In those cases we need some way to ask the client to use Room::edit and Room::reply instead. This will also be painful for the clients to implement, but there's not much we can do about that.
• Editing every Timeline test that uses their APIs to use UniqueId is going to be a fun ride.

[bnjbvr]

Thanks for opening an issue. I've marked #4100 as closing it, since the PR gets rid of all my concerns with the code duplication we had.

What happens to functions that already accept only EventId or only TransactionId parameters, like Timeline::replied_to_info_from_event_id? Should we also make them use UniqueId and return an error if the items is not local/remote as expected?

We could do that, but as far as I know, the replied_to_info_from_event_id is a way to sift through the timeline's events, considering it a source of truth for event data, when the event cache should be that source of truth instead. So we'd need to look at individual cases.

Editing every Timeline test that uses their APIs to use UniqueId is going to be a fun ride.

insert It's not much but it's honest work meme here

[bnjbvr]

For what it's worth, we've eventually gone down the other road, and use TimelineEventItemId as the ID for all things, instead of the unique-id, which was unstable in a few cases (across different timeline instances, across timeline resets b/o gappy syncs).