feat: activity places filter

[escapedcat]

Relates to this FE draft: teambtcmap/btcmap.org#928

What

Adds a ?places=id1,id2,... query param to GET /v4/activity, alongside
the existing ?area= / ?areas= filters. Place IDs union into the
element-filter HashSet so area+place overlap (e.g. saved country + a
place inside it) dedupes to one event.

Why

The FE is building /user/activity — a combined activity feed across a
signed-in user's saved places and saved areas. One request, server-side
merge + dedup, stateless/cacheable (no /users/me/activity needed).

Validation & limits

• places values must parse as i64 → otherwise 400 Invalid Input.
• days clamped to 1..=3650 → otherwise 400 (also protects the
  pre-existing unauthenticated global fetch path).
• places capped at 500 unique IDs.

Tests

cargo test activity → 16 passing, incl. new cases for:

• ?places=... only (pure places filter)
• ?areas=A&places=X where X is inside A → deduped to 1 event
• ?areas=A&places=X where X is outside A → both included
• Invalid places, out-of-range days, oversized places list → 400

Notes

No SQL changes, no new queries. The handler falls back from the per-area
optimized fetch to a global fetch + post-filter whenever places is
present; existing area-only callers are unchanged.

Summary by CodeRabbit

• New Features

  • Added support for filtering activities by specific places. Users can now query activities using comma-separated place identifiers in combination with existing area-based filters.

• Bug Fixes

  • Enhanced parameter validation with stricter input checks and improved error responses for invalid or out-of-range values.

[coderabbitai]

Warning

Rate limit exceeded

@escapedcat has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 32 minutes and 5 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 32 minutes and 5 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 228d2bf2-683c-4197-9d39-92ef3d8c83fd

📥 Commits

Reviewing files that changed from the base of the PR and between 3c863bd and 4c565e3.

📒 Files selected for processing (3)

• docs/rest/v4/README.md
• docs/rest/v4/activity.md
• src/rest/v4/activity.rs

📝 Walkthrough

Walkthrough

Added places parameter to the activity endpoint for filtering events by specific locations. Implemented validation for days (within 1 to MAX_DAYS) and places (comma-separated integers up to MAX_PLACES). Refactored filtering logic to use a generalized in_filter closure applied post-fetch. Updated database query optimization to use area-scoped queries only when appropriate.

Changes

Cohort / File(s)	Summary
Request Validation & Input Parsing src/rest/v4/activity.rs	Added places: Option<String> field to GetActivityArgs. Implemented validation for days parameter (1..=MAX_DAYS) and places parameter (comma-separated integer IDs, max MAX_PLACES entries). Returns RestApiError::invalid_input (HTTP 400) for validation failures.
Filtering & Query Logic src/rest/v4/activity.rs	Refactored filter logic from area-specific closure to generalized in_filter applied to events, comments, and boosts post-fetch. Updated control flow so area-optimized database queries execute only when areas is non-empty and places is empty; otherwise fetches globally with post-filtering.
Test Coverage src/rest/v4/activity.rs	Added test cases for place-based filtering, combined areas + places filtering with deduplication, and validation error scenarios (invalid days, invalid places, excessive place count).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feature(area): acitvity feed json #79 — Extends the same activity handler with place-based filtering and adjusts area-optimized database query execution logic.

Suggested reviewers

• bubelov

Poem

🐰 A rabbit hops through places new,
Filtering by location too,
With validation keeping all in check,
And queries optimized—what a heck!
Places parsed and tests all true,
Activity endpoints, we've made them new! 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change: adding a places filter to the activity endpoint, which aligns with the primary modification in the changeset.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/activity-places-filter

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/rest/v4/activity.rs (1)

162-233: Global fetch + post-filter can scale poorly when places is specified.

Whenever places is non-empty (with or without areas), both the event and comment paths call select_created_between(day_ago, period_end, ...) and then filter in Rust via in_filter. For a busy deployment with many events/comments in the day window, this loads the full unfiltered result set into memory on every request that uses a narrow places filter — exactly the case where a targeted query would return very few rows.

Also, the inline comment on Lines 162/215 lists three behaviors ("area-scoped (optimized), global, or global + post-filter") but the if/else only has two branches; the distinction between plain global and "global + post-filter" is implicit in whether elements is Some.

Consider adding a select_created_between_for_element_ids(element_ids, from, to, pool) query (per the blocking_queries layer convention) and routing the places-only and areas + places cases through a push-down filter instead of a global scan. Not a blocker for correctness — the current logic is correct and tests validate it — but worth an issue to track before this endpoint sees heavy use.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/rest/v4/activity.rs` around lines 162 - 233, The current logic fetches
all events/comments via select_created_between when any places are provided and
then post-filters with in_filter, which can OOM/scale poorly; add new
pushed-down queries (e.g.,
db::main::element_event::queries::select_created_between_for_element_ids and
db::main::element_comment::queries::select_created_between_for_element_ids
following the blocking_queries convention) and route the code paths in
activity.rs so that when places (or areas+places) is non-empty you call those
element_id-scoped queries instead of select_created_between, keeping the
existing area-scoped branch that uses select_created_between_for_area and
preserving result dedup/collection logic.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/rest/v4/activity.rs`:
- Around line 116-133: The current code parses the entire comma-separated string
into a HashSet before enforcing MAX_PLACES, allowing huge inputs to allocate
memory; instead, first count tokens in args.places (e.g., let token_count =
comma_separated_places.split(',').filter(|s| !s.trim().is_empty()).count()) and
immediately return RestApiError::invalid_input if token_count > MAX_PLACES to
cap allocation, then proceed to parse and collect into the HashSet<i64> (the
places variable) as before; keep using the same error message via
RestApiError::invalid_input and the same symbols args.places, places, and
MAX_PLACES to locate where to change the logic.

