Apply suggestions from code review

Co-authored-by: Benjamin Bouvier <[email protected]>
Signed-off-by: Daniel Salinas <[email protected]>

crates/matrix-sdk-base/src/store/memory_store.rs

@@ -754,15 +754,13 @@ impl StateStore for MemoryStore {
         kind: QueuedRequestKind,
         priority: usize,
     ) -> Result<(), Self::Error> {
-        self.inner.write().unwrap().send_queue_events.entry(room_id.to_owned()).or_default().push(
-            QueuedRequest {
-                kind,
-                transaction_id,
-                error: None,
-                priority,
-                created_at: Some(created_at),
-            },
-        );
+        self.inner
+            .write()
+            .unwrap()
+            .send_queue_events
+            .entry(room_id.to_owned())
+            .or_default()
+            .push(QueuedRequest { kind, transaction_id, error: None, priority, created_at });
         Ok(())
     }
 
@@ -875,7 +873,7 @@ impl StateStore for MemoryStore {
                 parent_transaction_id: parent_transaction_id.to_owned(),
                 own_transaction_id,
                 parent_key: None,
-                created_at: Some(created_at),
+                created_at,
             });
         Ok(())
     }

crates/matrix-sdk-base/src/store/send_queue.rs

@@ -133,8 +133,8 @@ pub struct QueuedRequest {
     /// should be handled.
     pub priority: usize,
 
-    /// The time that the request was original attempted.
-    pub created_at: Option<MilliSecondsSinceUnixEpoch>,
+    /// The time that the request was originally attempted.
+    pub created_at: MilliSecondsSinceUnixEpoch,
 }
 
 impl QueuedRequest {
@@ -376,9 +376,8 @@ pub struct DependentQueuedRequest {
     /// returned by the server once the local echo has been sent out.
     pub parent_key: Option<SentRequestKey>,
 
-    /// The time that the request was original attempted.
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub created_at: Option<MilliSecondsSinceUnixEpoch>,
+    /// The time that the request was originally attempted.
+    pub created_at: MilliSecondsSinceUnixEpoch,
 }
 
 impl DependentQueuedRequest {

crates/matrix-sdk-indexeddb/src/state_store/mod.rs

@@ -476,7 +476,7 @@ impl PersistedQueuedRequest {
             transaction_id: self.transaction_id,
             error,
             priority,
-            created_at: self.created_at,
+            created_at: self.created_at.unwrap_or(MilliSecondsSinceUnixEpoch::now()),
         })
     }
 }
