Skip to content

Flatten parallel if-let chains; codify the rule in CLAUDE.md #113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saltyskip merged 2 commits into from
May 1, 2026
Merged

Flatten parallel if-let chains; codify the rule in CLAUDE.md #113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
adf50f4
Flatten parallel if-let chains and codify the rule in CLAUDE.md
saltyskip May 1, 2026
fa373ea
Reshape CreateLinkInput builder to take Option<T>; use fluent style i…
saltyskip May 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>` 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<A>` → `Option<B>`), `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<T>` (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

Expand Down
73 changes: 35 additions & 38 deletions server/src/api/links/routes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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#"<div class="dest-section"><div class="dest-header">Destinations</div>"#);
for (label, url) in &dests {
Expand Down
40 changes: 28 additions & 12 deletions server/src/services/auth/tenants/repo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,9 @@ impl TenantsRepository for TenantsRepo {
update: SubscriptionUpdate,
) -> Result<bool, String> {
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",
Expand All @@ -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);
}
Expand Down
50 changes: 30 additions & 20 deletions server/src/services/links/models.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,16 @@ pub struct CreateLinkInput {
pub social_preview: Option<SocialPreview>,
}

/// Fluent builder for `CreateLinkInput`. Setters accept `Option<T>` 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 {
Expand All @@ -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<String>) -> 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<String>) -> Self {
self.android_deep_link = v;
self
}

pub fn ios_deep_link(mut self, v: impl Into<String>) -> Self {
self.ios_deep_link = Some(v.into());
pub fn web_url(mut self, v: Option<String>) -> Self {
self.web_url = v;
self
}

pub fn android_deep_link(mut self, v: impl Into<String>) -> Self {
self.android_deep_link = Some(v.into());
pub fn ios_store_url(mut self, v: Option<String>) -> Self {
self.ios_store_url = v;
self
}

pub fn web_url(mut self, v: impl Into<String>) -> Self {
self.web_url = Some(v.into());
pub fn android_store_url(mut self, v: Option<String>) -> Self {
self.android_store_url = v;
self
}

pub fn ios_store_url(mut self, v: impl Into<String>) -> Self {
self.ios_store_url = Some(v.into());
pub fn metadata(mut self, v: Option<Document>) -> Self {
self.metadata = v;
self
}

pub fn android_store_url(mut self, v: impl Into<String>) -> Self {
self.android_store_url = Some(v.into());
pub fn affiliate_id(mut self, v: Option<ObjectId>) -> 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<DateTime>) -> 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<AgentContext>) -> 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<SocialPreview>) -> Self {
self.social_preview = v;
self
}
}
Expand Down
112 changes: 52 additions & 60 deletions server/src/services/links/service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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<T: serde::Serialize>(v: &T) -> Option<mongodb::bson::Document> {
mongodb::bson::to_document(v).ok()
}

pub fn build_canonical_link_url(
public_url: &str,
link_id: &str,
Expand Down
Loading