Skip to content

Hide intervals for events you didn't do#1330

Open
skyfallwastaken wants to merge 4 commits into
mainfrom
interval-event-hide
Open

Hide intervals for events you didn't do#1330
skyfallwastaken wants to merge 4 commits into
mainfrom
interval-event-hide

Conversation

@skyfallwastaken
Copy link
Copy Markdown
Member

Summary of the problem

The interval picker would show events like Scrapyard and High Seas when you didn't even have an account then.

Describe your changes

Hide events you weren't around for.

Screenshots / Media

N/A

Copilot AI review requested due to automatic review settings May 21, 2026 13:58
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 21, 2026

Greptile Summary

This PR hides event intervals (High Seas, Scrapyard, etc.) from the interval picker for users who weren't active during those events. It introduces a bitmask column (event_participation) on users, populated at heartbeat ingest time, and a event_participation_backfilled flag to distinguish users who still need a historical backfill.

  • Event date ranges are extracted from hardcoded Ruby constants into config/events.json, making them the single source of truth for both the Rails backend and the Svelte frontend.
  • The IntervalSelect component now derives visible intervals using the bitmap (when available) or falls back to comparing user.created_at against the event end date for not-yet-backfilled users.
  • No tests are included for the new record_event_participation logic or the updated interval-filtering behavior, and no backfill job is provided for existing users whose event_participation_backfilled flag is set to false.

Confidence Score: 3/5

Not safe to merge without fixing the method name typo in heartbeat_ingest.rb — it will crash on every heartbeat processed during an active event window.

The event_participations (plural) call in record_event_participation will raise NoMethodError at runtime for any user sending heartbeats while an event is active, meaning participation is never recorded correctly. Additionally, existing users are left in an unbackfilled state with no job included to populate their bitmaps, so the UI improvement they would see is limited until a backfill is written and run. There are also no tests for the new ingest-side logic.

app/services/heartbeat_ingest.rb has the method name bug; db/migrate/20260521132654_add_event_participation_backfilled_to_users.rb highlights the missing backfill story.

Important Files Changed

Filename Overview
app/services/heartbeat_ingest.rb Adds record_event_participation to set bitmap flags on heartbeat ingest; contains a runtime NoMethodError due to event_participations (plural) instead of event_participation (singular), and calls the method with all import times regardless of how many rows were actually inserted.
app/models/concerns/time_range_filterable.rb Extracts event definitions from hardcoded ranges into config/events.json; EVENT_KEYS sorted alphabetically to keep bit positions stable. Logic looks correct.
app/javascript/pages/Home/signedIn/IntervalSelect.svelte Adds filtering logic to hide event intervals the user didn't participate in; correctly handles not-yet-backfilled users by falling back to the created_at heuristic. Uses $derived correctly per the no-effects rule.
db/migrate/20260521132654_add_event_participation_backfilled_to_users.rb Adds event_participation_backfilled boolean column with correct default flip for existing vs. new users, but no backfill job is included to actually populate the bitmap for existing users.
app/models/user.rb Adds flag :event_participation using alphabetically-sorted EVENT_KEYS to ensure stable bit positions. Change is minimal and correct.
app/controllers/inertia_controller.rb Passes created_at and event_participation (guarded by the backfilled flag) to the frontend; looks correct.
config/events.json New canonical source for event date ranges; clean extraction of what was previously hardcoded in the concern.
db/migrate/20260521131313_add_event_participation_to_users.rb Adds event_participation integer column with default: 0, null: false; straightforward bitmask column migration.

Sequence Diagram

sequenceDiagram
    participant Client as Browser (Svelte)
    participant Controller as InertiaController
    participant User as User Model
    participant Ingest as HeartbeatIngest
    participant DB as Database

    Client->>Controller: Page load (GET /)
    Controller->>User: current_user.event_participation
    User->>DB: SELECT event_participation, event_participation_backfilled FROM users
    DB-->>User: bitmap integer + backfilled flag
    Controller-->>Client: "{ created_at, event_participation: string[] | null }"

    Note over Client: visibleIntervals derived:<br/>ended event + backfilled → participation.has(key)<br/>active/future or not backfilled → created_at ≤ ends_at

    Client->>Controller: POST /heartbeats (heartbeat data)
    Controller->>Ingest: HeartbeatIngest.call(...)
    Ingest->>DB: INSERT heartbeat (ignore duplicate)
    Ingest->>Ingest: record_event_participation([time])
    loop each EVENT_RANGES entry
        Ingest->>User: event_participation.set?(key) [in-memory]
        alt bit not set AND time in range
            Ingest->>DB: "UPDATE users SET event_participation = event_participation | bit"
            Ingest->>User: event_participation.set(key) [in-memory sync]
        end
    end