---

Nitpick comments:
In `@src/rest/v4/activity.rs`:
- Around line 162-233: The current logic fetches all events/comments via
select_created_between when any places are provided and then post-filters with
in_filter, which can OOM/scale poorly; add new pushed-down queries (e.g.,
db::main::element_event::queries::select_created_between_for_element_ids and
db::main::element_comment::queries::select_created_between_for_element_ids
following the blocking_queries convention) and route the code paths in
activity.rs so that when places (or areas+places) is non-empty you call those
element_id-scoped queries instead of select_created_between, keeping the
existing area-scoped branch that uses select_created_between_for_area and
preserving result dedup/collection logic.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c94f9a25-b1b2-4afe-8d9b-76172b1c2b83

📥 Commits

Reviewing files that changed from the base of the PR and between e36979b and 98f7232.

📒 Files selected for processing (1)

• src/rest/v4/activity.rs

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/rest/v4/activity.rs (1)

163-181: ⚠️ Potential issue | 🟠 Major

Scalability concern: global fetch + in-memory filter for places.

When places is provided (with or without areas), this path calls select_created_between(day_ago, period_end, ..) which pulls every event in the window across the whole DB, then discards all but the matching element_ids in Rust. The same pattern repeats for comments at lines 216–234. For a small set of places (often just 1–10 IDs) over a 10‑year days window, this is O(total_events) per request and will degrade as the table grows — especially unfortunate given the area path already has an optimized per-area query.

Consider adding a select_created_between_for_elements(ids, from, to, ..) query in blocking_queries/queries (SQL WHERE element_id IN (...) or a bulk fetch) and using it when places is non-empty; union the results with the per-area fetch when both are supplied. This keeps both filter axes on the DB side and preserves the dedupe semantics the PR is after.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/rest/v4/activity.rs` around lines 163 - 181, The current code pulls all
events via select_created_between when any places are provided, causing
O(total_events) work; add a new DB query (e.g.,
element_event::queries::select_created_between_for_elements(ids, from, to,
&pool)) that does a WHERE element_id IN (...) (implement in
blocking_queries/queries SQL), then in src/rest/v4/activity.rs call that
function when places is non-empty (and still call the per-area
select_created_between_for_area for each area when areas is also supplied),
merge/union the results (use the existing HashSet<ElementEvent> dedupe pattern)
and map DB errors to RestApiError::database() as before; apply the same change
for the comments path (the select_created_between usage referenced around lines
216–234) using a corresponding comment query.

🧹 Nitpick comments (1)

src/rest/v4/activity.rs (1)

635-829: Good test coverage for the new surface.

Places-only, area+place inside (dedupe), area+place outside (union), oversized places list, out-of-range days, and invalid places tokens are all covered. One small gap: there's no test asserting that supplying places still returns comments and boosts (only events are exercised via places); worth adding if you want to lock in the comment/boost in_filter paths against regression.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/rest/v4/activity.rs` around lines 635 - 829, Add a test that mirrors
get_filtered_by_places but also inserts a comment and a boost tied to one of the
places (use db::main::comment::queries::insert and
db::main::boost::queries::insert or the equivalent helpers), call the handler
via super::get with the places query, and assert the returned
Vec<super::ActivityItem> contains items for those comment and boost records
(check identifying ActivityItem fields like comment_id/boost_id or kind) to
ensure comments and boosts follow the same places filtering/in_filter path as
element events.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/rest/v4/activity.rs`:
- Around line 116-134: Parse and deduplicate tokens before enforcing MAX_PLACES:
in the block handling args.places (the computation of places), first split and
map trimming, filter out empty/whitespace tokens (so trailing commas or
duplicated separators are ignored), parse remaining tokens to i64, collect into
a Result<HashSet<_>, _>, then check the HashSet.len() against MAX_PLACES and
return RestApiError::invalid_input with the existing "places must contain at
most {MAX_PLACES} IDs" message if exceeded; keep the existing parse-error
message for non-integer tokens and ensure None still yields an empty HashSet.

---

Outside diff comments:
In `@src/rest/v4/activity.rs`:
- Around line 163-181: The current code pulls all events via
select_created_between when any places are provided, causing O(total_events)
work; add a new DB query (e.g.,
element_event::queries::select_created_between_for_elements(ids, from, to,
&pool)) that does a WHERE element_id IN (...) (implement in
blocking_queries/queries SQL), then in src/rest/v4/activity.rs call that
function when places is non-empty (and still call the per-area
select_created_between_for_area for each area when areas is also supplied),
merge/union the results (use the existing HashSet<ElementEvent> dedupe pattern)
and map DB errors to RestApiError::database() as before; apply the same change
for the comments path (the select_created_between usage referenced around lines
216–234) using a corresponding comment query.

---

Nitpick comments:
In `@src/rest/v4/activity.rs`:
- Around line 635-829: Add a test that mirrors get_filtered_by_places but also
inserts a comment and a boost tied to one of the places (use
db::main::comment::queries::insert and db::main::boost::queries::insert or the
equivalent helpers), call the handler via super::get with the places query, and
assert the returned Vec<super::ActivityItem> contains items for those comment
and boost records (check identifying ActivityItem fields like
comment_id/boost_id or kind) to ensure comments and boosts follow the same
places filtering/in_filter path as element events.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7bba4b31-d062-4323-90b9-5d438219c932

📥 Commits

Reviewing files that changed from the base of the PR and between 98f7232 and 3c863bd.

📒 Files selected for processing (1)

• src/rest/v4/activity.rs