Clean up User model + AdminController

[skyfallwastaken]

No description provided.

[Copilot]

Pull request overview

This PR restructures the User model into smaller concerns and refactors the admin “quantized visualization” endpoint to use a dedicated service object, with accompanying tests and Swagger/RSwag documentation updates.

Changes:

• Extracts User responsibilities into concerns (Users::Identity, Users::AdminAndTrust, Users::Profile, Users::Authentication).
• Introduces Admin::VisualizationQuantizedQuery service and updates AdminController#visualization_quantized to delegate to it.
• Adds request/docs coverage for the new admin visualization endpoint and a unit test for the query service.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
test/services/admin/visualization_quantized_query_test.rb	Adds Minitest coverage for the new quantized query service.
swagger/v1/swagger.yaml	Documents the new admin visualization endpoint and updates username nullability.
spec/requests/api/admin/v1/admin_user_utils_spec.rb	Adds RSwag coverage for the new endpoint and updates schema for nullable username.
app/services/admin/visualization_quantized_query.rb	New service implementing quantized point selection + daily totals computation.
app/models/user.rb	Slims the model by including newly extracted concerns.
app/models/concerns/users/profile.rb	New concern containing profile/display-related methods (avatar/name/timezone parsing, etc.).
app/models/concerns/users/identity.rb	New concern containing identity lookup/search + username normalization/validation helpers.
app/models/concerns/users/authentication.rb	New concern containing token + API key rotation helpers.
app/models/concerns/users/admin_and_trust.rb	New concern containing trust/admin enums and related behaviors.
app/controllers/api/admin/v1/admin_controller.rb	Refactors visualization_quantized to call the new service and map errors.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

This PR refactors the User model by extracting methods into four focused concerns (Identity, AdminAndTrust, Profile, Authentication) and moves the quantized visualization SQL logic from the admin controller into a new Admin::VisualizationQuantizedQuery service object backed by Rswag specs and unit tests. The changes are purely organisational — no behaviour is altered and the existing callbacks (before_validation :normalize_username, validate :username_must_be_visible) remain correctly wired in user.rb.

Confidence Score: 5/5

Safe to merge — all findings are P2 style/consistency suggestions with no blocking correctness issues.

Both comments are P2: one is a concern-placement design smell (not_convicted/not_suspect) and the other is a pre-existing timezone inconsistency now surfaced in the new service. Neither breaks current behaviour in a UTC-hosted environment. Tests and Rswag specs are included for the new service.

app/models/concerns/users/identity.rb (cross-concern scope placement), app/services/admin/visualization_quantized_query.rb (local-TZ date resolution)

Important Files Changed

Filename	Overview
app/controllers/api/admin/v1/admin_controller.rb	Simplified by delegating SQL and validation to VisualizationQuantizedQuery; blank-check replaces old nil+range guard with equivalent behaviour preserved by the service.
app/models/concerns/users/identity.rb	Good extraction of identity/lookup logic, but not_convicted/not_suspect depend on trust_levels from AdminAndTrust, creating hidden cross-concern coupling.
app/models/concerns/users/admin_and_trust.rb	Clean extraction of trust/admin enum and trust-level management methods; logic is identical to what was inline in User.
app/models/concerns/users/authentication.rb	Straightforward extraction of sign-in token, API key rotation, and token lookup methods; no behaviour changes.
app/models/concerns/users/profile.rb	Clean move of country, avatar, display-name, extension-text, and timezone methods; minor stylistic cleanup (removed redundant self. on slack_avatar_url).
app/models/user.rb	Significantly slimmed down; callbacks (before_validation :normalize_username, validate :username_must_be_visible) correctly remain here and will resolve to the concern's private methods.
app/services/admin/visualization_quantized_query.rb	Good service-object extraction; switched to exec_query (adds type casting); timezone issue: Time.at(epoch).to_date uses local TZ while epochs are UTC-based.
spec/requests/api/admin/v1/admin_user_utils_spec.rb	Adds Rswag specs for the quantized visualization endpoint (200 with heartbeat data, 422 for invalid month) and corrects username schema to nullable: true.
test/services/admin/visualization_quantized_query_test.rb	Covers happy path (correct day count and points), invalid month, and very-large-year edge case; good coverage of the new service.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    U[User model] --> UI[Users::Identity\nscopes, lookup_by_identifier\nnormalize_username]
    U --> UAT[Users::AdminAndTrust\ntrust_level enum\nadmin_level enum\nset_trust / set_admin_level]
    U --> UP[Users::Profile\navatar_url, display_name\nformat_extension_text]
    U --> UA[Users::Authentication\nrotate_api_keys!\ncreate_email_signin_token]

    C[AdminController\nGET quantized] --> VQQ[Admin::VisualizationQuantizedQuery\nvalidate month\nbuild epochs\nexecute SQL\nreturn Result struct]
    VQQ --> DB[(PostgreSQL\nheartbeats)]
    VQQ --> RES{Result}
    RES -->|success?| JSON_OK[render json: days]
    RES -->|failure| JSON_ERR[render 422 error]

    UI -.->|depends on trust_levels from| UAT

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: app/models/concerns/users/identity.rb
Line: 49-55

