feat(v4): add area-scoped top-editors endpoint

[escapedcat]

Related to: teambtcmap/btcmap.org#927

Desc

GET /v4/areas/{id}/top-editors?period_start=&period_end=&limit=

Returns the same TopEditor shape as GET /v4/top-editors, but counts only events whose target element belongs to the given area (joined through area_element). The {id} segment accepts either the numeric area id or the string alias (handled by select_by_id_or_alias, same as v3).

Why: the btcmap.org Activity tab on country/community pages currently derives its "Supertaggers" list client-side by fetching osm_id for every place in the area (one /v4/places/{id} per place — ~7940 requests per visit on the US page) and matching against the global events feed. With this endpoint the FE can replace all that with a single REST call.

Notes for the maintainer:

• period_start / period_end are optional here (default to an open range via UNIX_EPOCH .. 2200-01-01). The FE has no period filter in its current Supertaggers UI; nothing prevents callers from supplying bounds though, and the format/parsing matches /v4/top-editors.
• The bot/spam blocklist (EXCLUDED_USER_IDS) is now declared once in top_editors.rs and shared, so the two views can't drift apart.
• area_element rows with deleted_at IS NOT NULL are excluded from the join — matches the convention in get_by_id_areas.

Tests:

• select_most_active_for_area unit test covers area scoping (out-of-area events excluded), excluded_ids filter, and unknown-area returning 0.
• Three handler tests cover unknown-area 404, end-to-end scoping, and the limit query param.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• New Features

  • Introduced a new /top-editors API endpoint to view the most active editors within a specific area, with optional date range filtering and configurable result limits.

• Improvements

  • Added validation for API parameters, including area ID length checks and editor count limits (1–1000 range).
  • Enhanced performance through optimization of spam/bot filtering mechanisms.

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes add area-scoped filtering to user activity queries by introducing a new select_most_active_for_area database function that joins with an area element table, exposing it through async wrappers and a corresponding REST v4 endpoint (GET {id}/top-editors). Supporting helper functions for validation and date parsing are also centralized and refactored.

Changes

Cohort / File(s)	Summary
Database Layer – Area-Scoped Queries src/db/main/osm_user/blocking_queries.rs, src/db/main/osm_user/queries.rs	Refactored select_most_active to use dynamic SQL parameter binding via ToSql trait. Added new select_most_active_for_area function with area join filter and async wrapper; both handle dynamic NOT IN (...) exclusions.
REST Endpoint – Area Top Editors src/rest/v4/areas.rs	Added new GET {id}/top-editors endpoint with optional period_start, period_end, limit query parameters. Added input validation for id length (max 128 chars) to both new and existing get_by_id endpoint. Includes 3 integration tests.
Helper Functions & Shared Utilities src/rest/v4/top_editors.rs	Centralized bot/spam filtering via EXCLUDED_USER_IDS constant. Promoted parse_date, validate_limit, far_future, and extract_tip_url to pub(crate) visibility. Optimized extract_tip_url with cached regex via OnceLock. Added validate_limit with range enforcement (1..=1000).
Route Registration src/main.rs	Registered new Actix-Web endpoint rest::v4::areas::get_by_id_top_editors in v4 areas scope.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Handler as HTTP Handler<br/>(get_by_id_top_editors)
    participant Pool as Connection Pool
    participant BlockingQueries as Blocking Queries
    participant Database

    Client->>Handler: GET /areas/{id}/top-editors?period_start=...&limit=...
    Handler->>Handler: Parse & validate query params
    Handler->>Handler: Look up area by id/alias
    Handler->>Pool: Acquire database connection
    Pool-->>BlockingQueries: Connection
    BlockingQueries->>BlockingQueries: Build dynamic SQL params<br/>(excluded_ids NOT IN (...))
    BlockingQueries->>Database: SELECT users with area join<br/>WHERE area_id=? AND deleted_at IS NULL<br/>ORDER BY edits DESC LIMIT ?
    Database-->>BlockingQueries: Query results
    BlockingQueries->>BlockingQueries: Map to Vec<SelectMostActive>
    BlockingQueries-->>Handler: Results
    Handler->>Handler: Map to Vec<TopEditor><br/>(extract tip_url)
    Handler-->>Client: JSON response

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feature(area): acitvity feed json #79: Follows the same pattern of adding area-scoped database query functions with async wrappers and extending REST endpoints to use those area-filtered queries.

