feat(firearms): shot count tracking per firearm (#11) - #34
Merged
Merged
Conversation
…estcontainers environment Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Replace the spawned `next start` (and the start-app.ts argv shim that forced
env past it) with Next's in-process `nextStart({ port, hostname })`. With no
child process, nothing re-loads a local .env or mise-cached env to clobber the
launcher's per-run DATABASE_URL/BETTER_AUTH_URL/BETTER_AUTH_SECRET, so sessions
minted against the test container validate correctly. Drops the secret-on-argv
and the internal-bin import; keeps the production build, ephemeral random ports,
in-process minting, and Ryuk cleanup. Mirrors the community next-dev in-process
e2e pattern. All 13 specs pass locally; CI (no .env) unaffected.
Also documents the root cause + fix in docs/solutions/test-failures/.
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Completes the shim removal: replace the spawned `next start` with Next's
in-process `nextStart({ port, hostname })` in the launcher, so no child process
re-loads a local .env / mise-cached env to clobber this run's DATABASE_URL /
BETTER_AUTH_URL / BETTER_AUTH_SECRET. Update the docs/solutions learning to the
in-process solution. All 13 e2e specs pass locally; CI unaffected.
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
First firearm child table (#11): one row per firearm per range trip, FK ON DELETE CASCADE, rounds_fired >= 1 check, nullable ammo_id seam for #7. No owner_id — visibility inherits from the parent firearm (R62). Fix the sqlfluff pre-commit hook: add .sqlfluff (dialect=postgres) and exclude the drizzle-generated src/db/migrations/ from sqlfluff, which otherwise errored (no dialect) and mangled generated SQL on every commit. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
… date) Returns all failure codes together (KTD4). rounds_fired must be a whole number >= 1; date must be a non-empty parseable ISO date. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
… totals CRUD authorized through the parent firearm (KTD2); delete needs only edit (KTD3). lifetimeRoundTotals derives per-firearm totals via SQL sum, cast to number (KTD1). Add visibleFirearmPermissions for UI control gating (KTD7). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
ActionResult-wrapped, mirroring firearm actions. Mutations revalidate /firearms; listRangeSessionsAction is read-only for the on-demand history. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Firearms list gains a Rounds column (derived total) and a per-row Sessions panel. History loads on demand with loading/empty/error states; log/edit/ delete controls are gated on the viewer's firearm permission (KTD7). Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…llow-ups Code review (3-reviewer agreement, P1): - updateRangeSession now authorizes before validating, so an invisible session can't be probed via invalid input (ValidationError vs NotFound); add a guard test for the invalid-input + no-visibility case (R70). - Remount the session form on edit-target switch (key) so stale field state can't overwrite a different session (P1 correctness). Simplify pass: - useCallback the history load (drop the exhaustive-deps suppression); extract toPermission and a shared expectRejects; derive the session list once and replace the 4-way ternary with an early-return render function. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
2 tasks
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 47 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds firearm range-session tracking with persisted sessions, derived lifetime round totals, permission-gated history controls, Playwright coverage, and updated test-server startup behavior. ChangesRange session shot count tracking
Sequence Diagram(s)sequenceDiagram
participant User
participant RangeSessionForm
participant SessionActions
participant RangeSessionService
participant Database
User->>RangeSessionForm: submit session data
RangeSessionForm->>SessionActions: logRangeSessionAction(input)
SessionActions->>RangeSessionService: createRangeSession(actorId, input)
RangeSessionService->>Database: insert range_session row
Database-->>RangeSessionService: created row
RangeSessionService-->>SessionActions: RangeSession
SessionActions-->>RangeSessionForm: ActionResult({ id })
sequenceDiagram
participant FirearmsPage
participant FirearmsView
participant RangeSessionHistory
participant SessionActions
FirearmsPage->>FirearmsView: FirearmListItem[] with roundTotal and permission
FirearmsView->>RangeSessionHistory: open history panel
RangeSessionHistory->>SessionActions: listRangeSessionsAction(firearmId)
SessionActions-->>RangeSessionHistory: sessions
RangeSessionHistory-->>FirearmsView: onChange -> router.refresh()
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches✨ Simplify code
Comment |
coderabbitai
Bot
added
backend
enhancement
There was a problem hiding this comment.
Pull request overview
Implements firearm-scoped range session logging to derive and display lifetime round totals (issue #11), including owner/grant-based authorization inherited from the parent firearm, plus end-to-end coverage and a small e2e harness/tooling reliability fix.
Changes:
- Add
range_sessionchild table + migration, factories, validation, and visibility-scoped domain service (CRUD + derived totals). - Wire server actions and firearms UI to show lifetime totals and load session history on demand with permission-based gating.
- Add integration + Playwright e2e coverage, and harden local e2e reliability by serving Next in-process; fix sqlfluff hook dialect/config.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test-support/factories.ts | Adds makeRangeSession DB factory for integration tests. |
| src/test-support/assertions.ts | Adds expectRejects helper for Drizzle thenables in Bun tests. |
| src/domain/validation-messages.ts | Adds user-facing messages for range-session validation codes. |
| src/domain/range-sessions/validate.ts | Introduces pure range-session input validation. |
| src/domain/range-sessions/service.ts | Adds authorized CRUD + derived lifetime totals for sessions. |
| src/domain/range-sessions/tests/validate.test.ts | Unit tests for range-session validation behavior. |
| src/domain/range-sessions/tests/service.test.ts | Integration tests for auth + totals + cascade behaviors. |
| src/db/migrations/meta/0006_snapshot.json | Updates Drizzle snapshot metadata for new table. |
| src/db/migrations/meta/_journal.json | Records new migration in Drizzle journal. |
| src/db/migrations/0006_wakeful_exodus.sql | Generated migration creating range_session table + index/FK/check. |
| src/db/inventory-schema.ts | Defines rangeSession table schema, constraints, and index. |
| src/auth/visibility.ts | Adds visibleFirearmPermissions for per-firearm UI gating signal. |
| e2e/start-test-server.ts | Serves Next in-process via nextStart to prevent env clobbering locally. |
| e2e/start-app.ts | Removes no-longer-needed spawned-app shim. |
| e2e/range-sessions.spec.ts | Adds e2e coverage for log/sum/delete derived total behavior. |
| e2e/fixtures/user-pool.ts | Adds range-sessions seeded user key for the new spec. |
| docs/solutions/test-failures/e2e-dotenv-mise-clobbers-launcher-env.md | Documents the local e2e env-clobber failure mode and fix. |
| docs/plans/2026-07-03-001-feat-shot-count-tracking-plan.md | Captures the implementation plan and technical decisions for #11. |
| app/(app)/firearms/session-actions.ts | Adds server actions for session CRUD + history reads. |
| app/(app)/firearms/range-session-history.tsx | Adds on-demand session history panel with edit/delete gating. |
| app/(app)/firearms/range-session-form.tsx | Adds client form for logging/editing sessions with validation feedback. |
| app/(app)/firearms/page.tsx | Joins derived totals + permissions into firearms list items. |
| app/(app)/firearms/firearms-view.tsx | Adds “Rounds” column and “Sessions” panel entrypoint. |
| .sqlfluff | Configures sqlfluff dialect to Postgres. |
| .pre-commit-config.yaml | Excludes generated migrations from sqlfluff hooks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.pre-commit-config.yaml:
- Around line 59-66: The SQLFluff hooks in .pre-commit-config.yaml are excluding
src/db/migrations/ entirely, which leaves migration SQL unchecked. Update the
sqlfluff-lint and sqlfluff-fix entries to keep migrations covered by narrowing
the suppression to only the generator-specific rule(s), or add a CI migration
validation step for the drizzle-kit output. Use the existing sqlfluff-lint and
sqlfluff-fix hook definitions as the place to fix this gating gap.
In `@app/`(app)/firearms/firearms-view.tsx:
- Line 59: The sessions panel is holding a stale snapshot of the selected
firearm because `sessionsFor` stores the whole `FirearmListItem` and is never
re-derived after `router.refresh()`. Update `firearms-view.tsx` to store only
the selected firearm id in the sessions state, then derive the current item from
the live `firearms` list on each render before passing it to
`RangeSessionHistory`. Also update the `setSessionsFor(...)` call sites to use
the id-based setter and clear it with null so renamed firearms or revoked grants
immediately reflect in the open panel.
In `@e2e/start-test-server.ts`:
- Around line 29-37: The test harness relies on the internal
next/dist/cli/next-start entry point from next in start-test-server, which is
unsafe while package.json still allows a floating ^16.2.9 range. Update the
dependency definition for next to an exact pinned version if nextStart is kept,
and keep the import in start-test-server aligned with that fixed version so
future patch/minor upgrades do not break this harness. Reference the nextStart
import and the next dependency declaration when making the change.
🪄 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: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 74269660-d362-456a-b487-ed6e9c8f2d58
📒 Files selected for processing (25)
.pre-commit-config.yaml.sqlfluffapp/(app)/firearms/firearms-view.tsxapp/(app)/firearms/page.tsxapp/(app)/firearms/range-session-form.tsxapp/(app)/firearms/range-session-history.tsxapp/(app)/firearms/session-actions.tsdocs/plans/2026-07-03-001-feat-shot-count-tracking-plan.mddocs/solutions/test-failures/e2e-dotenv-mise-clobbers-launcher-env.mde2e/fixtures/user-pool.tse2e/range-sessions.spec.tse2e/start-app.tse2e/start-test-server.tssrc/auth/visibility.tssrc/db/inventory-schema.tssrc/db/migrations/0006_wakeful_exodus.sqlsrc/db/migrations/meta/0006_snapshot.jsonsrc/db/migrations/meta/_journal.jsonsrc/domain/range-sessions/__tests__/service.test.tssrc/domain/range-sessions/__tests__/validate.test.tssrc/domain/range-sessions/service.tssrc/domain/range-sessions/validate.tssrc/domain/validation-messages.tssrc/test-support/assertions.tssrc/test-support/factories.ts
💤 Files with no reviewable changes (1)
- e2e/start-app.ts
…iduals - Derive the round-total visible set from visibleFirearmPermissions instead of a second getVisibleIds pass: lifetimeRoundTotals takes an optional pre-resolved set; the firearms page feeds it the permission map's keys, so the feature adds one owned∪granted scan (gating + totals), not three. - Add a two-user Playwright spec for AE2 at the UI layer: an owner shares a firearm view-only, and a second browser context (the viewer's session) reads the lifetime total and history but sees no Log/Edit/Delete controls (KTD7). Exports storageStateFor for multi-user specs. Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…ust to version 1.55.1 Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
…o version 26.1.0 Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
coderabbitai
Bot
added
the
dependencies
- lifetimeRoundTotals keeps sum(rounds_fired) as bigint and parses at the edge instead of casting ::int (removes int4 overflow risk). [copilot] - validateRangeSession rejects impossible calendar days that Date.parse normalizes (e.g. 2026-02-31); add day-overflow tests. [copilot] - firearms-view derives the sessions panel's firearm from the live list by id, so a rename/revoked-grant/delete reflects immediately instead of a stale snapshot. [coderabbit] Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
Signed-off-by: UncleSp1d3r <unclesp1d3r@evilbitlabs.io>
unclesp1d3r
enabled auto-merge (squash)
July 3, 2026 23:38
This was referenced Jul 4, 2026
Inventory log for firearms and magazines (inventoried / cleaned / lubed, with actor + timestamp)
#46
Closed
Open
This was referenced Jul 14, 2026
This was referenced Jul 29, 2026
Merged
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements shot count tracking per firearm (#11): a
range_sessionchild table logging rounds fired per firearm, with a derived lifetime round total surfaced on the firearms list, on-demand session history, and log/edit/delete — all owner-scoped through the parent firearm. Standalone slice; ammo (#7) and service-interval (#10) integrations are designed-for but deferred.Plan:
docs/plans/2026-07-03-001-feat-shot-count-tracking-plan.mdWhat shipped (U1–U6)
range_sessionschema: first firearm child (FKON DELETE CASCADE,rounds_fired >= 1check, nullableammo_idAdd ammo inventory tracking with low-stock alerts and summary rollups #7 seam, noowner_id) + generated migration + factory.rounds_firedis a whole number ≥ 1; date is a valid ISO date.lifetimeRoundTotalsderives per-firearm totals via a SQLSUM(cast to number);visibleFirearmPermissionssupplies the UI gating signal.Key decisions
owner_idand no own grant family; visibility and write-auth resolve through the parent firearm (R7/R62). Session mutations — including delete — require edit on the firearm (KTD3), deliberately looser than firearm delete (owner-only).Test plan
bun run typecheck— cleanbun run lint(biome) — cleanbun test— 219 pass / 0 fail (new range-session unit + integration tests cover AE1, AE2, the owner/edit/view/none authorization matrix, KTD7, cascade, the rounds check constraint, and the update existence-leak guard)bun run build— succeedsbun run test:e2e— 14 pass (incl. range-sessions AE1; no regressions in the firearms flows)Code review
Ran a multi-reviewer pass (correctness, security, testing, maintainability). Fixed before merge:
updateRangeSessionnow authorizes before validating, so an invisible session can't be probed via invalid input (ValidationErrorvsNotFound); added a guard test (R70 existence-hiding).key) so stale field state can't overwrite a different session.useCallbackthe history load (drop the exhaustive-deps suppression); extracttoPermissionand a sharedexpectRejects; derive the session list once and replace a 4-way ternary with an early-return render function.Tooling fix (out of plan)
The
sqlfluffpre-commit hook had no dialect configured anywhere in the repo and errored on every SQL commit (and its auto-fix mangled generated SQL). Added.sqlfluff(dialect = postgres) and excluded the drizzle-generatedsrc/db/migrations/from the hook — hand-linting generated migrations fights the generator (e.g.PG01NOT VALID/CONCURRENTLYrules drizzle never emits).Review residuals — resolved (no deferrals)
Both items the review flagged as deferrable were addressed in this PR:
e2e/range-sessions-sharing.spec.tsis a two-user spec: an owner shares a firearm view-only, and a second browser context (the viewer's session) reads the lifetime total + history but sees no Log/Edit/Delete controls (AE2 at the UI layer, KTD7). ExportsstorageStateForto support multi-user specs.lifetimeRoundTotalsnow accepts an optional pre-resolved visible set; the firearms page derives it once fromvisibleFirearmPermissionsand feeds it in, so the feature adds a single owned∪granted scan (serving both gating and the totals) rather than three.Closes #11.