Comment:
**Cross-concern dependency: trust scopes belong in `AdminAndTrust`**

`not_convicted` and `not_suspect` call `trust_levels[...]`, which is a class method generated by `enum :trust_level` defined in `Users::AdminAndTrust`. Placing these scopes in `Identity` creates a hidden coupling — any future use of `Users::Identity` without `Users::AdminAndTrust` would raise `NoMethodError`. Since these scopes are semantically about trust levels, they belong in `Users::AdminAndTrust` alongside the enum definition.

```suggestion
    end

    if Rails.env.development?
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: app/services/admin/visualization_quantized_query.rb
Line: 61-73

Comment:
**Local timezone used for UTC epoch day boundaries**

`Time.at(epoch).to_date` and `Time.at(row["time"].to_f).to_date` both resolve dates in the **server's local timezone**, while `start_epoch`/`end_epoch` are generated with `Time.utc(...)`. On a server not running in UTC, the day-boundary epochs (e.g. 2026-01-02 00:00 UTC) would map to the previous date in a UTC− timezone, causing `daily_totals[day]` and `points_by_day[day]` lookups to miss or return data for the wrong day. Use `.utc.to_date` to stay consistent with the UTC-based epochs.

```suggestion
    def build_points_by_day(rows)
      rows.each_with_object({}) do |row, hash|
        day = Time.at(row["time"].to_f).utc.to_date
        hash[day] ||= []
        hash[day] << {
          time: row["time"],
          lineno: row["lineno"],
          cursorpos: row["cursorpos"]
        }
      end
    end

    def build_days(start_epoch, end_epoch, daily_totals, points_by_day)
      (start_epoch...end_epoch).step(86_400).map do |epoch|
        day = Time.at(epoch).utc.to_date
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "Clean up `User` model + `AdminController..." | Re-trigger Greptile}

[skyfallwastaken]

@greptileai thoughts?

[greptile-apps]

Overall this is a solid cleanup. The big wins are clear:

What's working well:

• Extracting the raw SQL into Admin::VisualizationQuantizedQuery is the right call — controller went from ~130 lines to ~27, and the service is cleanly decomposed with proper private methods
• The UTC timezone fix (.utc.to_date) is already in place in the service
• Half-open interval (< ? instead of <= ?) is correct in the SQL
• ORDER BY "time" ASC before LIMIT in base_heartbeats — addressed
• Trust-level scopes (not_convicted, not_suspect) correctly live in AdminAndTrust alongside the enum definition
• lookup_by_identifier extracted cleanly into Users::Identity

One minor nit:

not_convicted and not_suspect are plain class_methods instead of scope. Rails scopes always return a chainable relation even when the block returns nil/false; class methods don't guarantee this. Prefer:

scope :not_convicted, -> { where.not(trust_level: trust_levels[:red]) }
scope :not_suspect, -> { where(trust_level: [ trust_levels[:blue], trust_levels[:green] ]) }

One thing to double-check:

valid_month? only validates that month is in 1–12, but year isn't validated — a non-numeric year param becomes 0 via .to_i, which raises Date::Error caught by the rescue block. It's handled, but silently returns :invalid_date instead of :invalid_parameters. Low risk, just worth being aware of.

Otherwise, clean and mergeable.