Loading

Comments Outside Diff (1)

  1. app/services/heartbeat_ingest.rb, line 192-204 (link)

    P2 record_event_participation called with all import times, not just newly inserted ones

    insert_all with unique_by: :fields_hash silently skips duplicates; inserted only counts new rows. But record_event_participation is called with the full records list, including duplicates that were already in the database. For a re-import of the exact same data (0 new rows), we still scan every event range against every record timestamp. The set? guard prevents double-writes, so correctness is maintained, but it is unnecessary work on large re-imports.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: app/services/heartbeat_ingest.rb
    Line: 192-204
    
    Comment:
    **`record_event_participation` called with all import times, not just newly inserted ones**
    
    `insert_all` with `unique_by: :fields_hash` silently skips duplicates; `inserted` only counts new rows. But `record_event_participation` is called with the full `records` list, including duplicates that were already in the database. For a re-import of the exact same data (0 new rows), we still scan every event range against every record timestamp. The `set?` guard prevents double-writes, so correctness is maintained, but it is unnecessary work on large re-imports.
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
app/services/heartbeat_ingest.rb:221
**Wrong method name — `NoMethodError` at runtime**

`active_flag` exposes a class/relation method named after the column verbatim: `event_participation` (singular). Calling `event_participations` (plural) on the `ActiveRecord::Relation` will raise `NoMethodError: undefined method 'event_participations'` on every heartbeat ingested during an active event window, silently failing to record participation. The correct call is `User.where(id: @user.id).event_participation.set_all!(key)`.

Per the gem's README: `Profile.languages.set_all!(:chinese)` — the receiver matches the flag column name exactly.

```suggestion
      User.where(id: @user.id).event_participation.set_all!(key)
```

### Issue 2 of 3
db/migrate/20260521132654_add_event_participation_backfilled_to_users.rb:1-12
**No backfill job included**

The migration sets `event_participation_backfilled = false` for all existing users and notes that they "need backfill," but there is no rake task or background job in this PR that actually iterates historical heartbeats and populates the `event_participation` bitmap. Until a backfill runs, every existing user will have a `null` `event_participation` on the frontend, causing the UI to fall back to the coarser `created_at <= event_ends_at` heuristic. That means users who signed up before an event ended but never actually participated will still see those event intervals, which is exactly the bug this PR aims to fix.

### Issue 3 of 3
app/services/heartbeat_ingest.rb:192-204
**`record_event_participation` called with all import times, not just newly inserted ones**

`insert_all` with `unique_by: :fields_hash` silently skips duplicates; `inserted` only counts new rows. But `record_event_participation` is called with the full `records` list, including duplicates that were already in the database. For a re-import of the exact same data (0 new rows), we still scan every event range against every record timestamp. The `set?` guard prevents double-writes, so correctness is maintained, but it is unnecessary work on large re-imports.

Reviews (1): Last reviewed commit: "fa" | Re-trigger Greptile

to_i = range.end.to_i
next unless times.any? { |t| t >= from_i && t <= to_i }

User.where(id: @user.id).event_participations.set_all!(key)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Wrong method name — NoMethodError at runtime

active_flag exposes a class/relation method named after the column verbatim: event_participation (singular). Calling event_participations (plural) on the ActiveRecord::Relation will raise NoMethodError: undefined method 'event_participations' on every heartbeat ingested during an active event window, silently failing to record participation. The correct call is User.where(id: @user.id).event_participation.set_all!(key).

Per the gem's README: Profile.languages.set_all!(:chinese) — the receiver matches the flag column name exactly.

Suggested change
User.where(id: @user.id).event_participations.set_all!(key)
User.where(id: @user.id).event_participation.set_all!(key)
Prompt To Fix With AI
This is a comment left during a code review.
Path: app/services/heartbeat_ingest.rb
Line: 221

Comment:
**Wrong method name — `NoMethodError` at runtime**

