Page MenuHomePhabricator
Differential D14144

[native] Stop resetting ChatThreadList on navigation
ClosedPublic
Actions

Authored by ashoat on Dec 11 2024, 3:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 20, 4:25 AM
Unknown Object (File)
Sun, Apr 20, 3:57 AM
Unknown Object (File)
Sun, Apr 20, 3:05 AM
Unknown Object (File)
Sun, Apr 20, 2:34 AM
Unknown Object (File)
Fri, Apr 18, 8:29 PM
Unknown Object (File)
Sun, Mar 30, 8:13 AM
Unknown Object (File)
Mar 11 2025, 4:15 PM
Unknown Object (File)
Mar 11 2025, 4:15 PM
View All 56 Files
Subscribers
None

Details

Reviewers
tomek
angelika
Commits
rCOMM58176056c7d3: [native] Stop resetting ChatThreadList on navigation
Summary

Currently, if the user scrolls down to a chat and then opens it, and then goes back, the ChatThreadList will not appear the same as before, and their chat may not be visible.

I think this is confusing behavior in the first place. It gets especially bad when the ChatThreadList is reloading its order async.

Depends on D14143

Test Plan

I tested this task by playing around with the ChatThreadList on mobile while using a stopwatch to measure how long various operations took. I tested the updated code 3 times against both the most recent Testflight build as well as master. I found that performance was approximately the same as before. I tested scrolling down, selecting threads that were read, selecting threads that were unread, and search.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Revision Contents
Changeset List

PathSize
native/
chat/
16 lines

Diff 46444

View Options

native/chat/chat-thread-list.react.js

use std::collections::HashMap; use std::collections::HashMap;
use comm_lib::aws::ddb::types::AttributeValue; use comm_lib::aws::ddb::types::AttributeValue;
use comm_lib::aws::ddb::types::ReturnValue;
use comm_lib::database::AttributeExtractor; use comm_lib::database::AttributeExtractor;
use comm_lib::database::AttributeMap; use comm_lib::database::AttributeMap;
use comm_lib::database::DBItemAttributeError; use comm_lib::database::DBItemAttributeError;
use comm_lib::database::DBItemError; use comm_lib::database::DBItemError;
use comm_lib::database::TryFromAttribute; use comm_lib::database::TryFromAttribute;
use comm_lib::database::Value; use comm_lib::database::Value;
use tracing::error; use tracing::error;
▲ Show 20 Lines • Show All 47 Lines • ▼ Show 20 Lines ) -> Result<Vec<FarcasterUserData>, Error> {
Ok(users) Ok(users)
} }
pub async fn add_farcaster_id( pub async fn add_farcaster_id(
&self, &self,
user_id: String, user_id: String,
farcaster_id: String, farcaster_id: String,
) -> Result<(), Error> { ) -> Result<(), Error> {
let update_expression = let update_expression = format!(
format!("SET {} = :val", USERS_TABLE_FARCASTER_ID_ATTRIBUTE_NAME); "SET {} = if_not_exists({}, :val)",
let expression_attribute_values = USERS_TABLE_FARCASTER_ID_ATTRIBUTE_NAME,
HashMap::from([(":val".to_string(), AttributeValue::S(farcaster_id))]); USERS_TABLE_FARCASTER_ID_ATTRIBUTE_NAME
);
bartekUnsubmitted
Not Done
Inline Actions
) -> Result<(), Error> {
let update_expression = format!(
- "SET {} = if_not_exists({}, :val)",
+ "SET {0} = if_not_exists({0}, :val)",
USERS_TABLE_FARCASTER_ID_ATTRIBUTE_NAME,
- USERS_TABLE_FARCASTER_ID_ATTRIBUTE_NAME
);
let expression_attribute_values = HashMap::from([(

You don't have to provide the same value twice

bartek: You don't have to provide the same value twice
let expression_attribute_values = HashMap::from([(
":val".to_string(),
AttributeValue::S(farcaster_id.clone()),
)]);
self let response = self
.client .client
.update_item() .update_item()
.table_name(USERS_TABLE) .table_name(USERS_TABLE)
.key(USERS_TABLE_PARTITION_KEY, AttributeValue::S(user_id)) .key(USERS_TABLE_PARTITION_KEY, AttributeValue::S(user_id))
.update_expression(update_expression) .update_expression(update_expression)
.set_expression_attribute_values(Some(expression_attribute_values)) .set_expression_attribute_values(Some(expression_attribute_values))
.return_values(ReturnValue::UpdatedNew)
.send() .send()
.await .await
.map_err(|e| Error::AwsSdk(e.into()))?; .map_err(|e| Error::AwsSdk(e.into()))?;
bartekUnsubmitted
Not Done
Inline Actions

Does UpdateExpression if_not_exists fail the operation if given value already exists? The docs don't say this explicitly, but I guess

  • if_not_exists (path, value)

If the item does not contain an attribute at the specified path, if_not_exists evaluates to value; otherwise, it evaluates to path.

means that if the ID already exists, the expression evaluates to SET farcasterID = farcasterID so it's a no-op.

According to your test plan, it succeeds so it's alright

bartek: Does `UpdateExpression` `if_not_exists` fail the operation if given value already exists?
varunAuthorUnsubmitted
Done
Inline Actions

yeah i checked and it succeeds so assuming DDB treats it as a no-op

varun: yeah i checked and it succeeds so assuming DDB treats it as a no-op
match response.attributes {
None => return Err(Error::MissingItem),
Some(mut attrs) => {
let farcaster_id_from_table: String =
attrs.take_attr(USERS_TABLE_FARCASTER_ID_ATTRIBUTE_NAME)?;
if farcaster_id_from_table != farcaster_id {
return Err(Error::CannotOverwrite);
}
}
}
Ok(()) Ok(())
} }
} }
impl TryFrom<AttributeMap> for FarcasterUserData { impl TryFrom<AttributeMap> for FarcasterUserData {
type Error = DBItemError; type Error = DBItemError;
fn try_from(mut attrs: AttributeMap) -> Result<Self, Self::Error> { fn try_from(mut attrs: AttributeMap) -> Result<Self, Self::Error> {
Show All 28 Lines