Suggested reviewers

• bubelov

Poem

🐰 Hops of joy through areas new,
Top editors sorted by their view,
Queries dance with joins so grand,
Helpers shared across the land! 🌿✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 76.19% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately describes the main change: adding a new area-scoped endpoint for viewing top editors, which is the core objective of this changeset.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/area-top-editors-rest

---

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]

🧹 Nitpick comments (4)

src/rest/v4/areas.rs (3)

474-501: Optional: also assert the lower-bound validation path.

The handler now returns 400 for limit<1 or limit>1000 via validate_limit, but there's no test covering that branch. A one-liner like requesting ?limit=0 and asserting 400 would lock in the contract and prevent regressions when the range is changed.

🤖 Prompt for AI Agents

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

In `@src/rest/v4/areas.rs` around lines 474 - 501, Add a test case exercising the
lower-bound validation for the top editors endpoint: in the existing test module
(where top_editors_for_area_respects_limit is defined) add a new async test that
initializes the same App with get_by_id_top_editors and sends a GET to
"/{area.id}/top-editors?limit=0", then assert the response status is 400; this
targets the validate_limit path invoked by get_by_id_top_editors to lock in the
contract for limit<1.

---

112-114: Minor: extract the id.len() > 128 guard into a helper.

The same length-cap check is now repeated in both get_by_id and get_by_id_top_editors. Pulling it into a tiny helper (e.g. validate_area_id_or_alias(&id)) would prevent the two sites from drifting and centralize the error message.

♻️ Proposed refactor

+fn validate_id_or_alias(id: &str) -> Result<(), RestApiError> {
+    if id.len() > 128 {
+        return Err(RestApiError::invalid_input("id too long"));
+    }
+    Ok(())
+}
+
 #[get("{id}")]
 pub async fn get_by_id(id: Path<String>, pool: Data<MainPool>) -> Res<GetByIdRes> {
-    if id.len() > 128 {
-        return Err(RestApiError::invalid_input("id too long"));
-    }
+    validate_id_or_alias(&id)?;
     ...
 }
 
 #[get("{id}/top-editors")]
 pub async fn get_by_id_top_editors(...) -> Res<Vec<TopEditor>> {
-    if id.len() > 128 {
-        return Err(RestApiError::invalid_input("id too long"));
-    }
+    validate_id_or_alias(&id)?;
     ...
 }

Also applies to: 212-214

🤖 Prompt for AI Agents

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

In `@src/rest/v4/areas.rs` around lines 112 - 114, Extract the repeated
length-check "id.len() > 128" into a small shared helper (e.g.
validate_area_id_or_alias(&id)) and call it from get_by_id and
get_by_id_top_editors; the helper should perform the guard and return Result<(),
RestApiError> using the same RestApiError::invalid_input("id too long") message
so both call sites share the exact validation and error text. Ensure the helper
name is exported/visible to both functions and replace the inline checks in
get_by_id and get_by_id_top_editors with a single call to
validate_area_id_or_alias(&id).

---

222-229: Consider validating period_start <= period_end.

Today, passing period_start=2030-01-01&period_end=2020-01-01 is silently accepted and the SQL BETWEEN ?1 AND ?2 just returns an empty result. Returning 400 Invalid input is friendlier to clients and matches what most range-based endpoints do. Non-blocking — the global /v4/top-editors has the same gap, so if you want to fix it, do it in the shared helper so both endpoints benefit.

🤖 Prompt for AI Agents

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

In `@src/rest/v4/areas.rs` around lines 222 - 229, The code accepts period_start
and period_end (parsed via parse_date and defaulting to
OffsetDateTime::UNIX_EPOCH / far_future) without checking their order; add
validation after computing period_start and period_end (or in the shared helper
used by both areas.rs and /v4/top-editors) to return a 400 Invalid input when
period_start > period_end, using the same error type/response pattern your
handlers use so clients receive a clear "period_start must be <= period_end"
validation error instead of an empty SQL result.

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

