Page MenuHomePhabricator
Differential D14053

[native] DisplayCommunityDirectoryPrompt
ClosedPublic
Actions

Authored by varun on Nov 26 2024, 10:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 14, 8:16 AM
Unknown Object (File)
Fri, Mar 14, 8:16 AM
Unknown Object (File)
Fri, Mar 14, 8:16 AM
Unknown Object (File)
Fri, Mar 14, 8:16 AM
Unknown Object (File)
Fri, Mar 14, 8:16 AM
Unknown Object (File)
Thu, Mar 13, 10:17 AM
Unknown Object (File)
Sun, Mar 9, 4:17 PM
Unknown Object (File)
Tue, Mar 4, 1:19 AM
View All 35 Files
Subscribers
tomek

Details

Reviewers
ashoat
Commits
rCOMM923ce0279648: [native] DisplayCommunityDirectoryPrompt
Summary

the second time a logged in user foregrounds the app, they will see the bottom sheet asking if they want to check out the community directory. if the user is not on the home screen, they won't get the prompt

Depends on D13997

Test Plan

confirmed that prompt only displays once per installation. it doesn't display if user is not logged in or not on the home screen

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

varun created this revision.Nov 26 2024, 10:14 PM
varun held this revision as a draft.
Herald added a subscriber: tomek. · View Herald TranscriptNov 26 2024, 10:14 PM
varun added a child revision: D14054: [native] navigate to community directory nux tip if user declines prompt or closes bottom sheet.Nov 26 2024, 10:19 PM
varun added inline comments.Nov 26 2024, 10:20 PM
native/components/display-community-directory-prompt.react.js
53 ↗(On Diff #46094)

an undefined fid means the user hasn't been prompted to set a farcaster association yet

varun added inline comments.Nov 26 2024, 10:26 PM
native/components/display-community-directory-prompt.react.js
66 ↗(On Diff #46094)

as @ashoat noted on a previous diff in this stack, this isn't a promise, but rather a function that returns a promise. i should revise this before publishing to avoid calling the keyserver endpoint multiple times

Harbormaster completed remote builds in B32879: Diff 46094.Nov 26 2024, 10:32 PM
varun updated this revision to Diff 47013.Feb 11 2025, 8:06 PM

fix up promises and use useCurrentLeafRouteName hook

varun published this revision for review.Feb 11 2025, 8:15 PM
varun edited the summary of this revision. (Show Details)
varun edited the test plan for this revision. (Show Details)
This comment was removed by varun.
Harbormaster completed remote builds in B33476: Diff 47013.Feb 11 2025, 8:27 PM
ashoat requested changes to this revision.Feb 11 2025, 9:10 PM
ashoat added inline comments.
native/components/display-community-directory-prompt.react.js
17 ↗(On Diff #47013)

Can you prioritize moving this constant, instead of adding more references to it? See comment here. You created a follow-up task for this four weeks ago, but this is the kind of thing that should be trivial and probably doesn't even need a Linear task. Should take 5 minutes to put up a diff

51–54 ↗(On Diff #47013)

Why do we need this? Doesn't useOnFirstLaunchEffect already guarantee that the effect will only fire once?

61 ↗(On Diff #47013)

Does this include GENESIS? If so, let's bump the check to 4

62 ↗(On Diff #47013)

Hmm... this approach risks the prompt never being displayed, as it relies on the user foregrounding the app shortly after backgrounding it.

Why did you opt for this approach instead of the one from the task description that references uses connectFarcasterAlertInfo (persisted Redux state)? This would allow us to show the prompt on the second time the app is cold started, which is more reliable.

The whole goal here is simply to avoid showing the prompt at the same time as the NUX.

This revision now requires changes to proceed.Feb 11 2025, 9:10 PM
ashoat mentioned this in D14204: [native] fix avatars in directory.Feb 11 2025, 9:27 PM
ashoat added inline comments.
native/components/display-community-directory-prompt.react.js
84 ↗(On Diff #47013)

We discussed on the call filtering to only communities with farcasterChannelID

ashoat added inline comments.Feb 11 2025, 9:30 PM
native/components/display-community-directory-prompt.react.js
62 ↗(On Diff #47013)

Some context was lost, but looks like @varun had initially taken an approach like the one I'm suggesting in D13509

varun updated this revision to Diff 47296.Thu, Feb 27, 9:36 PM

address feedback

Harbormaster completed remote builds in B33646: Diff 47296.Thu, Feb 27, 9:55 PM
ashoat accepted this revision.Thu, Feb 27, 11:37 PM

Shouldn't D14391 be added to the stack here? You seem to be depending on it

Please make sure to address the comment about the effect running more than once before landing

native/components/display-community-directory-prompt.react.js
68 ↗(On Diff #47296)

I'm worried that this effect will run more than once. In the old system, useOnFirstLaunchEffect gave us this guarantee... but the equivalent check in the new system has been moved upstream, to DisplayCommunityDirectoryPromptHandler. We have to rely on React to unrender this component (due to the Redux dispatch) before it reruns the effect, and I'm not sure that's very reliable. Can we add a ref or something else to make sure this only runs once?

71 ↗(On Diff #47296)

Can you remind me why we need to dispatch this to Redux? Guessing we just want to keep it updated with the latest state? Not necessary for this work, but no downside and we might as well?

75 ↗(On Diff #47296)

I guess we call this here because the data might be out-of-date (it's only updated when CommunityDrawerContent is opened, right?)

This revision is now accepted and ready to land.Thu, Feb 27, 11:37 PM
varun added a parent revision: D14391: [native] ColdStartTracker.Sat, Mar 1, 9:09 AM
varun added inline comments.Sat, Mar 1, 1:47 PM
native/components/display-community-directory-prompt.react.js
71 ↗(On Diff #47296)

Not necessary for this work, but no downside and we might as well?

that's right

75 ↗(On Diff #47296)

that's correct

Closed by commit rCOMM923ce0279648: [native] DisplayCommunityDirectoryPrompt (authored by varun). · Explain WhySat, Mar 1, 2:25 PM
This revision was automatically updated to reflect the committed changes.
varun added a commit: rCOMM923ce0279648: [native] DisplayCommunityDirectoryPrompt.

Revision Contents
Changeset List

PathSize
lib/
types/
2 lines
native/
components/
104 lines
navigation/
2 lines
2 lines
2 lines
utils/
3 lines

Diff 47318

View Options

lib/types/alert-types.js

Show First 20 Lines • Show All 138 Lines • ▼ Show 20 Lines ) -> Self {
Self { Self {
user_id: user_id.into(), user_id: user_id.into(),
device_ids, device_ids,
timestamp: update_info.timestamp.unwrap_or_else(Utc::now), timestamp: update_info.timestamp.unwrap_or_else(Utc::now),
current_primary_signature: update_info.current_signature.clone(), current_primary_signature: update_info.current_signature.clone(),
last_primary_signature: update_info.last_signature.clone(), last_primary_signature: update_info.last_signature.clone(),
} }
} }
pub fn has_device(&self, device_id: &String) -> bool {
self.device_ids.contains(device_id)
}
pub fn is_primary_device(&self, device_id: &String) -> bool {
self
.device_ids
.first()
.filter(|it| *it == device_id)
.is_some()
}
pub fn has_secondary_device(&self, device_id: &String) -> bool {
self.has_device(device_id) && !self.is_primary_device(device_id)
}
} }
// helper structs for converting to/from attribute values for sort key (a.k.a itemID) // helper structs for converting to/from attribute values for sort key (a.k.a itemID)
pub struct DeviceIDAttribute(pub String); pub struct DeviceIDAttribute(pub String);
struct DeviceListKeyAttribute(DateTime<Utc>); struct DeviceListKeyAttribute(DateTime<Utc>);
impl From<DeviceIDAttribute> for AttributeValue { impl From<DeviceIDAttribute> for AttributeValue {
fn from(value: DeviceIDAttribute) -> Self { fn from(value: DeviceIDAttribute) -> Self {
▲ Show 20 Lines • Show All 656 Lines • ▼ Show 20 Lines self
&content_one_time_keys, &content_one_time_keys,
&notif_one_time_keys, &notif_one_time_keys,
) )
.await?; .await?;
Ok(()) Ok(())
} }
/// Removes device data from devices table. If the device doesn't exist,
/// it is a no-op. This does not update the device list; the device ID
/// should be removed from the device list separately.
#[tracing::instrument(skip_all)]
pub async fn remove_device_data(
&self,
user_id: impl Into<String>,
device_id: impl Into<String>,
) -> Result<(), Error> {
let user_id = user_id.into();
let device_id = device_id.into();
self
.client
.delete_item()
.table_name(devices_table::NAME)
.key(ATTR_USER_ID, AttributeValue::S(user_id))
.key(ATTR_ITEM_ID, DeviceIDAttribute(device_id).into())
.send()
.await
.map_err(|e| {
error!(
errorType = error_types::DEVICE_LIST_DB_LOG,
"Failed to delete device data: {:?}", e
);
Error::AwsSdk(e.into())
})?;
Ok(())
}
/// Registers primary device for user, stores its signed device list /// Registers primary device for user, stores its signed device list
pub async fn register_primary_device( pub async fn register_primary_device(
&self, &self,
user_id: impl Into<String>, user_id: impl Into<String>,
device_key_upload: FlattenedDeviceKeyUpload, device_key_upload: FlattenedDeviceKeyUpload,
code_version: u64, code_version: u64,
login_time: DateTime<Utc>, login_time: DateTime<Utc>,
initial_device_list: DeviceListUpdate, initial_device_list: DeviceListUpdate,
▲ Show 20 Lines • Show All 793 Lines • Show Last 20 Lines
View Options

native/components/display-community-directory-prompt.react.js

Show First 20 Lines • Show All 138 Lines • ▼ Show 20 Lines ) -> Self {
Self { Self {
user_id: user_id.into(), user_id: user_id.into(),
device_ids, device_ids,
timestamp: update_info.timestamp.unwrap_or_else(Utc::now), timestamp: update_info.timestamp.unwrap_or_else(Utc::now),
current_primary_signature: update_info.current_signature.clone(), current_primary_signature: update_info.current_signature.clone(),
last_primary_signature: update_info.last_signature.clone(), last_primary_signature: update_info.last_signature.clone(),
} }
} }
pub fn has_device(&self, device_id: &String) -> bool {
self.device_ids.contains(device_id)
}
pub fn is_primary_device(&self, device_id: &String) -> bool {
self
.device_ids
.first()
.filter(|it| *it == device_id)
.is_some()
}
pub fn has_secondary_device(&self, device_id: &String) -> bool {
self.has_device(device_id) && !self.is_primary_device(device_id)
}
} }
// helper structs for converting to/from attribute values for sort key (a.k.a itemID) // helper structs for converting to/from attribute values for sort key (a.k.a itemID)
pub struct DeviceIDAttribute(pub String); pub struct DeviceIDAttribute(pub String);
struct DeviceListKeyAttribute(DateTime<Utc>); struct DeviceListKeyAttribute(DateTime<Utc>);
impl From<DeviceIDAttribute> for AttributeValue { impl From<DeviceIDAttribute> for AttributeValue {
fn from(value: DeviceIDAttribute) -> Self { fn from(value: DeviceIDAttribute) -> Self {
▲ Show 20 Lines • Show All 656 Lines • ▼ Show 20 Lines self
&content_one_time_keys, &content_one_time_keys,
&notif_one_time_keys, &notif_one_time_keys,
) )
.await?; .await?;
Ok(()) Ok(())
} }
/// Removes device data from devices table. If the device doesn't exist,
/// it is a no-op. This does not update the device list; the device ID
/// should be removed from the device list separately.
#[tracing::instrument(skip_all)]
pub async fn remove_device_data(
&self,
user_id: impl Into<String>,
device_id: impl Into<String>,
) -> Result<(), Error> {
let user_id = user_id.into();
let device_id = device_id.into();
self
.client
.delete_item()
.table_name(devices_table::NAME)
.key(ATTR_USER_ID, AttributeValue::S(user_id))
.key(ATTR_ITEM_ID, DeviceIDAttribute(device_id).into())
.send()
.await
.map_err(|e| {
error!(
errorType = error_types::DEVICE_LIST_DB_LOG,
"Failed to delete device data: {:?}", e
);
Error::AwsSdk(e.into())
})?;
Ok(())
}
/// Registers primary device for user, stores its signed device list /// Registers primary device for user, stores its signed device list
pub async fn register_primary_device( pub async fn register_primary_device(
&self, &self,
user_id: impl Into<String>, user_id: impl Into<String>,
device_key_upload: FlattenedDeviceKeyUpload, device_key_upload: FlattenedDeviceKeyUpload,
code_version: u64, code_version: u64,
login_time: DateTime<Utc>, login_time: DateTime<Utc>,
initial_device_list: DeviceListUpdate, initial_device_list: DeviceListUpdate,
▲ Show 20 Lines • Show All 793 Lines • Show Last 20 Lines
View Options

native/navigation/community-drawer-content.react.js

Show First 20 Lines • Show All 138 Lines • ▼ Show 20 Lines ) -> Self {
Self { Self {
user_id: user_id.into(), user_id: user_id.into(),
device_ids, device_ids,
timestamp: update_info.timestamp.unwrap_or_else(Utc::now), timestamp: update_info.timestamp.unwrap_or_else(Utc::now),
current_primary_signature: update_info.current_signature.clone(), current_primary_signature: update_info.current_signature.clone(),
last_primary_signature: update_info.last_signature.clone(), last_primary_signature: update_info.last_signature.clone(),
} }
} }
pub fn has_device(&self, device_id: &String) -> bool {
self.device_ids.contains(device_id)
}
pub fn is_primary_device(&self, device_id: &String) -> bool {
self
.device_ids
.first()
.filter(|it| *it == device_id)
.is_some()
}
pub fn has_secondary_device(&self, device_id: &String) -> bool {
self.has_device(device_id) && !self.is_primary_device(device_id)
}
} }
// helper structs for converting to/from attribute values for sort key (a.k.a itemID) // helper structs for converting to/from attribute values for sort key (a.k.a itemID)
pub struct DeviceIDAttribute(pub String); pub struct DeviceIDAttribute(pub String);
struct DeviceListKeyAttribute(DateTime<Utc>); struct DeviceListKeyAttribute(DateTime<Utc>);
impl From<DeviceIDAttribute> for AttributeValue { impl From<DeviceIDAttribute> for AttributeValue {
fn from(value: DeviceIDAttribute) -> Self { fn from(value: DeviceIDAttribute) -> Self {
▲ Show 20 Lines • Show All 656 Lines • ▼ Show 20 Lines self
&content_one_time_keys, &content_one_time_keys,
&notif_one_time_keys, &notif_one_time_keys,
) )
.await?; .await?;
Ok(()) Ok(())
} }
/// Removes device data from devices table. If the device doesn't exist,
/// it is a no-op. This does not update the device list; the device ID
/// should be removed from the device list separately.
#[tracing::instrument(skip_all)]
pub async fn remove_device_data(
&self,
user_id: impl Into<String>,
device_id: impl Into<String>,
) -> Result<(), Error> {
let user_id = user_id.into();
let device_id = device_id.into();
self
.client
.delete_item()
.table_name(devices_table::NAME)
.key(ATTR_USER_ID, AttributeValue::S(user_id))
.key(ATTR_ITEM_ID, DeviceIDAttribute(device_id).into())
.send()
.await
.map_err(|e| {
error!(
errorType = error_types::DEVICE_LIST_DB_LOG,
"Failed to delete device data: {:?}", e
);
Error::AwsSdk(e.into())
})?;
Ok(())
}
/// Registers primary device for user, stores its signed device list /// Registers primary device for user, stores its signed device list
pub async fn register_primary_device( pub async fn register_primary_device(
&self, &self,
user_id: impl Into<String>, user_id: impl Into<String>,
device_key_upload: FlattenedDeviceKeyUpload, device_key_upload: FlattenedDeviceKeyUpload,
code_version: u64, code_version: u64,
login_time: DateTime<Utc>, login_time: DateTime<Utc>,
initial_device_list: DeviceListUpdate, initial_device_list: DeviceListUpdate,
▲ Show 20 Lines • Show All 793 Lines • Show Last 20 Lines
View Options

native/navigation/root-navigator.react.js

Show First 20 Lines • Show All 138 Lines • ▼ Show 20 Lines ) -> Self {
Self { Self {
user_id: user_id.into(), user_id: user_id.into(),
device_ids, device_ids,
timestamp: update_info.timestamp.unwrap_or_else(Utc::now), timestamp: update_info.timestamp.unwrap_or_else(Utc::now),
current_primary_signature: update_info.current_signature.clone(), current_primary_signature: update_info.current_signature.clone(),
last_primary_signature: update_info.last_signature.clone(), last_primary_signature: update_info.last_signature.clone(),
} }
} }
pub fn has_device(&self, device_id: &String) -> bool {
self.device_ids.contains(device_id)
}
pub fn is_primary_device(&self, device_id: &String) -> bool {
self
.device_ids
.first()
.filter(|it| *it == device_id)
.is_some()
}
pub fn has_secondary_device(&self, device_id: &String) -> bool {
self.has_device(device_id) && !self.is_primary_device(device_id)
}
} }
// helper structs for converting to/from attribute values for sort key (a.k.a itemID) // helper structs for converting to/from attribute values for sort key (a.k.a itemID)
pub struct DeviceIDAttribute(pub String); pub struct DeviceIDAttribute(pub String);
struct DeviceListKeyAttribute(DateTime<Utc>); struct DeviceListKeyAttribute(DateTime<Utc>);
impl From<DeviceIDAttribute> for AttributeValue { impl From<DeviceIDAttribute> for AttributeValue {
fn from(value: DeviceIDAttribute) -> Self { fn from(value: DeviceIDAttribute) -> Self {
▲ Show 20 Lines • Show All 656 Lines • ▼ Show 20 Lines self
&content_one_time_keys, &content_one_time_keys,
&notif_one_time_keys, &notif_one_time_keys,
) )
.await?; .await?;
Ok(()) Ok(())
} }
/// Removes device data from devices table. If the device doesn't exist,
/// it is a no-op. This does not update the device list; the device ID
/// should be removed from the device list separately.
#[tracing::instrument(skip_all)]
pub async fn remove_device_data(
&self,
user_id: impl Into<String>,
device_id: impl Into<String>,
) -> Result<(), Error> {
let user_id = user_id.into();
let device_id = device_id.into();
self
.client
.delete_item()
.table_name(devices_table::NAME)
.key(ATTR_USER_ID, AttributeValue::S(user_id))
.key(ATTR_ITEM_ID, DeviceIDAttribute(device_id).into())
.send()
.await
.map_err(|e| {
error!(
errorType = error_types::DEVICE_LIST_DB_LOG,
"Failed to delete device data: {:?}", e
);
Error::AwsSdk(e.into())
})?;
Ok(())
}
/// Registers primary device for user, stores its signed device list /// Registers primary device for user, stores its signed device list
pub async fn register_primary_device( pub async fn register_primary_device(
&self, &self,
user_id: impl Into<String>, user_id: impl Into<String>,
device_key_upload: FlattenedDeviceKeyUpload, device_key_upload: FlattenedDeviceKeyUpload,
code_version: u64, code_version: u64,
login_time: DateTime<Utc>, login_time: DateTime<Utc>,
initial_device_list: DeviceListUpdate, initial_device_list: DeviceListUpdate,
▲ Show 20 Lines • Show All 793 Lines • Show Last 20 Lines
View Options

native/root.react.js

Loading...
View Options

native/utils/directory-utils.js

Loading...