Maintenance plans / Recurring maintenance#1466
Open
vicwiller wants to merge 3 commits into
Open
Conversation
Contributor
WalkthroughImplements recurring maintenance plans: DB schema and migrations, repo-level plan CRUD and rolling logic, entry linking and overdue filters, API endpoints and routes, frontend API clients, recurrence utilities and UI changes, plus tests. ChangesRecurring maintenance feature
Sequence Diagram(s)sequenceDiagram
participant Client
participant APIHandler as V1Controller
participant Repo as MaintenanceEntryRepository
participant DB as Database
Client->>APIHandler: POST /entities/{id}/maintenance/plans (plan create)
APIHandler->>Repo: CreatePlan(groupID, itemID, CreateDTO)
Repo->>Repo: computeNextDue(startDate, interval)
Repo->>DB: INSERT maintenance_plans
Repo->>DB: INSERT maintenance_entries (plan_id, scheduledDate=nextDueAt)
Repo-->>APIHandler: MaintenancePlan (201)
Client->>APIHandler: PUT /maintenance/entries/{id} (mark complete)
APIHandler->>Repo: Update(entryID, CompletedDate)
Repo->>DB: UPDATE maintenance_entries (completedDate)
Repo->>Repo: rollPlanFromCompletion(planID)
Repo->>DB: UPDATE maintenance_plans (lastCompletedAt, nextDueAt)
Repo->>DB: INSERT new maintenance_entries (open, scheduled=nextDue)
Repo-->>APIHandler: Updated Entry (200)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Security Recommendations
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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. Comment |
coderabbitai
Bot
requested review from
katosdev,
tankerkiller125 and
tonyaellie
coderabbitai
Bot
added
⬆️ enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/app/api/routes.go (1)
167-185:⚠️ Potential issue | 🔴 Critical🚨 Critical: UpdatePlan and DeletePlan lack group ownership validation
The repository methods
UpdatePlan()andDeletePlan()inbackend/internal/data/repo/repo_maintenance_plan.godirectly operate on plan IDs without verifying the plan belongs to the requesting user's group. Both methods callUpdateOneID(planID)andDeleteOneID(id)respectively, with no WHERE clause filtering by group.This allows any authenticated user to update or delete maintenance plans from other groups by knowing their UUIDs. Compare to the secure pattern in
ListPlansByItemID(), which validates group ownership via:entity.HasGroupWith(group.IDEQ(groupID))Fix required: Both methods must accept
groupIDas a parameter and add a WHERE clause validating the plan belongs to that group before mutating, following the multi-tenancy pattern established in other handlers.Routes are correctly behind
userMWand handler registration is sound — the vulnerability is in the repository layer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/routes.go` around lines 167 - 185, Update the insecure repository methods in backend/internal/data/repo/repo_maintenance_plan.go: modify UpdatePlan(...) and DeletePlan(...) to accept a groupID parameter and apply the same multi-tenant WHERE clause used elsewhere (e.g., validate the plan belongs to the group using the pattern entity.HasGroupWith(group.IDEQ(groupID)) or equivalent predicate) before calling the mutating operations (replace direct UpdateOneID(planID) / DeleteOneID(id) with an Update/Delete that includes the group filter). Ensure callers/handlers that invoke UpdatePlan and DeletePlan pass the requesting user's group ID.backend/internal/data/repo/repo_maintenance_entry.go (1)
112-132: 🛠️ Refactor suggestion | 🟠 MajorNon-atomic Create: entry persists even if
SetPlanIDfails ⚛️The current flow is
Save()→ conditionalUpdateOneID().SetPlanID().Save(). If the second call fails (network blip, DB constraint, context cancellation), you end up with a maintenance entry that's not linked to the plan the caller asked for, and the API returns the wrapped error — leaving silent inconsistency. Easier to do this in one shot:♻️ Suggested fix
func (r *MaintenanceEntryRepository) Create(ctx context.Context, itemID uuid.UUID, input MaintenanceEntryCreate) (MaintenanceEntry, error) { - item, err := r.db.MaintenanceEntry.Create(). + creator := r.db.MaintenanceEntry.Create(). SetEntityID(itemID). SetDate(input.CompletedDate.Time()). SetScheduledDate(input.ScheduledDate.Time()). SetName(input.Name). SetDescription(input.Description). - SetCost(input.Cost). - Save(ctx) - if err != nil { - return mapMaintenanceEntryErr(item, err) - } - - if input.PlanID != uuid.Nil { - item, err = r.db.MaintenanceEntry.UpdateOneID(item.ID). - SetPlanID(input.PlanID). - Save(ctx) - } - + SetCost(input.Cost) + if input.PlanID != uuid.Nil { + creator = creator.SetPlanID(input.PlanID) + } + item, err := creator.Save(ctx) return mapMaintenanceEntryErr(item, err) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/data/repo/repo_maintenance_entry.go` around lines 112 - 132, The Create method is non-atomic: it calls r.db.MaintenanceEntry.Create().Save() then conditionally does an UpdateOneID(...).SetPlanID(...).Save(), which can leave a created entry unlinked if the second save fails; instead, set PlanID on the initial builder when input.PlanID != uuid.Nil so the single Create builder (r.db.MaintenanceEntry.Create()...) includes SetPlanID(input.PlanID) before calling Save(ctx), and return mapMaintenanceEntryErr on that single Save error to keep the operation atomic (refer to Create, r.db.MaintenanceEntry.Create(), SetPlanID, UpdateOneID, Save, and MaintenanceEntryCreate.input.PlanID).frontend/lib/api/types/data-contracts.ts (1)
1-1006:⚠️ Potential issue | 🟠 MajorThis file is out of sync with the swagger specification and cannot be considered properly generated.
The TypeScript file contains fields (
isOverdue,planID,itemID,itemName) that do not exist inbackend/app/api/static/docs/openapi-3.json. Since this file is marked as auto-generated from swagger-typescript-api, either:
- The file was manually edited (violates the guideline), or
task generatewas not run after the backend schema/handler changesThe backend code (
repo_maintenance_entry.go) clearly defines these fields, but the swagger spec hasn't been regenerated. Runtask generateto sync the swagger specs and regenerate TypeScript types to ensure consistency between backend and frontend.Security note: Type mismatches between swagger specs and generated TypeScript can lead to frontend code making API calls with incorrect payload shapes, potentially causing runtime failures or security validation bypasses.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/api/types/data-contracts.ts` around lines 1 - 1006, The generated TypeScript types are out of sync with the OpenAPI spec: fields isOverdue, planID, itemID and itemName appear in frontend/lib/api/types/data-contracts.ts but are missing from the OpenAPI spec (openapi-3.json); fix by regenerating the swagger spec and TS types—run the project generator (task generate) to update the OpenAPI spec from the backend (see repo_maintenance_entry.go which defines those fields) and then re-run swagger-typescript-api to produce a new data-contracts.ts so the frontend types match the backend.
🧹 Nitpick comments (8)
backend/internal/data/repo/repo_maintenance_plan_test.go (1)
10-18: Nice baseline coverage — optional: pin month-end normalization behavior.You cover all five branches of
computeNextDue. One thing not tested is Go'stime.AddDatenormalization at month boundaries (e.g., Jan 31 + 1 month yields March 3, not Feb 28). If the user-visible "Rolling tasks" semantics expect "the same calendar day each month, clamped to month end",time.AddDatedoes not give you that, and a regression here would silently shift maintenance schedules. Worth a documented test that locks down the chosen behavior.A second optional addition: assert the default/unknown unit falls back to
base(current behavior), so a future refactor doesn't accidentally start mutating it.♻️ Suggested additions
+ // Document month-end rollover semantics (AddDate normalizes Jan 31 + 1mo -> Mar 3). + endOfMonth := time.Date(2026, time.January, 31, 10, 0, 0, 0, time.UTC) + assert.Equal(t, + time.Date(2026, time.March, 3, 10, 0, 0, 0, time.UTC), + computeNextDue(endOfMonth, 1, MaintenancePlanIntervalUnitMonth), + ) + + // Unknown unit falls back to base. + assert.Equal(t, base, computeNextDue(base, 1, MaintenancePlanIntervalUnit("decade")))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/data/repo/repo_maintenance_plan_test.go` around lines 10 - 18, Add tests to explicitly pin month-end normalization and the unknown-unit fallback in TestComputeNextDue: (1) add a case where base is a month-end date (e.g., Jan 31) and assert whether computeNextDue(base, 1, MaintenancePlanIntervalUnitMonth) returns the expected clamped month-end or the behavior you want documented; (2) add a case that passes an invalid/unknown unit and assert computeNextDue returns the original base (current fallback). Refer to the computeNextDue function and the TestComputeNextDue test to locate where to add these assertions.frontend/locales/en.json (1)
696-701: Minor i18n: hardcoded "(s)" plural notation.
"Day(s)","Hour(s)", etc. work in English but rely on a single static label rather than ICU plurals driven by theinterval_value. Other languages (Slavic, Arabic, etc.) have multiple plural forms and "Day(s)" maps poorly. Since these strings are also used for a unit dropdown rather than a sentence with the count, the parens are probably acceptable as-is, but consider switching to{count, plural, one {Day} other {Days}}if the dropdown ever becomes contextual ("Every 2 days", "Every 1 day").Weblate will localize the rest of
en.jsonfor you, so no changes required to other locale files. As per coding guidelines: "i18n translations must be maintained infrontend/locales/".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/locales/en.json` around lines 696 - 701, Replace the hardcoded plural labels ("interval_day", "interval_hour", "interval_month", "interval_week", "interval_year") with ICU plural variants that use a count variable (e.g., "{count, plural, one {Day} other {Days}}") so translations can handle language-specific plural rules; keep "interval_value" as-is for the static "Every" token and ensure the UI uses the same count variable when composing contextual strings like "Every {count} {interval_day}" so the pluralization resolves correctly.frontend/lib/maintenance/recurrence.test.ts (1)
5-26: Add month-end edge-case and interval coverage tests to prevent maintenance scheduling gaps.The current test suite only covers mid-month dates with Week and Month intervals at intervalValue=1. The implementation uses
setMonth()without clamping, so Jan 31 + 1 month will advance to March 2–3 rather than staying on a month boundary—a breaking UX surprise for users. This directly affects the EditModal UI that displays upcoming maintenance dates; incorrect scheduling could cause critical systems to miss maintenance windows.Suggested additions (covering month-end, Hour, Day, Year, and intervalValue > 1):
Test additions
+ it("handles month-end rollover deterministically", () => { + const scheduledDate = new Date("2026-01-31T00:00:00Z"); + const dueDates = getNextNDueDates(scheduledDate, 1, MaintenancePlanIntervalUnit.Month, 3); + // Pin whether behavior is rollover-to-next-month or clamp-to-month-end + expect(dueDates[0]?.toISOString()).toBe("2026-01-31T00:00:00.000Z"); + // expect(dueDates[1]?.toISOString()).toBe("2026-02-28T00:00:00.000Z"); // or March rollover if intentional + }); + + it.each([ + [MaintenancePlanIntervalUnit.Hour, 6], + [MaintenancePlanIntervalUnit.Day, 2], + [MaintenancePlanIntervalUnit.Year, 1], + ])("advances by %s × %i", (unit, value) => { + const start = new Date("2026-06-15T12:00:00Z"); + const [first, second] = getNextNDueDates(start, value, unit, 2); + expect(first?.toISOString()).toBe(start.toISOString()); + expect(second).toBeDefined(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/maintenance/recurrence.test.ts` around lines 5 - 26, Add tests and fix getNextNDueDates to handle month-end clamping and broader interval coverage: the implementation currently advances Jan 31 by using Date.setMonth() which rolls into March for Feb overflow, so add unit tests for month-end (e.g., Jan 31 with Month interval and intervalValue > 1), and add tests for Hour, Day, Year intervals and intervalValue > 1 to cover edge cases; modify getNextNDueDates to clamp day-of-month when adding months (preserve last-day-of-month behavior) so months advance to the last valid day if the target month is shorter, and ensure the function still includes the scheduledDate as the first due date. Identify changes around the getNextNDueDates function and any helper that computes month additions (the code path using Date.setMonth).backend/internal/data/repo/repo_maintenance.go (1)
61-68: Overdue filter looks correct — small consistency nit 🧹The predicate logic (scheduled in the past AND no completion) is right. Two small things to consider for parity with
repo_maintenance_entry.go'sMaintenanceFilterStatusOverduecase:
- The sibling repo orders overdue entries
ByScheduledDate(sql.OrderAsc()); the global query here keeps the existing ordering at line 80. Worth aligning so "overdue" lists feel the same in both contexts.time.Now()is captured per-request, which is fine, but be aware allscheduled_daterows in the DB are stored in UTC by Ent — make sure the frontend'sdateToApiValue(which formats local Y-M-D) doesn't widen the overdue window by a day near midnight in non-UTC timezones.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/data/repo/repo_maintenance.go` around lines 61 - 68, The overdue filter block in the query (MaintenanceFilterStatusOverdue) correctly selects scheduled dates in the past with no completion, but needs two fixes: add ordering to match the sibling repo by appending .Order(maintenanceentry.ByScheduledDate(sql.OrderAsc())) to the query when handling MaintenanceFilterStatusOverdue, and ensure the time cutoff uses UTC by replacing time.Now() with time.Now().UTC() (or explicitly convert to UTC before comparison) so ScheduledDateLT uses a UTC timestamp consistent with the DB/Ent stored values; update the MaintenanceFilterStatusOverdue case in repo_maintenance.go to apply both changes.frontend/lib/maintenance/recurrence.ts (1)
21-23: Default case silently returns the base date 🤫If a future enum member is added (or a string is forced through the API) and the switch doesn't match, the loop in
getNextNDueDateswill silently emit the same datecounttimes — an infinite-looking "preview" that's actually frozen. Throwing or doing an exhaustiveness check will surface bugs faster.♻️ Suggested exhaustiveness guard
default: - return next; + throw new Error(`Unsupported interval unit: ${intervalUnit as string}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/lib/maintenance/recurrence.ts` around lines 21 - 23, The switch's default branch currently returns the input date, causing getNextNDueDates to silently repeat the same date if an unknown recurrence kind is passed; replace the default return with an exhaustiveness guard by throwing an Error (or calling an assertNever/assertUnreachable helper) so unknown enum/string values surface immediately; update the switch in the recurrence computation (the default branch used by getNextNDueDates) to throw and include the offending value in the message to aid debugging.backend/internal/data/repo/repo_maintenance_entry_test.go (1)
87-107: Tests don't clean up created entries 🧽
TestMaintenanceEntryRepository_GetLogis careful to delete its 10 entries at the end (lines 81-84) precisely becauseuseEntitiesshares state across tests. The four new tests (_GetLog_Overdue,_Update_ScheduledCanBeCompleted,_Update_RecurringCompletionCreatesNextEntry,_CreatePlan_UsesStartDateAsFirstDueDate) leave their entries (and plans) behind, which can leak into later assertions if tests run in the same process or are reordered.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/data/repo/repo_maintenance_entry_test.go` around lines 87 - 107, The new tests TestMaintenanceEntryRepository_GetLog_Overdue, TestMaintenanceEntryRepository_Update_ScheduledCanBeCompleted, TestMaintenanceEntryRepository_Update_RecurringCompletionCreatesNextEntry, and TestMaintenanceEntryRepository_CreatePlan_UsesStartDateAsFirstDueDate create maintenance entries (via tRepos.MaintEntry.Create) and plans but do not remove them, causing shared-state leaks; fix by deleting the created entries/plans at the end of each test (or registering t.Cleanup callbacks) using the repository methods used elsewhere in tests (e.g., tRepos.MaintEntry.Delete / tRepos.MaintPlan.Delete or the same cleanup helper the original TestMaintenanceEntryRepository_GetLog uses) so each test leaves the DB state unchanged.frontend/components/Maintenance/EditModal.vue (1)
14-27: Free-textintervalValueplus raw<select>deviates from Shadcn-vue 🎨Two tiny gripes here:
entry.intervalValueis bound to aFormTextFieldand stored as a string. Users can type "1.5", "abc", or leave it blank; theparseInt(...) || 1fallback hides input errors instead of surfacing them. Considertype="number"withmin="1"and an integer validator.- The native
<select>doesn't match the rest of the modal which usescomponents/ui/(Shadcn-vue) primitives. As per coding guidelines (frontend/**/*.vue: "Use Shadcn-vue UI components fromcomponents/ui/for consistent design system"), this should use the project'sSelectcomponent for visual consistency and a11y.Also worth wrapping the
<input type="checkbox">in the same shadcnCheckboxcomponent used elsewhere — the hardcoded:true-value="true" :false-value="false"is unnecessary for a boolean v-model.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/components/Maintenance/EditModal.vue` around lines 14 - 27, The current modal uses a free-text FormTextField bound to entry.intervalValue (stored as string) and a native <select> for entry.intervalUnit, and a raw checkbox input; replace the FormTextField with a numeric input (use the FormTextField props or switch to a number-capable field) enforcing type="number", min="1" and integer validation for entry.intervalValue (remove the parseInt(...)||1 fallback and validate/parses explicitly), swap the native <select> for the project's Shadcn Select component from components/ui to bind entry.intervalUnit to the MaintenancePlanIntervalUnit options, and replace the raw <input type="checkbox"> with the project's Checkbox component so v-model binds a boolean directly; update any parsing/submit logic that previously relied on the fallback to use the validated numeric value.backend/internal/data/repo/repo_maintenance_plan.go (1)
229-244:computeNextDueswallows unknown interval units — fail loud instead.If
plan.IntervalUnitever ends up as a value outside the five known constants (DB drift, future migration, partial enum rollback…), this returnsbaseunchanged.rollPlanFromCompletionwill then storeLastCompletedAt == NextDueAt, the dedupe check on line 199 may match, and the user silently loses the rolling schedule with no log line to grep for. Returning(time.Time, error)and propagating the error up throughrollPlanFromCompletionwould surface the misconfiguration at completion time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/data/repo/repo_maintenance_plan.go` around lines 229 - 244, computeNextDue currently returns base for unknown MaintenancePlanIntervalUnit values which silently breaks rolling schedules; change computeNextDue to return (time.Time, error) and on the default case return an error describing the invalid plan.IntervalUnit, update all cases to return (computedTime, nil), and propagate this error up through rollPlanFromCompletion (adjust rollPlanFromCompletion signature/handling) so callers either log/return the error instead of writing LastCompletedAt==NextDueAt; reference computeNextDue, rollPlanFromCompletion, plan.IntervalUnit and the MaintenancePlanIntervalUnit constants when making these changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/api/handlers/v1/v1_ctrl_maintenance_plan.go`:
- Line 19: The Swagger `@Success` annotation in v1_ctrl_maintenance_plan.go
currently uses `{array} repo.MaintenancePlan[]`; remove the redundant `[]` so it
reads `{array} repo.MaintenancePlan` to match swaggo conventions and other
annotations (look for the `@Success 200 {array} repo.MaintenancePlan[]` comment
and update it accordingly).
- Around line 41-48: Both HandleMaintenancePlanCreate and
HandleMaintenancePlanUpdate currently forward the incoming body to the repo
without running its validation; call body.Validate() at the start of each
handler (in HandleMaintenancePlanCreate's fn and in
HandleMaintenancePlanUpdate's fn) and if it returns an error, return that error
immediately instead of calling ctrl.repo.MaintEntry.CreatePlan / UpdatePlan so
invalid payloads (e.g., IntervalValue < 1 or unknown IntervalUnit) are rejected
before reaching the repository.
In
`@backend/internal/data/migrations/postgres/20260424120001_add_maintenance_plans.sql`:
- Around line 20-23: The migration adds maintenance_entries.plan_id and an index
but omits a foreign key constraint; update the Postgres migration to add a FK
from maintenance_entries(plan_id) to maintenance_plans(id) with ON DELETE SET
NULL (and keep or recreate idx_maintenance_entries_plan_id as needed) so deletes
of plans nullify the plan_id rather than cascade-delete entries; also mirror
this referential integrity in the corresponding SQLite migration for
maintenance_entries using the rename/create/copy/drop pattern since SQLite
cannot add a FK to an existing column.
In
`@backend/internal/data/migrations/sqlite3/20260424120000_add_maintenance_plans.sql`:
- Around line 20-23: The new maintenance_entries.plan_id column lacks a foreign
key constraint to maintenance_plans(id), risking orphaned or invalid references;
update the migration so maintenance_entries is recreated with plan_id declared
as a UUID that references maintenance_plans(id) with an appropriate ON DELETE
action (e.g., ON DELETE SET NULL or ON DELETE CASCADE) and re-create the index
idx_maintenance_entries_plan_id after the table is rebuilt; ensure the migration
follows SQLite’s procedure for adding FKs (create new table with FK, copy data,
drop old table, rename new table) and uses the same column/index names
(maintenance_entries.plan_id and idx_maintenance_entries_plan_id).
In `@backend/internal/data/repo/repo_maintenance_entry_test.go`:
- Around line 196-225: The test compares two time.Time values with assert.Equal
which can fail across SQLite/Postgres due to location/monotonic differences;
update TestMaintenanceEntryRepository_CreatePlan_UsesStartDateAsFirstDueDate to
compare instants instead of raw structs—e.g., assert that
plan.NextDueAt.Equal(startDate) or use assert.WithinDuration/Unix timestamps
after converting both to UTC—so reference plan.NextDueAt and startDate when
changing the assertion.
In `@backend/internal/data/repo/repo_maintenance_entry.go`:
- Around line 134-176: The Update/Delete logic allows unsafe partial updates and
lacks tenant checks; change the DTO to pointer types (e.g., *string for
Name/Description, *float64 for Cost, *uuid.UUID for PlanID) so nil = "no change"
and explicit zero/empty values are applied, update
MaintenanceEntryRepository.Update to apply fields only when pointers are non-nil
(use SetName/SetDescription/SetCost even for empty/zero when pointer provided)
and replace the PlanID branch to SetPlanID when input.PlanID != nil otherwise
leave unchanged (remove ClearPlanID unless the caller explicitly provides a
nil-plan sentinel); ensure Create writes PlanID atomically (include PlanID in
the initial Create builder instead of a separate Update); and add group
ownership predicates to queries in MaintenanceEntryRepository.Update (replace
Query().Where(maintenanceentry.IDEQ(id)).Only(ctx) with
Query().Where(maintenanceentry.IDEQ(id),
maintenanceentry.HasEntityWith(entity.HasGroupWith(group.IDEQ(gid)))) ) and to
Delete (use DeleteOne().Where/ensure DeleteOneID is scoped with
HasEntityWith(...)) and ensure HandleMaintenanceEntryUpdate passes gid from auth
context into these calls.
In `@backend/internal/data/repo/repo_maintenance_plan.go`:
- Around line 199-211: The equality check using ScheduledDateEQ(nextDue) in the
MaintenanceEntry.Query is fragile because nextDue is an in-memory time.Time with
nanosecond/monotonic precision and may not match database-stored timestamps;
update the query to avoid exact timestamp equality — either remove the
ScheduledDateEQ(nextDue) filter and rely on the invariant "at most one open
entry per plan" (i.e., only use maintenanceentry.PlanIDEQ(planID) and the
open-date predicate used there), or replace ScheduledDateEQ(nextDue) with a
tolerant range using ScheduledDateGTE(nextDue.Add(-delta)) and
ScheduledDateLTE(nextDue.Add(delta)) (choose delta like 1s); change the logic in
the block around MaintenanceEntry.Query, openCount variable, and any dedupe
decisions accordingly so duplicates cannot be created.
- Around line 125-157: CreatePlan (and similarly UpdatePlan and DeletePlan) is
missing a tenancy check and will attach plans to any entity UUID; update the
repository methods (CreatePlan, UpdatePlan, DeletePlan) to require the caller's
group ID (auth.GID) by either: (a) validating up-front that the entity with
itemID belongs to the provided groupID, or (b) including group_id in the ORM
predicates when creating/updating/deleting (same pattern used in
ListPlansByItemID); then change the handlers in v1_ctrl_maintenance_plan.go to
pass auth.GID into these repo calls so every mutation enforces group ownership.
- Around line 159-178: Update UpdatePlan so it no longer treats a zero time as
"omit": change the MaintenancePlanUpdate DTO to use *time.Time for NextDueAt (or
add an explicit ClearNextDueAt bool) and in UpdatePlan use that pointer/flag to
decide between SetNextDueAt, ClearNextDueAt, or leaving the field untouched;
also enforce tenancy/IDOR by threading groupID into the update path and
constraining the Ent update with a predicate (e.g., use Where(...
HasEntityWith(HasGroupWith(...))) or perform an existence check that the plan
belongs to the given group before calling
r.db.MaintenancePlan.UpdateOneID(planID).Set...) so only plans belonging to the
caller’s group can be updated.
In `@frontend/components/Maintenance/EditModal.vue`:
- Around line 257-272: The current client-side workaround (involving
createdPlanId, entry.itemIdForPlanLookup, api.items.maintenance.getLog and
scanning for planID === createdPlanId then calling api.maintenance.delete) is
fragile and racy; change the flow so the client no longer re-fetches the whole
log to find-and-delete the auto-created entry: either (A) update the createPlan
API to accept a linkExistingEntryID (or similar flag) so the backend will not
auto-create the first entry when linking to an existing entry, or (B) have
createPlan return the ID of the auto-created entry so the client can delete that
exact ID (instead of scanning by planID); then remove the getLog + scan + delete
code and use the new API behavior/returned id to perform a single precise
operation. Ensure calls that currently reference createdPlanId,
api.items.maintenance.getLog, and api.maintenance.delete are updated to use the
new createPlan parameter or returned autoCreatedEntryId.
- Around line 178-185: The plan API calls are sending raw JS Date objects which
reintroduce the timezone truncation bug; update the createPlan and updatePlan
call sites in createEntry and editEntry to pass dateToApiValue(firstDueDate) (or
the equivalent date-only string) instead of the Date object, i.e. replace usages
of firstDueDate passed directly into api.items.maintenance.createPlan and
api.items.maintenance.updatePlan with dateToApiValue(firstDueDate); also ensure
the backend types (MaintenancePlanCreate.startDate and
MaintenancePlanUpdate.nextDueAt) expect/parse date-only values so the change is
end-to-end.
- Around line 282-298: The update path for recurring plans is overwriting
nextDueAt on every edit; change the payload construction before calling
api.maintenance.updatePlan so nextDueAt is only included when the user actually
changed the scheduled date or the recurrence (compare entry.scheduledDate,
entry.intervalUnit, and entry.intervalValue against the original/previous values
you have in the component—e.g., originalEntry or props.original—otherwise omit
nextDueAt), keep the existing intervalValue normalization
(Math.max(parseInt(entry.intervalValue,10)||1,1)) and only set nextDueAt
(currently computed as entry.scheduledDate ?? entry.completedDate ?? new Date())
when that comparison shows a real change; call api.maintenance.updatePlan with
that conditional payload so the backend retains the rolling schedule when
nextDueAt was not modified.
In `@frontend/components/Maintenance/ListView.vue`:
- Around line 161-171: The watcher on itemSearch currently calls searchItems on
every keystroke causing many API calls and potential out-of-order responses;
wrap the call to searchItems with a debounced function using useDebounceFn from
`@vueuse/core` (debounce maybe ~300ms) and replace the direct call in the watch
with the debounced version, and add a simple stale-response guard (e.g.,
incrementing requestId / token captured by the debounced call) so that responses
that don't match the latest token are ignored before updating availableItems;
update references to itemSearch, itemPickerOpen, searchItems, and availableItems
accordingly.
In `@frontend/lib/maintenance/recurrence.ts`:
- Around line 15-20: The Month/Year branches using next.setMonth(...) and
next.setFullYear(...) can overflow for end-of-month dates (e.g., Jan 31 + 1
month or Feb 29 + 1 year); modify the MaintenancePlanIntervalUnit.Month and
.Year handling to clamp the resulting date to the last day of the target month:
compute the target year/month (for Year add to getFullYear, for Month add to
getMonth), determine that month’s max day (e.g., lastDay = new Date(year,
month+1, 0).getDate()), and set next.setDate(Math.min(next.getDate(), lastDay))
after adjusting month/year so the day is clamped correctly; add small unit tests
covering Jan 31 + 1 month and Feb 29 + 1 year in recurrence.test.ts to validate
behavior.
---
Outside diff comments:
In `@backend/app/api/routes.go`:
- Around line 167-185: Update the insecure repository methods in
backend/internal/data/repo/repo_maintenance_plan.go: modify UpdatePlan(...) and
DeletePlan(...) to accept a groupID parameter and apply the same multi-tenant
WHERE clause used elsewhere (e.g., validate the plan belongs to the group using
the pattern entity.HasGroupWith(group.IDEQ(groupID)) or equivalent predicate)
before calling the mutating operations (replace direct UpdateOneID(planID) /
DeleteOneID(id) with an Update/Delete that includes the group filter). Ensure
callers/handlers that invoke UpdatePlan and DeletePlan pass the requesting
user's group ID.
In `@backend/internal/data/repo/repo_maintenance_entry.go`:
- Around line 112-132: The Create method is non-atomic: it calls
r.db.MaintenanceEntry.Create().Save() then conditionally does an
UpdateOneID(...).SetPlanID(...).Save(), which can leave a created entry unlinked
if the second save fails; instead, set PlanID on the initial builder when
input.PlanID != uuid.Nil so the single Create builder
(r.db.MaintenanceEntry.Create()...) includes SetPlanID(input.PlanID) before
calling Save(ctx), and return mapMaintenanceEntryErr on that single Save error
to keep the operation atomic (refer to Create, r.db.MaintenanceEntry.Create(),
SetPlanID, UpdateOneID, Save, and MaintenanceEntryCreate.input.PlanID).
In `@frontend/lib/api/types/data-contracts.ts`:
- Around line 1-1006: The generated TypeScript types are out of sync with the
OpenAPI spec: fields isOverdue, planID, itemID and itemName appear in
frontend/lib/api/types/data-contracts.ts but are missing from the OpenAPI spec
(openapi-3.json); fix by regenerating the swagger spec and TS types—run the
project generator (task generate) to update the OpenAPI spec from the backend
(see repo_maintenance_entry.go which defines those fields) and then re-run
swagger-typescript-api to produce a new data-contracts.ts so the frontend types
match the backend.
---
Nitpick comments:
In `@backend/internal/data/repo/repo_maintenance_entry_test.go`:
- Around line 87-107: The new tests
TestMaintenanceEntryRepository_GetLog_Overdue,
TestMaintenanceEntryRepository_Update_ScheduledCanBeCompleted,
TestMaintenanceEntryRepository_Update_RecurringCompletionCreatesNextEntry, and
TestMaintenanceEntryRepository_CreatePlan_UsesStartDateAsFirstDueDate create
maintenance entries (via tRepos.MaintEntry.Create) and plans but do not remove
them, causing shared-state leaks; fix by deleting the created entries/plans at
the end of each test (or registering t.Cleanup callbacks) using the repository
methods used elsewhere in tests (e.g., tRepos.MaintEntry.Delete /
tRepos.MaintPlan.Delete or the same cleanup helper the original
TestMaintenanceEntryRepository_GetLog uses) so each test leaves the DB state
unchanged.
In `@backend/internal/data/repo/repo_maintenance_plan_test.go`:
- Around line 10-18: Add tests to explicitly pin month-end normalization and the
unknown-unit fallback in TestComputeNextDue: (1) add a case where base is a
month-end date (e.g., Jan 31) and assert whether computeNextDue(base, 1,
MaintenancePlanIntervalUnitMonth) returns the expected clamped month-end or the
behavior you want documented; (2) add a case that passes an invalid/unknown unit
and assert computeNextDue returns the original base (current fallback). Refer to
the computeNextDue function and the TestComputeNextDue test to locate where to
add these assertions.
In `@backend/internal/data/repo/repo_maintenance_plan.go`:
- Around line 229-244: computeNextDue currently returns base for unknown
MaintenancePlanIntervalUnit values which silently breaks rolling schedules;
change computeNextDue to return (time.Time, error) and on the default case
return an error describing the invalid plan.IntervalUnit, update all cases to
return (computedTime, nil), and propagate this error up through
rollPlanFromCompletion (adjust rollPlanFromCompletion signature/handling) so
callers either log/return the error instead of writing
LastCompletedAt==NextDueAt; reference computeNextDue, rollPlanFromCompletion,
plan.IntervalUnit and the MaintenancePlanIntervalUnit constants when making
these changes.
In `@backend/internal/data/repo/repo_maintenance.go`:
- Around line 61-68: The overdue filter block in the query
(MaintenanceFilterStatusOverdue) correctly selects scheduled dates in the past
with no completion, but needs two fixes: add ordering to match the sibling repo
by appending .Order(maintenanceentry.ByScheduledDate(sql.OrderAsc())) to the
query when handling MaintenanceFilterStatusOverdue, and ensure the time cutoff
uses UTC by replacing time.Now() with time.Now().UTC() (or explicitly convert to
UTC before comparison) so ScheduledDateLT uses a UTC timestamp consistent with
the DB/Ent stored values; update the MaintenanceFilterStatusOverdue case in
repo_maintenance.go to apply both changes.
In `@frontend/components/Maintenance/EditModal.vue`:
- Around line 14-27: The current modal uses a free-text FormTextField bound to
entry.intervalValue (stored as string) and a native <select> for
entry.intervalUnit, and a raw checkbox input; replace the FormTextField with a
numeric input (use the FormTextField props or switch to a number-capable field)
enforcing type="number", min="1" and integer validation for entry.intervalValue
(remove the parseInt(...)||1 fallback and validate/parses explicitly), swap the
native <select> for the project's Shadcn Select component from components/ui to
bind entry.intervalUnit to the MaintenancePlanIntervalUnit options, and replace
the raw <input type="checkbox"> with the project's Checkbox component so v-model
binds a boolean directly; update any parsing/submit logic that previously relied
on the fallback to use the validated numeric value.
In `@frontend/lib/maintenance/recurrence.test.ts`:
- Around line 5-26: Add tests and fix getNextNDueDates to handle month-end
clamping and broader interval coverage: the implementation currently advances
Jan 31 by using Date.setMonth() which rolls into March for Feb overflow, so add
unit tests for month-end (e.g., Jan 31 with Month interval and intervalValue >
1), and add tests for Hour, Day, Year intervals and intervalValue > 1 to cover
edge cases; modify getNextNDueDates to clamp day-of-month when adding months
(preserve last-day-of-month behavior) so months advance to the last valid day if
the target month is shorter, and ensure the function still includes the
scheduledDate as the first due date. Identify changes around the
getNextNDueDates function and any helper that computes month additions (the code
path using Date.setMonth).
In `@frontend/lib/maintenance/recurrence.ts`:
- Around line 21-23: The switch's default branch currently returns the input
date, causing getNextNDueDates to silently repeat the same date if an unknown
recurrence kind is passed; replace the default return with an exhaustiveness
guard by throwing an Error (or calling an assertNever/assertUnreachable helper)
so unknown enum/string values surface immediately; update the switch in the
recurrence computation (the default branch used by getNextNDueDates) to throw
and include the offending value in the message to aid debugging.
In `@frontend/locales/en.json`:
- Around line 696-701: Replace the hardcoded plural labels ("interval_day",
"interval_hour", "interval_month", "interval_week", "interval_year") with ICU
plural variants that use a count variable (e.g., "{count, plural, one {Day}
other {Days}}") so translations can handle language-specific plural rules; keep
"interval_value" as-is for the static "Every" token and ensure the UI uses the
same count variable when composing contextual strings like "Every {count}
{interval_day}" so the pluralization resolves correctly.
🪄 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 UI
Review profile: CHILL
Plan: Pro
Run ID: d96b8fa3-4b1b-4c86-9738-099eea8cdce2
⛔ Files ignored due to path filters (32)
backend/go.sumis excluded by!**/*.sumbackend/internal/data/ent/client.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/ent.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/entity.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/entity/entity.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/entity/where.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/entity_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/entity_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/entity_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/has_id.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/hook/hook.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceentry.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceentry/maintenanceentry.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceentry/where.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceentry_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceentry_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceentry_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceplan.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceplan/maintenanceplan.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceplan/where.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceplan_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceplan_delete.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceplan_query.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/maintenanceplan_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/migrate/schema.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/mutation.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/predicate/predicate.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/runtime.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/entity.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/maintenance_entry.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/maintenance_plan.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/tx.gois excluded by!backend/internal/data/ent/**
📒 Files selected for processing (18)
backend/app/api/handlers/v1/v1_ctrl_maintenance_plan.gobackend/app/api/routes.gobackend/internal/data/migrations/postgres/20260424120001_add_maintenance_plans.sqlbackend/internal/data/migrations/sqlite3/20260424120000_add_maintenance_plans.sqlbackend/internal/data/repo/repo_maintenance.gobackend/internal/data/repo/repo_maintenance_entry.gobackend/internal/data/repo/repo_maintenance_entry_test.gobackend/internal/data/repo/repo_maintenance_plan.gobackend/internal/data/repo/repo_maintenance_plan_test.gofrontend/components/Maintenance/EditModal.vuefrontend/components/Maintenance/ListView.vuefrontend/components/ui/dialog-provider/utils.tsfrontend/lib/api/classes/items.tsfrontend/lib/api/classes/maintenance.tsfrontend/lib/api/types/data-contracts.tsfrontend/lib/maintenance/recurrence.test.tsfrontend/lib/maintenance/recurrence.tsfrontend/locales/en.json
Comment on lines
+41
to
+48
| func (ctrl *V1Controller) HandleMaintenancePlanCreate() errchain.HandlerFunc { | ||
| fn := func(r *http.Request, itemID uuid.UUID, body repo.MaintenancePlanCreate) (repo.MaintenancePlan, error) { | ||
| auth := services.NewContext(r.Context()) | ||
| return ctrl.repo.MaintEntry.CreatePlan(auth, itemID, body) | ||
| } | ||
|
|
||
| return adapters.ActionID("id", fn, http.StatusCreated) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm no other call site invokes Validate() on these DTOs (to be sure the handler is the only entry point).
rg -nP '\b(MaintenancePlanCreate|MaintenancePlanUpdate)\b' --type=go -g '!**/vendor/**'
rg -nP '\.Validate\(\)' --type=go -g '!**/vendor/**' | rg -i 'maintenance'Repository: sysadminsmedia/homebox
Length of output: 13038
🏁 Script executed:
#!/bin/bash
# Check the actual Validate() implementations
sed -n '56,75p' backend/internal/data/repo/repo_maintenance_plan.go
echo "---"
sed -n '64,75p' backend/internal/data/repo/repo_maintenance_plan.go
echo "---"
# Check the full handler file
cat -n backend/app/api/handlers/v1/v1_ctrl_maintenance_plan.go | head -75Repository: sysadminsmedia/homebox
Length of output: 4032
🏁 Script executed:
#!/bin/bash
# Check the CreatePlan and UpdatePlan implementations
sed -n '125,160p' backend/internal/data/repo/repo_maintenance_plan.go
echo "---"
# Check if validate package is used in other handlers
rg -n 'validate\.' backend/app/api/handlers/v1/ --type=go | head -20
echo "---"
# Check if CreatePlan calls Validate()
rg -A10 'func.*CreatePlan' backend/internal/data/repo/repo_maintenance_plan.go | head -20Repository: sysadminsmedia/homebox
Length of output: 4020
🏁 Script executed:
#!/bin/bash
# Check coding guidelines for handler validation requirements
grep -n "Validate\|validation\|validate" backend/app/api/handlers/v1/*.go | grep -i "pattern\|should\|must" | head -10
echo "---"
# Check other handlers with body validation to see the pattern
rg -B3 'body\.Validate' backend/app/api/handlers/v1/ --type=go | head -30Repository: sysadminsmedia/homebox
Length of output: 70
🏁 Script executed:
#!/bin/bash
# Check if any handler calls Validate() on body
rg 'body\.Validate\(\)' backend/app/api/handlers/ --type=go
echo "---"
# Check if CreatePlan/UpdatePlan methods have any validation despite not calling Validate()
sed -n '125,175p' backend/internal/data/repo/repo_maintenance_plan.goRepository: sysadminsmedia/homebox
Length of output: 1551
Call body.Validate() in both handlers before passing to repository methods.
MaintenancePlanCreate.Validate() and MaintenancePlanUpdate.Validate() enforce critical constraints (IntervalValue >= 1, known IntervalUnit), but neither handler invokes them. Invalid input bypasses these checks and flows directly into the repository layer, where only Ent's enum validation provides defense at best. A client posting {"intervalValue":0} will not be caught.
Affected handlers and fix
HandleMaintenancePlanCreate (lines 42–47):
fn := func(r *http.Request, itemID uuid.UUID, body repo.MaintenancePlanCreate) (repo.MaintenancePlan, error) {
+ if err := body.Validate(); err != nil {
+ return repo.MaintenancePlan{}, validate.NewRequestError(err, http.StatusBadRequest)
+ }
auth := services.NewContext(r.Context())
return ctrl.repo.MaintEntry.CreatePlan(auth, itemID, body)
}HandleMaintenancePlanUpdate (lines 61–66): Apply the same pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/api/handlers/v1/v1_ctrl_maintenance_plan.go` around lines 41 -
48, Both HandleMaintenancePlanCreate and HandleMaintenancePlanUpdate currently
forward the incoming body to the repo without running its validation; call
body.Validate() at the start of each handler (in HandleMaintenancePlanCreate's
fn and in HandleMaintenancePlanUpdate's fn) and if it returns an error, return
that error immediately instead of calling ctrl.repo.MaintEntry.CreatePlan /
UpdatePlan so invalid payloads (e.g., IntervalValue < 1 or unknown IntervalUnit)
are rejected before reaching the repository.
Comment on lines
+20
to
+23
| ALTER TABLE maintenance_entries | ||
| ADD COLUMN plan_id uuid; | ||
|
|
||
| CREATE INDEX idx_maintenance_entries_plan_id ON maintenance_entries (plan_id); |
Contributor
There was a problem hiding this comment.
🛡️ Same FK concern as the SQLite migration: no FK on maintenance_entries.plan_id.
In Postgres this is straightforward to bake in and is worth doing for referential integrity (and to let the planner reason about the relationship). Recommend ON DELETE SET NULL so that deleting a plan doesn't cascade-delete historical maintenance entries — the entries are likely valuable history even after the plan goes away.
♻️ Suggested change
ALTER TABLE maintenance_entries
- ADD COLUMN plan_id uuid;
+ ADD COLUMN plan_id uuid
+ REFERENCES maintenance_plans (id) ON DELETE SET NULL;If you go with this, mirror it in the SQLite migration (which requires the longer "rename, create, copy, drop" dance, since SQLite cannot add a FK to an existing column).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ALTER TABLE maintenance_entries | |
| ADD COLUMN plan_id uuid; | |
| CREATE INDEX idx_maintenance_entries_plan_id ON maintenance_entries (plan_id); | |
| ALTER TABLE maintenance_entries | |
| ADD COLUMN plan_id uuid | |
| REFERENCES maintenance_plans (id) ON DELETE SET NULL; | |
| CREATE INDEX idx_maintenance_entries_plan_id ON maintenance_entries (plan_id); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/internal/data/migrations/postgres/20260424120001_add_maintenance_plans.sql`
around lines 20 - 23, The migration adds maintenance_entries.plan_id and an
index but omits a foreign key constraint; update the Postgres migration to add a
FK from maintenance_entries(plan_id) to maintenance_plans(id) with ON DELETE SET
NULL (and keep or recreate idx_maintenance_entries_plan_id as needed) so deletes
of plans nullify the plan_id rather than cascade-delete entries; also mirror
this referential integrity in the corresponding SQLite migration for
maintenance_entries using the rename/create/copy/drop pattern since SQLite
cannot add a FK to an existing column.
Comment on lines
+20
to
+23
| ALTER TABLE maintenance_entries | ||
| ADD COLUMN plan_id uuid; | ||
|
|
||
| CREATE INDEX idx_maintenance_entries_plan_id ON maintenance_entries (plan_id); |
Contributor
There was a problem hiding this comment.
🛡️ Security/integrity: maintenance_entries.plan_id has no foreign key constraint.
Adding the column without a FK to maintenance_plans(id) lets stale/orphaned plan_id values survive a plan deletion (or, worse, allow inserts referencing non-existent plans). Given this is a multi-tenant app, an orphaned plan_id could even, post-recycle, point at a row owned by a different group. An ON DELETE SET NULL (or CASCADE, depending on desired semantics) constraint is the safer default and mirrors how entity_id is handled on maintenance_plans above.
Note: SQLite requires recreating the table to add a FK to an existing column, so this is easier to bake in now than later.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/internal/data/migrations/sqlite3/20260424120000_add_maintenance_plans.sql`
around lines 20 - 23, The new maintenance_entries.plan_id column lacks a foreign
key constraint to maintenance_plans(id), risking orphaned or invalid references;
update the migration so maintenance_entries is recreated with plan_id declared
as a UUID that references maintenance_plans(id) with an appropriate ON DELETE
action (e.g., ON DELETE SET NULL or ON DELETE CASCADE) and re-create the index
idx_maintenance_entries_plan_id after the table is rebuilt; ensure the migration
follows SQLite’s procedure for adding FKs (create new table with FK, copy data,
drop old table, rename new table) and uses the same column/index names
(maintenance_entries.plan_id and idx_maintenance_entries_plan_id).
Comment on lines
+196
to
+225
| func TestMaintenanceEntryRepository_CreatePlan_UsesStartDateAsFirstDueDate(t *testing.T) { | ||
| item := useEntities(t, 1)[0] | ||
| startDate := time.Date(2026, time.March, 10, 9, 30, 0, 0, time.UTC) | ||
|
|
||
| plan, err := tRepos.MaintEntry.CreatePlan(context.Background(), item.ID, MaintenancePlanCreate{ | ||
| Name: "Weekly maintenance", | ||
| Description: "Recurring task with explicit scheduled date", | ||
| IntervalValue: 1, | ||
| IntervalUnit: MaintenancePlanIntervalUnitWeek, | ||
| StartDate: startDate, | ||
| Active: true, | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| assert.Equal(t, startDate, plan.NextDueAt) | ||
|
|
||
| openEntries, err := tRepos.MaintEntry.db.MaintenanceEntry.Query(). | ||
| Where( | ||
| maintenanceentry.EntityID(item.ID), | ||
| maintenanceentry.PlanIDEQ(plan.ID), | ||
| maintenanceentry.ScheduledDateEQ(startDate), | ||
| maintenanceentry.Or( | ||
| maintenanceentry.DateIsNil(), | ||
| maintenanceentry.DateEQ(time.Time{}), | ||
| ), | ||
| ). | ||
| All(context.Background()) | ||
| require.NoError(t, err) | ||
| require.Len(t, openEntries, 1) | ||
| } |
Contributor
There was a problem hiding this comment.
assert.Equal(t, startDate, plan.NextDueAt) may flake across drivers 🪤
time.Time equality with assert.Equal compares wall-clock + monotonic + *Location. After a round-trip through SQLite/Postgres, the returned time.Time typically loses its monotonic reading and may come back in time.Local or time.UTC depending on driver/dialect — even though Equal() would say they're the same instant.
Per coding guidelines (backend/internal/data/repo/*_test.go: "test with both SQLite and PostgreSQL in CI"), this assertion is the kind of thing that survives SQLite but breaks under Postgres.
🛡️ Suggested fix
- assert.Equal(t, startDate, plan.NextDueAt)
+ assert.True(t, startDate.Equal(plan.NextDueAt), "expected NextDueAt %v to equal %v", plan.NextDueAt, startDate)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/internal/data/repo/repo_maintenance_entry_test.go` around lines 196 -
225, The test compares two time.Time values with assert.Equal which can fail
across SQLite/Postgres due to location/monotonic differences; update
TestMaintenanceEntryRepository_CreatePlan_UsesStartDateAsFirstDueDate to compare
instants instead of raw structs—e.g., assert that
plan.NextDueAt.Equal(startDate) or use assert.WithinDuration/Unix timestamps
after converting both to UTC—so reference plan.NextDueAt and startDate when
changing the assertion.
Comment on lines
+178
to
+185
| const { error: planError } = await api.items.maintenance.createPlan(itemId, { | ||
| name: entry.name, | ||
| description: entry.description, | ||
| active: true, | ||
| intervalValue: parseInt(entry.intervalValue, 10) || 1, | ||
| intervalUnit: entry.intervalUnit, | ||
| startDate: firstDueDate, | ||
| }); |
Contributor
There was a problem hiding this comment.
Plan APIs receive raw Date objects, undoing the date-only fix for plans 🧯
The PR description specifically calls out the date-offset bug fix that's implemented via dateToApiValue (line 127-136), and you correctly use it for entry create/update at lines 196-197 and 245-246. But the plan calls (createPlan at 184 and 231, updatePlan at 291) hand a JS Date straight to the JSON serializer, which produces an ISO timestamp like 2026-04-30T22:00:00.000Z — exactly the case that produced "one day earlier than selected" in the original bug.
The Go side parses this back into a time.Time and stores the truncated UTC date, so depending on the user's timezone the plan's first due date can land a day off. Same fix should apply to plan dates.
🐛 Suggested fix
- const firstDueDate = entry.scheduledDate ?? entry.completedDate ?? new Date(Date.now());
+ const firstDueDate = dateToApiValue(entry.scheduledDate ?? entry.completedDate ?? new Date(), "");…applied at the three call sites (createPlan in createEntry, createPlan/updatePlan in editEntry). You'll likely also want MaintenancePlanCreate.startDate / MaintenancePlanUpdate.nextDueAt to be parsed as date-only on the backend.
Also applies to: 223-232, 282-292
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/Maintenance/EditModal.vue` around lines 178 - 185, The
plan API calls are sending raw JS Date objects which reintroduce the timezone
truncation bug; update the createPlan and updatePlan call sites in createEntry
and editEntry to pass dateToApiValue(firstDueDate) (or the equivalent date-only
string) instead of the Date object, i.e. replace usages of firstDueDate passed
directly into api.items.maintenance.createPlan and
api.items.maintenance.updatePlan with dateToApiValue(firstDueDate); also ensure
the backend types (MaintenancePlanCreate.startDate and
MaintenancePlanUpdate.nextDueAt) expect/parse date-only values so the change is
end-to-end.
Comment on lines
+257
to
+272
| if (createdPlanId && entry.itemIdForPlanLookup) { | ||
| const { data: logData, error: logError } = await api.items.maintenance.getLog(entry.itemIdForPlanLookup, { | ||
| status: MaintenanceFilterStatus.MaintenanceFilterStatusBoth, | ||
| }); | ||
|
|
||
| if (!logError && logData) { | ||
| const autoCreatedEntry = logData.find(logEntry => logEntry.planID === createdPlanId && logEntry.id !== entry.id); | ||
| if (autoCreatedEntry) { | ||
| const { error: deleteError } = await api.maintenance.delete(autoCreatedEntry.id); | ||
| if (deleteError) { | ||
| toast.error(t("maintenance.toast.failed_to_update")); | ||
| return; | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Fragile workaround: fetch-the-log-then-delete to clean up the auto-created entry 🩹
When the user toggles recurrence on for an existing entry, this code:
- creates a plan (which on the backend auto-creates a new open entry for
nextDueAt), - updates the original entry to point at the new plan,
- then re-fetches the entire log for the item, scans for the plan's auto-created entry, and deletes it.
A few problems:
- Race: between create-plan and the GET, another user (or webhook) could create entries that match
planID === createdPlanId && id !== entry.id, which would then be wrongly deleted. - Cost: an extra round trip per edit just to undo backend behavior.
- Semantics: the user requested recurrence with a specific scheduled date; deleting the plan's first generated entry is correct only if the original entry's scheduled date matches the plan's first due date — otherwise you may end up keeping an entry whose schedule disagrees with the plan.
Consider letting the backend opt out of auto-creating the first entry when a linkExistingEntryID is supplied to createPlan, or have the API return the auto-created entry ID directly so the client doesn't have to re-scan.
Security flavor 🛡️: the matching key is planID only. If two plans are created concurrently for the same item, this loop can delete an entry created by the other plan. Worth tightening the predicate (or, better, removing this fetch-and-delete dance entirely).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/Maintenance/EditModal.vue` around lines 257 - 272, The
current client-side workaround (involving createdPlanId,
entry.itemIdForPlanLookup, api.items.maintenance.getLog and scanning for planID
=== createdPlanId then calling api.maintenance.delete) is fragile and racy;
change the flow so the client no longer re-fetches the whole log to
find-and-delete the auto-created entry: either (A) update the createPlan API to
accept a linkExistingEntryID (or similar flag) so the backend will not
auto-create the first entry when linking to an existing entry, or (B) have
createPlan return the ID of the auto-created entry so the client can delete that
exact ID (instead of scanning by planID); then remove the getLog + scan + delete
code and use the new API behavior/returned id to perform a single precise
operation. Ensure calls that currently reference createdPlanId,
api.items.maintenance.getLog, and api.maintenance.delete are updated to use the
new createPlan parameter or returned autoCreatedEntryId.
Comment on lines
+282
to
+298
| if (isRecurring && entry.planID) { | ||
| const intervalValue = Math.max(parseInt(entry.intervalValue, 10) || 1, 1); | ||
| const firstDueDate = entry.scheduledDate ?? entry.completedDate ?? new Date(Date.now()); | ||
| const { error: planError } = await api.maintenance.updatePlan(entry.planID, { | ||
| name: entry.name, | ||
| description: entry.description, | ||
| active: true, | ||
| intervalValue, | ||
| intervalUnit: entry.intervalUnit, | ||
| nextDueAt: firstDueDate, | ||
| }); | ||
|
|
||
| if (planError) { | ||
| toast.error(t("maintenance.toast.failed_to_update")); | ||
| return; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
updatePlan resets nextDueAt on every edit — kisses the rolling schedule goodbye 💋📅
Every time the user edits a recurring entry (e.g., just tweaks notes or cost), this branch fires and overrides nextDueAt with entry.scheduledDate ?? entry.completedDate ?? new Date(). That means:
- An entry currently scheduled for 2026-09-01 (rolled forward by a previous completion) will get pushed back to whatever this entry's
scheduledDateis — typically the original due date. - If the modal is opened on a completed entry,
nextDueAtwill be set to the completion date, which directly contradicts the rolling-task behavior described in the PR objectives.
nextDueAt should only be sent when the user actually changed the scheduled date or interval. Otherwise omit it from the payload and let the backend keep its computed value.
🐛 Suggested guard
if (isRecurring && entry.planID) {
const intervalValue = Math.max(parseInt(entry.intervalValue, 10) || 1, 1);
- const firstDueDate = entry.scheduledDate ?? entry.completedDate ?? new Date(Date.now());
const { error: planError } = await api.maintenance.updatePlan(entry.planID, {
name: entry.name,
description: entry.description,
active: true,
intervalValue,
intervalUnit: entry.intervalUnit,
- nextDueAt: firstDueDate,
+ // Only update nextDueAt when the plan was just created or interval/schedule changed;
+ // otherwise let the backend's rolling schedule remain authoritative.
+ ...(createdPlanId ? { nextDueAt: entry.scheduledDate ?? entry.completedDate ?? new Date() } : {}),
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/Maintenance/EditModal.vue` around lines 282 - 298, The
update path for recurring plans is overwriting nextDueAt on every edit; change
the payload construction before calling api.maintenance.updatePlan so nextDueAt
is only included when the user actually changed the scheduled date or the
recurrence (compare entry.scheduledDate, entry.intervalUnit, and
entry.intervalValue against the original/previous values you have in the
component—e.g., originalEntry or props.original—otherwise omit nextDueAt), keep
the existing intervalValue normalization
(Math.max(parseInt(entry.intervalValue,10)||1,1)) and only set nextDueAt
(currently computed as entry.scheduledDate ?? entry.completedDate ?? new Date())
when that comparison shows a real change; call api.maintenance.updatePlan with
that conditional payload so the backend retains the rolling schedule when
nextDueAt was not modified.
Comment on lines
+161
to
+171
| watch( | ||
| itemSearch, | ||
| async query => { | ||
| if (!itemPickerOpen.value) { | ||
| return; | ||
| } | ||
|
|
||
| await searchItems(query); | ||
| }, | ||
| { immediate: false } | ||
| ); |
Contributor
There was a problem hiding this comment.
Item search fires an API call on every keystroke 🐎
The watcher calls searchItems synchronously on every change to itemSearch with no debounce — a user typing "lawnmower" produces 9 calls to GET /entities. With pageSize: 100 and a typical homebox catalog, that's a lot of unnecessary load (and out-of-order responses can flicker the dropdown).
Wrap the watcher with a debounce — useDebounceFn from @vueuse/core (already a dependency, per the manifest) is a one-liner.
♻️ Suggested debounce
- watch(
- itemSearch,
- async query => {
- if (!itemPickerOpen.value) {
- return;
- }
-
- await searchItems(query);
- },
- { immediate: false }
- );
+ const debouncedSearchItems = useDebounceFn(searchItems, 250);
+ watch(
+ itemSearch,
+ async query => {
+ if (!itemPickerOpen.value) {
+ return;
+ }
+ await debouncedSearchItems(query);
+ },
+ { immediate: false }
+ );While you're there, you may also want to cancel/ignore stale results so an older response can't overwrite a newer availableItems.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/components/Maintenance/ListView.vue` around lines 161 - 171, The
watcher on itemSearch currently calls searchItems on every keystroke causing
many API calls and potential out-of-order responses; wrap the call to
searchItems with a debounced function using useDebounceFn from `@vueuse/core`
(debounce maybe ~300ms) and replace the direct call in the watch with the
debounced version, and add a simple stale-response guard (e.g., incrementing
requestId / token captured by the debounced call) so that responses that don't
match the latest token are ignored before updating availableItems; update
references to itemSearch, itemPickerOpen, searchItems, and availableItems
accordingly.
Comment on lines
+15
to
+20
| case MaintenancePlanIntervalUnit.Month: | ||
| next.setMonth(next.getMonth() + intervalValue); | ||
| return next; | ||
| case MaintenancePlanIntervalUnit.Year: | ||
| next.setFullYear(next.getFullYear() + intervalValue); | ||
| return next; |
Contributor
There was a problem hiding this comment.
Month/year arithmetic will overflow on end-of-month dates 🗓️
setMonth and setFullYear happily roll forward when the current day-of-month doesn't exist in the target month. Examples that will bite users:
Jan 31 + 1 month→Mar 3(orMar 2in leap years), notFeb 28/29.Feb 29, 2024 + 1 year→Mar 1, 2025, notFeb 28, 2025.
Since this drives both the UI preview and (transitively) what users expect their plan cadence to be, it's worth clamping to the last day of the target month for month/year units.
♻️ Suggested clamp helper
+function lastDayOfMonth(year: number, month: number): number {
+ return new Date(year, month + 1, 0).getDate();
+}
+
function addInterval(base: Date, intervalValue: number, intervalUnit: MaintenancePlanIntervalUnit): Date {
const next = new Date(base);
switch (intervalUnit) {
case MaintenancePlanIntervalUnit.Hour:
next.setHours(next.getHours() + intervalValue);
return next;
case MaintenancePlanIntervalUnit.Day:
next.setDate(next.getDate() + intervalValue);
return next;
case MaintenancePlanIntervalUnit.Week:
next.setDate(next.getDate() + intervalValue * 7);
return next;
- case MaintenancePlanIntervalUnit.Month:
- next.setMonth(next.getMonth() + intervalValue);
- return next;
- case MaintenancePlanIntervalUnit.Year:
- next.setFullYear(next.getFullYear() + intervalValue);
- return next;
+ case MaintenancePlanIntervalUnit.Month: {
+ const targetMonth = next.getMonth() + intervalValue;
+ const targetYear = next.getFullYear() + Math.floor(targetMonth / 12);
+ const normalizedMonth = ((targetMonth % 12) + 12) % 12;
+ const day = Math.min(next.getDate(), lastDayOfMonth(targetYear, normalizedMonth));
+ next.setFullYear(targetYear, normalizedMonth, day);
+ return next;
+ }
+ case MaintenancePlanIntervalUnit.Year: {
+ const targetYear = next.getFullYear() + intervalValue;
+ const day = Math.min(next.getDate(), lastDayOfMonth(targetYear, next.getMonth()));
+ next.setFullYear(targetYear, next.getMonth(), day);
+ return next;
+ }
default:
return next;
}
}This is also a good spot to add unit tests for Jan 31 and Feb 29 boundary cases — recurrence.test.ts currently only covers benign weekly/monthly cases.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| case MaintenancePlanIntervalUnit.Month: | |
| next.setMonth(next.getMonth() + intervalValue); | |
| return next; | |
| case MaintenancePlanIntervalUnit.Year: | |
| next.setFullYear(next.getFullYear() + intervalValue); | |
| return next; | |
| function lastDayOfMonth(year: number, month: number): number { | |
| return new Date(year, month + 1, 0).getDate(); | |
| } | |
| function addInterval(base: Date, intervalValue: number, intervalUnit: MaintenancePlanIntervalUnit): Date { | |
| const next = new Date(base); | |
| switch (intervalUnit) { | |
| case MaintenancePlanIntervalUnit.Hour: | |
| next.setHours(next.getHours() + intervalValue); | |
| return next; | |
| case MaintenancePlanIntervalUnit.Day: | |
| next.setDate(next.getDate() + intervalValue); | |
| return next; | |
| case MaintenancePlanIntervalUnit.Week: | |
| next.setDate(next.getDate() + intervalValue * 7); | |
| return next; | |
| case MaintenancePlanIntervalUnit.Month: { | |
| const targetMonth = next.getMonth() + intervalValue; | |
| const targetYear = next.getFullYear() + Math.floor(targetMonth / 12); | |
| const normalizedMonth = ((targetMonth % 12) + 12) % 12; | |
| const day = Math.min(next.getDate(), lastDayOfMonth(targetYear, normalizedMonth)); | |
| next.setFullYear(targetYear, normalizedMonth, day); | |
| return next; | |
| } | |
| case MaintenancePlanIntervalUnit.Year: { | |
| const targetYear = next.getFullYear() + intervalValue; | |
| const day = Math.min(next.getDate(), lastDayOfMonth(targetYear, next.getMonth())); | |
| next.setFullYear(targetYear, next.getMonth(), day); | |
| return next; | |
| } | |
| default: | |
| return next; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/lib/maintenance/recurrence.ts` around lines 15 - 20, The Month/Year
branches using next.setMonth(...) and next.setFullYear(...) can overflow for
end-of-month dates (e.g., Jan 31 + 1 month or Feb 29 + 1 year); modify the
MaintenancePlanIntervalUnit.Month and .Year handling to clamp the resulting date
to the last day of the target month: compute the target year/month (for Year add
to getFullYear, for Month add to getMonth), determine that month’s max day
(e.g., lastDay = new Date(year, month+1, 0).getDate()), and set
next.setDate(Math.min(next.getDate(), lastDay)) after adjusting month/year so
the day is clamped correctly; add small unit tests covering Jan 31 + 1 month and
Feb 29 + 1 year in recurrence.test.ts to validate behavior.
3 tasks
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/api/handlers/v1/v1_ctrl_maintenance_plan.go (2)
28-28: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse
adapters.CommandIDfor this GET endpoint instead ofadapters.QueryID.This endpoint has a UUID path param and no query params, so
CommandIDis the expected adapter here.As per coding guidelines, "Use
adapters.Commandfor GET endpoints with no body and no query parameters" and "Useadapters.CommandIDfor GET endpoints that retrieve a single item by UUID path parameter."🤖 Prompt for 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. In `@backend/app/api/handlers/v1/v1_ctrl_maintenance_plan.go` at line 28, The handler currently uses adapters.QueryID but this endpoint is a GET with a UUID path param and no query params, so replace the call to adapters.QueryID("id", fn, http.StatusOK) with adapters.CommandID("id", fn, http.StatusOK) in v1_ctrl_maintenance_plan.go; ensure the same handler function reference (fn) and status code (http.StatusOK) are retained when swapping to adapters.CommandID so routing and behavior remain unchanged.
42-45:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftEnforce group scoping on create/update/delete to prevent cross-tenant access.
CreatePlan,UpdatePlan, andDeletePlanare called without an explicitauth.GIDscope check in the handler path. This creates a multi-tenancy authorization gap if a valid foreign UUID is supplied.Suggested direction
func (ctrl *V1Controller) HandleMaintenancePlanCreate() errchain.HandlerFunc { fn := func(r *http.Request, itemID uuid.UUID, body repo.MaintenancePlanCreate) (repo.MaintenancePlan, error) { auth := services.NewContext(r.Context()) - return ctrl.repo.MaintEntry.CreatePlan(auth, itemID, body) + return ctrl.repo.MaintEntry.CreatePlanScoped(auth, auth.GID, itemID, body) } return adapters.ActionID("id", fn, http.StatusCreated) }Apply the same scoping pattern to update/delete (or perform an explicit pre-check that the plan belongs to
auth.GIDbefore mutating).As per coding guidelines, "Use
auth.GIDto scope all queries to the user's group for multi-tenancy enforcement."Also applies to: 61-64, 79-82
🤖 Prompt for 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. In `@backend/app/api/handlers/v1/v1_ctrl_maintenance_plan.go` around lines 42 - 45, The handler calls CreatePlan/UpdatePlan/DeletePlan without scoping by the request's group ID, allowing cross-tenant mutations; update the handler closures that call ctrl.repo.MaintEntry.CreatePlan, .UpdatePlan and .DeletePlan to pass auth.GID (from services.NewContext(r.Context())) into the repo call or perform an explicit pre-check that the target plan's GID equals auth.GID before mutating so all repo mutations are scoped to auth.GID; locate the anonymous functions (fn) around the CreatePlan/UpdatePlan/DeletePlan invocations and add the auth.GID parameter or ownership check accordingly.
🤖 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.
Outside diff comments:
In `@backend/app/api/handlers/v1/v1_ctrl_maintenance_plan.go`:
- Line 28: The handler currently uses adapters.QueryID but this endpoint is a
GET with a UUID path param and no query params, so replace the call to
adapters.QueryID("id", fn, http.StatusOK) with adapters.CommandID("id", fn,
http.StatusOK) in v1_ctrl_maintenance_plan.go; ensure the same handler function
reference (fn) and status code (http.StatusOK) are retained when swapping to
adapters.CommandID so routing and behavior remain unchanged.
- Around line 42-45: The handler calls CreatePlan/UpdatePlan/DeletePlan without
scoping by the request's group ID, allowing cross-tenant mutations; update the
handler closures that call ctrl.repo.MaintEntry.CreatePlan, .UpdatePlan and
.DeletePlan to pass auth.GID (from services.NewContext(r.Context())) into the
repo call or perform an explicit pre-check that the target plan's GID equals
auth.GID before mutating so all repo mutations are scoped to auth.GID; locate
the anonymous functions (fn) around the CreatePlan/UpdatePlan/DeletePlan
invocations and add the auth.GID parameter or ownership check accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3736ac94-dc07-4664-af3b-12b2ad56c6d2
⛔ Files ignored due to path filters (17)
backend/go.sumis excluded by!**/*.sumbackend/internal/data/ent/client.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/ent.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/entity.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/entity/entity.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/entity/where.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/entity_create.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/entity_update.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/has_id.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/hook/hook.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/migrate/schema.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/mutation.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/predicate/predicate.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/runtime.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/entity.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/maintenance_plan.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/tx.gois excluded by!backend/internal/data/ent/**
📒 Files selected for processing (16)
backend/app/api/handlers/v1/v1_ctrl_maintenance_plan.gobackend/app/api/routes.gobackend/internal/data/migrations/postgres/20260424120001_add_maintenance_plans.sqlbackend/internal/data/migrations/sqlite3/20260424120000_add_maintenance_plans.sqlbackend/internal/data/repo/repo_maintenance_entry.gobackend/internal/data/repo/repo_maintenance_entry_test.gobackend/internal/data/repo/repo_maintenance_plan.gofrontend/components/Maintenance/EditModal.vuefrontend/components/Maintenance/ListView.vuefrontend/components/ui/dialog-provider/DialogProvider.vuefrontend/components/ui/dialog-provider/utils.tsfrontend/lib/api/classes/items.tsfrontend/lib/api/types/data-contracts.tsfrontend/lib/maintenance/recurrence.test.tsfrontend/lib/maintenance/recurrence.tsfrontend/locales/en.json
💤 Files with no reviewable changes (14)
- frontend/lib/maintenance/recurrence.ts
- frontend/lib/maintenance/recurrence.test.ts
- frontend/lib/api/classes/items.ts
- backend/internal/data/migrations/postgres/20260424120001_add_maintenance_plans.sql
- backend/internal/data/migrations/sqlite3/20260424120000_add_maintenance_plans.sql
- frontend/components/ui/dialog-provider/utils.ts
- frontend/components/ui/dialog-provider/DialogProvider.vue
- backend/internal/data/repo/repo_maintenance_entry_test.go
- backend/internal/data/repo/repo_maintenance_entry.go
- frontend/components/Maintenance/ListView.vue
- frontend/components/Maintenance/EditModal.vue
- frontend/lib/api/types/data-contracts.ts
- backend/internal/data/repo/repo_maintenance_plan.go
- frontend/locales/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/api/routes.go
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
1 participant
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.
What type of PR is this?
What this PR does / why we need it:
-Adds recurring-plan creation in the maintenance entry modal via a Recurring Plan checkbox and configurable interval (hour, day, week, month, year) with interval value input.
-Adds a preview of the next 3 scheduled dates in the modal to make recurrence behavior visible before saving.
-Implements rolling schedule behavior: when a recurring maintenance entry is completed, the next scheduled date is generated based on the actual completion date plus configured interval (per the issue discussion "Rolling tasks": Maintenance Plans (automated maintenance scheduling) #172 (comment)).
-Improves maintenance list UX:
-recurring entries are marked with a repeat icon next to dates;
-overdue entries are highlighted with a destructive/red badge and shown as overdue since the due date.
-Extends creation flow by allowing maintenance entry creation directly from /maintenance:
-clicking New opens item-selection, then the existing maintenance modal for that selected item.
-Fixes date handling bug where scheduled date submitted from frontend could be one day earlier than selected; dates are now sent as date-only values to preserve the chosen day.
Which issue(s) this PR fixes:
Fixes #172
Special notes for your reviewer:
(fill-in or delete this section)
I dont have any GO experience, so I relied on an LLM for implementing this feature. Rolling recurring maintenance is a feature that makes HomeBox way more useful for myself and I think for many others. I tested it and find it pretty useful.
Testing
(fill-in or delete this section)
Tested it manually by creating recurring maintenance entries and checking the scheduled dates.