103-108: Optional: consider LazyLock to tighten this pattern.

On Rust 1.80+, std::sync::LazyLock lets you drop the explicit get_or_init boilerplate. Your project runs 1.93.0, so this is available. Non-blocking — current OnceLock form is correct.

♻️ Proposed refactor

-static TIP_URL_RE: OnceLock<Regex> = OnceLock::new();
-
-pub(crate) fn extract_tip_url(description: &str) -> Option<String> {
-    let re = TIP_URL_RE.get_or_init(|| Regex::new(r"(lightning:[^)]+)").unwrap());
-    re.captures(description).map(|c| c[1].to_string())
-}
+static TIP_URL_RE: std::sync::LazyLock<Regex> =
+    std::sync::LazyLock::new(|| Regex::new(r"(lightning:[^)]+)").unwrap());
+
+pub(crate) fn extract_tip_url(description: &str) -> Option<String> {
+    TIP_URL_RE.captures(description).map(|c| c[1].to_string())
+}

🤖 Prompt for AI Agents

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

In `@src/rest/v4/top_editors.rs` around lines 103 - 108, Replace the manual
OnceLock/get_or_init pattern with std::sync::LazyLock: change the static
TIP_URL_RE: OnceLock<Regex> to a static TIP_URL_RE: LazyLock<Regex> initialized
with LazyLock::new(|| Regex::new(r"(lightning:[^)]+)").unwrap()), and update
extract_tip_url to call the regex directly via TIP_URL_RE (e.g.,
TIP_URL_RE.captures(description).map(|c| c[1].to_string())); this removes the
explicit get_or_init boilerplate while keeping the same behavior for TIP_URL_RE
and extract_tip_url.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@src/rest/v4/areas.rs`:
- Around line 474-501: Add a test case exercising the lower-bound validation for
the top editors endpoint: in the existing test module (where
top_editors_for_area_respects_limit is defined) add a new async test that
initializes the same App with get_by_id_top_editors and sends a GET to
"/{area.id}/top-editors?limit=0", then assert the response status is 400; this
targets the validate_limit path invoked by get_by_id_top_editors to lock in the
contract for limit<1.
- Around line 112-114: Extract the repeated length-check "id.len() > 128" into a
small shared helper (e.g. validate_area_id_or_alias(&id)) and call it from
get_by_id and get_by_id_top_editors; the helper should perform the guard and
return Result<(), RestApiError> using the same RestApiError::invalid_input("id
too long") message so both call sites share the exact validation and error text.
Ensure the helper name is exported/visible to both functions and replace the
inline checks in get_by_id and get_by_id_top_editors with a single call to
validate_area_id_or_alias(&id).
- Around line 222-229: The code accepts period_start and period_end (parsed via
parse_date and defaulting to OffsetDateTime::UNIX_EPOCH / far_future) without
checking their order; add validation after computing period_start and period_end
(or in the shared helper used by both areas.rs and /v4/top-editors) to return a
400 Invalid input when period_start > period_end, using the same error
type/response pattern your handlers use so clients receive a clear "period_start
must be <= period_end" validation error instead of an empty SQL result.

In `@src/rest/v4/top_editors.rs`:
- Around line 103-108: Replace the manual OnceLock/get_or_init pattern with
std::sync::LazyLock: change the static TIP_URL_RE: OnceLock<Regex> to a static
TIP_URL_RE: LazyLock<Regex> initialized with LazyLock::new(||
Regex::new(r"(lightning:[^)]+)").unwrap()), and update extract_tip_url to call
the regex directly via TIP_URL_RE (e.g.,
TIP_URL_RE.captures(description).map(|c| c[1].to_string())); this removes the
explicit get_or_init boilerplate while keeping the same behavior for TIP_URL_RE
and extract_tip_url.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 33daee4b-cf74-4501-8887-9cf2e7d2eae5

📥 Commits

Reviewing files that changed from the base of the PR and between 2606688 and 81ebe7e.

📒 Files selected for processing (5)

• src/db/main/osm_user/blocking_queries.rs
• src/db/main/osm_user/queries.rs
• src/main.rs
• src/rest/v4/areas.rs
• src/rest/v4/top_editors.rs