@@ -1402,6 +1402,7 @@ impl_state_store!({
             || Ok(Vec::new()),
             |val| self.deserialize_value::<Vec<PersistedQueuedRequest>>(&val),
         )?;
+
         // Push the new request.
         prev.push(PersistedQueuedRequest {
             room_id: room_id.to_owned(),
@@ -1608,7 +1609,7 @@ impl_state_store!({
             parent_transaction_id: parent_txn_id.to_owned(),
             own_transaction_id: own_txn_id,
             parent_key: None,
-            created_at: Some(created_at),
+            created_at,
         });
 
         // Save the new vector into db.

crates/matrix-sdk-sqlite/src/state_store.rs

@@ -1862,7 +1862,8 @@ impl StateStore for SqliteStateStore {
 
         let mut requests = Vec::with_capacity(res.len());
         for entry in res {
-            let created_at = UInt::new(entry.4).map(MilliSecondsSinceUnixEpoch);
+            let created_at = UInt::new(entry.4)
+                .map_or_else(|| MilliSecondsSinceUnixEpoch::now(), MilliSecondsSinceUnixEpoch);
             requests.push(QueuedRequest {
                 transaction_id: entry.0.into(),
                 kind: self.deserialize_json(&entry.1)?,
@@ -2059,7 +2060,8 @@ impl StateStore for SqliteStateStore {
 
         let mut dependent_events = Vec::with_capacity(res.len());
         for entry in res {
-            let created_at = UInt::new(entry.4).map(MilliSecondsSinceUnixEpoch);
+            let created_at = UInt::new(entry.4)
+                .map_or_else(|| MilliSecondsSinceUnixEpoch::now(), MilliSecondsSinceUnixEpoch);
             dependent_events.push(DependentQueuedRequest {
                 own_transaction_id: entry.0.into(),
                 parent_transaction_id: entry.1.into(),
@@ -2467,6 +2469,7 @@ mod migration_tests {
 
         Ok(())
     }
+
     fn add_dependent_send_queue_event_v7(
         this: &SqliteStateStore,
         txn: &Transaction<'_>,

crates/matrix-sdk-ui/src/timeline/event_handler.rs

@@ -1033,7 +1033,7 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> {
                 send_state: EventSendState::NotSentYet,
                 transaction_id: txn_id.to_owned(),
                 send_handle: send_handle.clone(),
-                created_at: send_handle.clone().and_then(|h| h.created_at),
+                created_at: send_handle.clone().map(|h| h.created_at),
             }
             .into(),
 

crates/matrix-sdk-ui/src/timeline/event_item/mod.rs

@@ -268,7 +268,7 @@ impl EventTimelineItem {
         as_variant!(&self.kind, EventTimelineItemKind::Local(local) => &local.send_state)
     }
 
-    /// Get the local time that the event was enqueued at.
+    /// Get the time that the local event was pushed in the send queue at.
     pub fn local_created_at(&self) -> Option<MilliSecondsSinceUnixEpoch> {
         as_variant!(&self.kind, EventTimelineItemKind::Local(local) => local.created_at).flatten()
     }

crates/matrix-sdk-ui/src/timeline/tests/echo.rs

@@ -334,7 +334,7 @@ async fn test_no_reuse_of_counters() {
     let local_id = assert_next_matches_with_timeout!(stream, VectorDiff::PushBack { value: item } => {
         let event_item = item.as_event().unwrap();
         assert!(event_item.is_local_echo());
-        assert_matches!(event_item.send_state(), Some(EventSendState::NotSentYet{ .. }));
+        assert_matches!(event_item.send_state(), Some(EventSendState::NotSentYet));
         assert!(!event_item.can_be_replied_to());
         item.unique_id().to_owned()
     });

crates/matrix-sdk-ui/tests/integration/timeline/echo.rs

@@ -172,7 +172,7 @@ async fn test_retry_failed() {
 
     // First, local echo is added.
     assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => {
-        assert_matches!(value.send_state(), Some(EventSendState::NotSentYet{ ..}));
+        assert_matches!(value.send_state(), Some(EventSendState::NotSentYet));
     });
 
     // Sending fails, because the error is a transient one that's recoverable,
@@ -318,7 +318,7 @@ async fn test_cancel_failed() {
 
     // Local echo is added (immediately)
     assert_next_matches!(timeline_stream, VectorDiff::PushBack { value } => {
-        assert_matches!(value.send_state(), Some(EventSendState::NotSentYet{ ..}));
+        assert_matches!(value.send_state(), Some(EventSendState::NotSentYet));
     });
 
     // Sending fails, the mock server has no matching route

crates/matrix-sdk-ui/tests/integration/timeline/queue.rs

@@ -273,7 +273,7 @@ async fn test_reloaded_failed_local_echoes_are_marked_as_failed() {
     // Local echoes are updated with the failed send state as soon as the error
     // response has been received.
     assert_let!(Some(VectorDiff::Set { index: 0, value: first }) = timeline_stream.next().await);
-    let (error, is_recoverable) = assert_matches!(first.send_state().unwrap(), EventSendState::SendingFailed { error, is_recoverable, .. } => (error, is_recoverable));
+    let (error, is_recoverable) = assert_matches!(first.send_state().unwrap(), EventSendState::SendingFailed { error, is_recoverable } => (error, is_recoverable));
 
     // The error is not recoverable.
     assert!(!is_recoverable);
@@ -292,7 +292,7 @@ async fn test_reloaded_failed_local_echoes_are_marked_as_failed() {
     assert_eq!(initial.len(), 1);
     assert_eq!(initial[0].content().as_message().unwrap().body(), "wall of text");
     assert_let!(
-        Some(EventSendState::SendingFailed { error, is_recoverable, .. }) = initial[0].send_state()
+        Some(EventSendState::SendingFailed { error, is_recoverable }) = initial[0].send_state()
     );
 
     // Same recoverable status as above.

crates/matrix-sdk/src/send_queue.rs

@@ -454,7 +454,7 @@ impl RoomSendQueue {
             room: self.clone(),
             transaction_id: transaction_id.clone(),
             media_handles: None,
-            created_at: Some(created_at),
+            created_at,
         };
 
         let _ = self.inner.updates.send(RoomSendQueueUpdate::NewLocalEvent(LocalEcho {
@@ -1916,7 +1916,7 @@ pub struct SendHandle {
     media_handles: Option<MediaHandles>,
 
     /// The time that this send handle was first created
-    pub created_at: Option<MilliSecondsSinceUnixEpoch>,
+    pub created_at: MilliSecondsSinceUnixEpoch,
 }
 
 impl SendHandle {
@@ -2168,7 +2168,7 @@ impl SendReactionHandle {
             room: self.room.clone(),
             transaction_id: self.transaction_id.clone().into(),
             media_handles: None,
-            created_at: Some(MilliSecondsSinceUnixEpoch::now()),
+            created_at: MilliSecondsSinceUnixEpoch::now(),
         };
 
         handle.abort().await
@@ -2263,7 +2263,7 @@ mod tests {
                 .unwrap(),
             },
             parent_key: None,
-            created_at: Some(created_at),
+            created_at,
         };
 
         let res = canonicalize_dependent_requests(&[edit]);
@@ -2275,7 +2275,7 @@ mod tests {
         );
         assert_eq!(msg.body(), "edit");
         assert_eq!(res[0].parent_transaction_id, txn);
-        assert_eq!(res[0].created_at, Some(created_at));
+        assert_eq!(res[0].created_at, created_at);
     }
 
     #[async_test]
@@ -2338,7 +2338,7 @@ mod tests {
                 .unwrap(),
             },
             parent_key: None,
-            created_at: None,
+            created_at: MilliSecondsSinceUnixEpoch::now(),
         };
         let res = canonicalize_dependent_requests(&[edit]);
 
@@ -2359,7 +2359,7 @@ mod tests {
             parent_transaction_id: txn.clone(),
             kind: DependentQueuedRequestKind::RedactEvent,
             parent_key: None,
-            created_at: None,
+            created_at: MilliSecondsSinceUnixEpoch::now(),
         };
 
         let edit = DependentQueuedRequest {
@@ -2372,7 +2372,7 @@ mod tests {
                 .unwrap(),
             },
             parent_key: None,
-            created_at: None,
+            created_at: MilliSecondsSinceUnixEpoch::now(),
         };
 
         inputs.push({
@@ -2412,7 +2412,7 @@ mod tests {
                     .unwrap(),
                 },
                 parent_key: None,
-                created_at: None,
+                created_at: MilliSecondsSinceUnixEpoch::now(),
             })
             .collect::<Vec<_>>();
 
@@ -2444,7 +2444,7 @@ mod tests {
                 kind: DependentQueuedRequestKind::RedactEvent,
                 parent_transaction_id: txn1.clone(),
                 parent_key: None,
-                created_at: None,
+                created_at: MilliSecondsSinceUnixEpoch::now(),
             },
             // This one pertains to txn2.
             DependentQueuedRequest {
@@ -2457,7 +2457,7 @@ mod tests {
                 },
                 parent_transaction_id: txn2.clone(),
                 parent_key: None,
-                created_at: None,
+                created_at: MilliSecondsSinceUnixEpoch::now(),
             },
         ];
 
@@ -2488,7 +2488,7 @@ mod tests {
             kind: DependentQueuedRequestKind::ReactEvent { key: "🧠".to_owned() },
             parent_transaction_id: txn.clone(),
             parent_key: None,
-            created_at: None,
+            created_at: MilliSecondsSinceUnixEpoch::now(),
         };
 
         let edit_id = ChildTransactionId::new();
@@ -2502,7 +2502,7 @@ mod tests {
             },
             parent_transaction_id: txn,
             parent_key: None,
-            created_at: None,
+            created_at: MilliSecondsSinceUnixEpoch::now(),
         };
 
         let res = canonicalize_dependent_requests(&[react, edit]);

crates/matrix-sdk/src/send_queue/upload.rs

@@ -207,7 +207,7 @@ impl RoomSendQueue {
             room: self.clone(),
             transaction_id: send_event_txn.clone().into(),
             media_handles: Some(MediaHandles { upload_thumbnail_txn, upload_file_txn }),
-            created_at: Some(created_at),
+            created_at,
         };
 
         let _ = self.inner.updates.send(RoomSendQueueUpdate::NewLocalEvent(LocalEcho {