diff --git a/CLAUDE.md b/CLAUDE.md index ce1f35d..cd25543 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,6 +84,7 @@ Post-install events (signups, purchases, deposits) flow through a **sources** ab - Use `filter_map` to combine filtering and transformation - Flatten with `?` operator, `.ok()`, `.and_then()` chains - Use `let-else` for early returns +- Three or more `if let` statements in a row applying parallel logic is a smell — there's almost always a flatter form: a struct literal (when each branch sets one field of the same struct), a fluent builder with `Option` setters (when callers need to propagate optionality and you'd value validation/encapsulation/required-field enforcement at the constructor — see `CreateLinkInput`), `.map()` / `.and_then()` (when transforming `Option` → `Option`), `filter_map` (when conditionally building a `Vec`), or `match` (when conditions overlap). `if let` is for *one* conditional unwrap with a side effect, not a substitute for declarative construction. **Builder design note:** if you write a builder, make the setters take `Option` (not `T`) — taking `T` forces every caller into an `if let` chain and reintroduces the smell at every call site. - All route handlers must have `#[tracing::instrument]` for Sentry visibility - `ErrorResponse` lives in `error.rs` and is shared across all slices diff --git a/server/src/api/links/routes.rs b/server/src/api/links/routes.rs index 24a9176..38ac471 100644 --- a/server/src/api/links/routes.rs +++ b/server/src/api/links/routes.rs @@ -1477,28 +1477,31 @@ fn render_smart_landing_page(ctx: &LandingPageContext) -> String { .map(action_to_schema_type) .unwrap_or("ViewAction"); - let mut entry_points = Vec::new(); - if let Some(dl) = &ctx.link.ios_deep_link { - entry_points.push(json!({ - "@type": "EntryPoint", - "urlTemplate": dl, - "actionPlatform": "http://schema.org/IOSPlatform" - })); - } - if let Some(dl) = &ctx.link.android_deep_link { - entry_points.push(json!({ - "@type": "EntryPoint", - "urlTemplate": dl, - "actionPlatform": "http://schema.org/AndroidPlatform" - })); - } - if let Some(url) = &ctx.link.web_url { - entry_points.push(json!({ - "@type": "EntryPoint", - "urlTemplate": url, - "actionPlatform": "http://schema.org/DesktopWebPlatform" - })); - } + let entry_points: Vec<_> = [ + ( + ctx.link.ios_deep_link.as_deref(), + "http://schema.org/IOSPlatform", + ), + ( + ctx.link.android_deep_link.as_deref(), + "http://schema.org/AndroidPlatform", + ), + ( + ctx.link.web_url.as_deref(), + "http://schema.org/DesktopWebPlatform", + ), + ] + .into_iter() + .filter_map(|(opt, platform)| { + opt.map(|url| { + json!({ + "@type": "EntryPoint", + "urlTemplate": url, + "actionPlatform": platform, + }) + }) + }) + .collect(); let mut action = json!({ "@context": "https://schema.org", @@ -2135,22 +2138,16 @@ fn build_agent_panel(ctx: &LandingPageContext) -> String { } // Destinations - let mut dests = Vec::new(); - if let Some(v) = &link.ios_deep_link { - dests.push(("iOS", v.as_str())); - } - if let Some(v) = &link.android_deep_link { - dests.push(("Android", v.as_str())); - } - if let Some(v) = &link.web_url { - dests.push(("Web", v.as_str())); - } - if let Some(v) = &link.ios_store_url { - dests.push(("App Store", v.as_str())); - } - if let Some(v) = &link.android_store_url { - dests.push(("Play Store", v.as_str())); - } + let dests: Vec<(&str, &str)> = [ + ("iOS", link.ios_deep_link.as_deref()), + ("Android", link.android_deep_link.as_deref()), + ("Web", link.web_url.as_deref()), + ("App Store", link.ios_store_url.as_deref()), + ("Play Store", link.android_store_url.as_deref()), + ] + .into_iter() + .filter_map(|(label, opt)| opt.map(|v| (label, v))) + .collect(); if !dests.is_empty() { html.push_str(r#"
Destinations
"#); for (label, url) in &dests { diff --git a/server/src/services/auth/tenants/repo.rs b/server/src/services/auth/tenants/repo.rs index ee1ab1b..ef4d885 100644 --- a/server/src/services/auth/tenants/repo.rs +++ b/server/src/services/auth/tenants/repo.rs @@ -205,6 +205,9 @@ impl TenantsRepository for TenantsRepo { update: SubscriptionUpdate, ) -> Result { let mut set_doc = mongodb::bson::Document::new(); + + // Fallible bson encoding — kept as separate statements so `?` lifts + // the error inline. These three need it; the rest don't. if let Some(tier) = update.plan_tier { set_doc.insert( "plan_tier", @@ -220,18 +223,31 @@ impl TenantsRepository for TenantsRepo { if let Some(status) = update.status { set_doc.insert("status", bson::to_bson(&status).map_err(|e| e.to_string())?); } - if let Some(start) = update.current_period_start { - set_doc.insert("current_period_start", start); - } - if let Some(end) = update.current_period_end { - set_doc.insert("current_period_end", end); - } - if let Some(cust) = update.stripe_customer_id { - set_doc.insert("stripe_customer_id", cust); - } - if let Some(sub) = update.stripe_subscription_id { - set_doc.insert("stripe_subscription_id", sub); - } + + // Infallible fields fold into one filter_map. + use mongodb::bson::Bson; + set_doc.extend( + [ + ( + "current_period_start", + update.current_period_start.map(Bson::DateTime), + ), + ( + "current_period_end", + update.current_period_end.map(Bson::DateTime), + ), + ( + "stripe_customer_id", + update.stripe_customer_id.map(Bson::String), + ), + ( + "stripe_subscription_id", + update.stripe_subscription_id.map(Bson::String), + ), + ] + .into_iter() + .filter_map(|(k, v)| v.map(|b| (k.to_string(), b))), + ); if set_doc.is_empty() { return Ok(true); } diff --git a/server/src/services/links/models.rs b/server/src/services/links/models.rs index b7e4a00..776f738 100644 --- a/server/src/services/links/models.rs +++ b/server/src/services/links/models.rs @@ -158,6 +158,16 @@ pub struct CreateLinkInput { pub social_preview: Option, } +/// Fluent builder for `CreateLinkInput`. Setters accept `Option` directly +/// so callers can propagate optionality from a request struct without +/// `if let` chains at the call site: +/// +/// ```ignore +/// CreateLinkInput::new(tenant_id, link_id) +/// .web_url(req.web_url) +/// .ios_deep_link(req.ios_deep_link) +/// .metadata(metadata_doc) +/// ``` impl CreateLinkInput { pub fn new(tenant_id: ObjectId, link_id: String) -> Self { Self { @@ -176,53 +186,53 @@ impl CreateLinkInput { } } - pub fn affiliate_id(mut self, v: ObjectId) -> Self { - self.affiliate_id = Some(v); + pub fn ios_deep_link(mut self, v: Option) -> Self { + self.ios_deep_link = v; self } - pub fn expires_at(mut self, v: DateTime) -> Self { - self.expires_at = Some(v); + pub fn android_deep_link(mut self, v: Option) -> Self { + self.android_deep_link = v; self } - pub fn ios_deep_link(mut self, v: impl Into) -> Self { - self.ios_deep_link = Some(v.into()); + pub fn web_url(mut self, v: Option) -> Self { + self.web_url = v; self } - pub fn android_deep_link(mut self, v: impl Into) -> Self { - self.android_deep_link = Some(v.into()); + pub fn ios_store_url(mut self, v: Option) -> Self { + self.ios_store_url = v; self } - pub fn web_url(mut self, v: impl Into) -> Self { - self.web_url = Some(v.into()); + pub fn android_store_url(mut self, v: Option) -> Self { + self.android_store_url = v; self } - pub fn ios_store_url(mut self, v: impl Into) -> Self { - self.ios_store_url = Some(v.into()); + pub fn metadata(mut self, v: Option) -> Self { + self.metadata = v; self } - pub fn android_store_url(mut self, v: impl Into) -> Self { - self.android_store_url = Some(v.into()); + pub fn affiliate_id(mut self, v: Option) -> Self { + self.affiliate_id = v; self } - pub fn metadata(mut self, v: Document) -> Self { - self.metadata = Some(v); + pub fn expires_at(mut self, v: Option) -> Self { + self.expires_at = v; self } - pub fn agent_context(mut self, v: AgentContext) -> Self { - self.agent_context = Some(v); + pub fn agent_context(mut self, v: Option) -> Self { + self.agent_context = v; self } - pub fn social_preview(mut self, v: SocialPreview) -> Self { - self.social_preview = Some(v); + pub fn social_preview(mut self, v: Option) -> Self { + self.social_preview = v; self } } diff --git a/server/src/services/links/service.rs b/server/src/services/links/service.rs index 627f3c4..040fb6c 100644 --- a/server/src/services/links/service.rs +++ b/server/src/services/links/service.rs @@ -252,44 +252,24 @@ impl LinksService { .metadata .and_then(|v| mongodb::bson::to_document(&v).ok()); - let mut input = CreateLinkInput::new(tenant_id, link_id.clone()); - if let Some(v) = req.ios_deep_link { - input = input.ios_deep_link(v); - } - if let Some(v) = req.android_deep_link { - input = input.android_deep_link(v); - } - if let Some(v) = req.web_url { - input = input.web_url(v); - } - if let Some(v) = req.ios_store_url { - input = input.ios_store_url(v); - } - if let Some(v) = req.android_store_url { - input = input.android_store_url(v); - } - if let Some(v) = metadata { - input = input.metadata(v); - } - if let Some(aff) = resolved_affiliate_id { - input = input.affiliate_id(aff); - } - if let Some(ac) = req.agent_context { - input = input.agent_context(ac); - } - if let Some(sp) = req.social_preview { - input = input.social_preview(sp); - } - // Links without a verified custom domain expire after 30 days. - let expires_at = if !has_verified_domain { + let expires_at_dt = (!has_verified_domain).then(|| { let thirty_days_ms = 30 * 24 * 60 * 60 * 1000_i64; - let expiry = DateTime::from_millis(DateTime::now().timestamp_millis() + thirty_days_ms); - input = input.expires_at(expiry); - Some(expiry.try_to_rfc3339_string().unwrap_or_default()) - } else { - None - }; + DateTime::from_millis(DateTime::now().timestamp_millis() + thirty_days_ms) + }); + let expires_at = expires_at_dt.map(|dt| dt.try_to_rfc3339_string().unwrap_or_default()); + + let input = CreateLinkInput::new(tenant_id, link_id.clone()) + .ios_deep_link(req.ios_deep_link) + .android_deep_link(req.android_deep_link) + .web_url(req.web_url) + .ios_store_url(req.ios_store_url) + .android_store_url(req.android_store_url) + .metadata(metadata) + .affiliate_id(resolved_affiliate_id) + .expires_at(expires_at_dt) + .agent_context(req.agent_context) + .social_preview(req.social_preview); self.links_repo.create_link(input).await.map_err(|e| { if e.contains("E11000") { @@ -431,30 +411,35 @@ impl LinksService { } } - if let Some(v) = &req.web_url { - update.insert("web_url", v.clone()); - } - if let Some(v) = &req.ios_store_url { - update.insert("ios_store_url", v.clone()); - } - if let Some(v) = &req.android_store_url { - update.insert("android_store_url", v.clone()); - } - if let Some(v) = &req.metadata { - if let Ok(doc) = mongodb::bson::to_document(v) { - update.insert("metadata", doc); - } - } - if let Some(ref ac) = req.agent_context { - if let Ok(doc) = mongodb::bson::to_document(ac) { - update.insert("agent_context", doc); - } - } - if let Some(ref sp) = req.social_preview { - if let Ok(doc) = mongodb::bson::to_document(sp) { - update.insert("social_preview", doc); - } - } + // String fields and serializable structs share the same shape: + // "if Some, insert into the $set doc". Flatten both into one filter_map + // chain so the parallel branches collapse to data + a single insert. + use mongodb::bson::Bson; + let string_fields = [ + ("web_url", req.web_url.as_ref()), + ("ios_store_url", req.ios_store_url.as_ref()), + ("android_store_url", req.android_store_url.as_ref()), + ] + .into_iter() + .filter_map(|(k, v)| v.map(|s| (k.to_string(), Bson::String(s.clone())))); + + // bson::to_document errors silently drop the field — preserves the + // pre-refactor behavior. Validation has already gated bad input. + let doc_fields = [ + ("metadata", req.metadata.as_ref().and_then(to_doc_value)), + ( + "agent_context", + req.agent_context.as_ref().and_then(to_doc_value), + ), + ( + "social_preview", + req.social_preview.as_ref().and_then(to_doc_value), + ), + ] + .into_iter() + .filter_map(|(k, v)| v.map(|d| (k.to_string(), Bson::Document(d)))); + + update.extend(string_fields.chain(doc_fields)); if update.is_empty() && unset.is_empty() { return Err(LinkError::EmptyUpdate); @@ -606,6 +591,13 @@ impl LinksService { } } +/// Serialize any `Serialize` value to a Bson `Document`, dropping the value +/// silently on failure. Used by `update_link` to fold optional struct fields +/// (metadata, agent_context, social_preview) into the `$set` payload. +fn to_doc_value(v: &T) -> Option { + mongodb::bson::to_document(v).ok() +} + pub fn build_canonical_link_url( public_url: &str, link_id: &str,