feat(edit): Add edit revisions list#6630
Conversation
|
bxdxnn
force-pushed
the
edit-revisions
branch
4 times, most recently
from
e97032f to
1a1352c
Compare
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Hello,
Thanks for your contributions.
There is an annoying design issue with your proposal. Doing that on the Timeline means it only works on in-memory events: it misses all the events we have in the database and that are not loaded yet in the Timeline yet.
This feature should instead be implemented on the Event Cache, where we have methods like find_events_with_relations that can be used to retrieve all events related to a particular event, such as: all edits for event EventTimelineItem to retrieve all edits for a particular event would be the best way to approach this.
I think it's a better approach to the problem.
Also, please, add tests.
Thoughts?
Signed-off-by: bxdxnn <267911624+bxdxnn@users.noreply.github.com>
|
Sorry, still learning how the SDK works. Yes, I agree, makes more sense. Rewrote the implementation |
| if let Some(content) = TimelineItemContent::from_event(room, original_event).await { | ||
| revisions.push(EditRevision { |
There was a problem hiding this comment.
Why not keeping TimelineItemContent directly?
There was a problem hiding this comment.
Because we also need the timestamp of the event, TimelineItemContent doesn't provide it.
There was a problem hiding this comment.
Can't we manipulate a TimelineItem directly then?
There was a problem hiding this comment.
TimelineItem is pretty heavy and also has a lot of irrelevant info
Signed-off-by: bxdxnn <267911624+bxdxnn@users.noreply.github.com>
|
|
||
| let timeline = room.timeline().await.unwrap(); | ||
|
|
||
| let revisions = timeline.edit_revisions(event_id!("$nonexistent")).await.unwrap(); |
There was a problem hiding this comment.
I wonder if it wouldn't better to have edit_revisions on a TimelineItem instead of Timeline? cc @stefanceriu @jmartinesp
There was a problem hiding this comment.
In our case, the timeline items are mapped to record types ASAP, so having to hold to the original item is difficult. It's simpler to have it on Timeline.
There was a problem hiding this comment.
I've checked a summary from an LLM previously and it seems to be not worth it, but that's up to you as I don't really understand the logic between components ATM.
Co-authored-by: Ivan Enderlin <ivan@mnt.io> Signed-off-by: bxdxnn <267911624+bxdxnn@users.noreply.github.com>
3 participants
Add a new
revisionsfield for timeline items that stores the history of the item.Rust part of element-hq/element-meta#2573
(see Writing changelog entries).
Signed-off-by: bxdxnn 267911624+bxdxnn@users.noreply.github.com