`active_flag` exposes a class/relation method named after the column verbatim: `event_participation` (singular). Calling `event_participations` (plural) on the `ActiveRecord::Relation` will raise `NoMethodError: undefined method 'event_participations'` on every heartbeat ingested during an active event window, silently failing to record participation. The correct call is `User.where(id: @user.id).event_participation.set_all!(key)`.

Per the gem's README: `Profile.languages.set_all!(:chinese)` — the receiver matches the flag column name exactly.

```suggestion
      User.where(id: @user.id).event_participation.set_all!(key)
```

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

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to hide event-based date intervals in the dashboard interval picker when the current user did not participate in (or did not exist for) those events. It introduces a shared events configuration and tracks per-user event participation so the UI can filter event intervals more accurately.

Changes:

  • Added config/events.json and refactored server-side time-range definitions to load event ranges from that config.
  • Added an event_participation bitmask (via active_flag) plus an event_participation_backfilled flag, and updated heartbeat ingestion to set participation bits when new heartbeats land.
  • Updated the signed-in interval picker to build event intervals from events.json and hide ended events based on participation/backfill status.

Reviewed changes

Copilot reviewed 12 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
vite.config.ts Attempts to enable TS path alias resolution for Vite.
tsconfig.json Adds $config/* path mapping for importing config JSON from TS/Svelte.
Gemfile / Gemfile.lock Adds active_flag gem for bitmask flags on User.
db/schema.rb Updates schema version and includes DB changes (currently includes large unrelated diffs).
db/migrate/20260521131313_add_event_participation_to_users.rb Adds users.event_participation integer column.
db/migrate/20260521132654_add_event_participation_backfilled_to_users.rb Adds users.event_participation_backfilled with “existing false, new true” default behavior.
config/events.json Introduces centralized event definitions (dates/timezone/human name).
app/services/heartbeat_ingest.rb Sets event participation bits when direct/import heartbeats are persisted.
app/models/user.rb Declares the event_participation flag on User.
app/models/concerns/time_range_filterable.rb Loads event ranges/keys from config/events.json and merges into available ranges.
app/javascript/types/index.ts Adds created_at and event_participation to the typed current user payload.
app/javascript/pages/Home/signedIn/IntervalSelect.svelte Builds event intervals dynamically and filters visible event intervals based on user participation/existence.
app/controllers/inertia_controller.rb Exposes created_at and event_participation (or nil pre-backfill) to the frontend.

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

Comment thread vite.config.ts
Comment on lines +43 to +45
# mahad says: NEVER remove entries from the events JSON
# if you need to get rid of an event, add a retired flag or something
EVENT_KEYS = EVENT_DEFINITIONS.keys.sort.map(&:to_sym).freeze
Comment on lines +59 to +66
const endsAt = Date.parse(range.ends_at);
// Ended event + backfilled: show only if the user actually participated.
// Otherwise (active/future event, or not-yet-backfilled user) fall back
// to the cheap "did the user exist before the event ended" check.
if (endsAt < Date.now() && participated) {
return participated.has(interval.key);
}
return userCreatedAt <= endsAt;
Comment thread db/schema.rb
Comment on lines +156 to +184
create_table "github_app_installations", force: :cascade do |t|
t.bigint "account_github_id"
t.string "account_login", null: false
t.string "account_type", null: false
t.datetime "created_at", null: false
t.bigint "installation_id", null: false
t.datetime "suspended_at"
t.datetime "updated_at", null: false
t.bigint "user_id"
t.index ["account_github_id"], name: "index_github_app_installations_on_account_github_id"
t.index ["installation_id"], name: "index_github_app_installations_on_installation_id", unique: true
t.index ["user_id"], name: "index_github_app_installations_on_user_id"
end

create_table "github_app_repositories", force: :cascade do |t|
t.boolean "archived", default: false, null: false
t.datetime "created_at", null: false
t.string "full_name", null: false
t.bigint "github_app_installation_id", null: false
t.bigint "github_repo_id", null: false
t.string "html_url", null: false
t.boolean "private", default: false, null: false
t.bigint "repository_id"
t.datetime "updated_at", null: false
t.index ["full_name"], name: "index_github_app_repositories_on_full_name"
t.index ["github_app_installation_id"], name: "index_github_app_repositories_on_github_app_installation_id"
t.index ["github_repo_id"], name: "index_github_app_repositories_on_github_repo_id", unique: true
t.index ["repository_id"], name: "index_github_app_repositories_on_repository_id"
end
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants