feat: add opt-in warranty expiration notifications#1586
Open
elemated wants to merge 3 commits into
Open
Conversation
Contributor
WalkthroughThis PR adds opt-in warranty-expiration notifications. It adds the entity flag and schema column, repository lookup for warranties expiring on a target day, daily notifier output that includes warranty items, and frontend/docs updates for the new setting and message format. ChangesWarranty expiration notifications
Sequence Diagram(s)The daily notifier now looks up expiring warranties before composing the combined message body. sequenceDiagram
participant SendNotifiersToday
participant EntityRepository
participant entities_table
SendNotifiersToday->>SendNotifiersToday: compute warrantyTarget
SendNotifiersToday->>EntityRepository: GetWarrantyExpiringOn(warrantyTarget)
EntityRepository->>entities_table: query WarrantyExpires and notify_warranty_expiration
entities_table-->>EntityRepository: matching rows
EntityRepository-->>SendNotifiersToday: warrantyItems
SendNotifiersToday->>SendNotifiersToday: append maintenance and warranty sections
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/internal/core/services/service_background.go (1)
52-63: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueOptional: a single group's lookup failure aborts notifications for all remaining groups.
If
GetWarrantyExpiringOn(or the existingGetScheduled) returns an error for one group, the earlyreturn errskips every group that hasn't been processed yet in this run. Since this is a daily best-effort batch, consider logging andcontinue-ing to the next group so one bad group doesn't suppress everyone's notifications. This mirrors the pre-existing maintenance behavior, so it's a judgment call.🤖 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/internal/core/services/service_background.go` around lines 52 - 63, The group lookup in service_background.go stops the entire notification run when GetWarrantyExpiringOn (and the existing GetScheduled call) returns an error for one group. Update the loop in the background notification job to treat these as best-effort failures by logging the error with the group context (for example group.ID and group.Name) and then continuing to the next group instead of returning, so one bad group does not block remaining notifications.backend/internal/data/migrations/postgres/20260513000000_add_warranty_notification.sql (1)
1-3: 📐 Maintainability & Code Quality | 🔵 TrivialConsider adding a
-- +goose Downsection for reversibility.The migration lacks a
-- +goose Downblock. While a review of the migrations directory shows that roughly 75% of recent migrations in this repository omit the Down section, including the rollback script for simple transactions likeADD COLUMNis a robust safety measure. It ensuresgoose downworks correctly if a rollback is needed, avoiding manual intervention.For this specific change, a simple reverse statement suffices:
-- +goose Down ALTER TABLE entities DROP COLUMN notify_warranty_expiration;Security Note: Ensure that any default values or sensitive data added in migrations are handled securely and do not inadvertently expose sensitive information in logs or revert to insecure states during rollbacks.
🤖 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/internal/data/migrations/postgres/20260513000000_add_warranty_notification.sql` around lines 1 - 3, The migration currently only defines the Goose Up path and is missing a reversible Goose Down block. Update the migration by adding a `-- +goose Down` section in `20260513000000_add_warranty_notification.sql` that cleanly reverses the `ALTER TABLE entities ADD COLUMN notify_warranty_expiration` change, so `goose down` can rollback the schema using the same migration file.
🤖 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 `@frontend/pages/item/`[id]/index/edit.vue:
- Around line 280-284: The notifyWarrantyExpiration checkbox is currently
available even when the item can never produce a warranty reminder. Update the
edit form logic in the item edit component so this toggle is hidden or disabled
whenever lifetimeWarranty is enabled or warrantyExpires is missing, and make
sure notifyWarrantyExpiration is cleared when those conditions become true. Use
the existing form field definitions around the checkbox and the related item
state handlers in the edit view to keep the behavior consistent.
---
Nitpick comments:
In `@backend/internal/core/services/service_background.go`:
- Around line 52-63: The group lookup in service_background.go stops the entire
notification run when GetWarrantyExpiringOn (and the existing GetScheduled call)
returns an error for one group. Update the loop in the background notification
job to treat these as best-effort failures by logging the error with the group
context (for example group.ID and group.Name) and then continuing to the next
group instead of returning, so one bad group does not block remaining
notifications.
In
`@backend/internal/data/migrations/postgres/20260513000000_add_warranty_notification.sql`:
- Around line 1-3: The migration currently only defines the Goose Up path and is
missing a reversible Goose Down block. Update the migration by adding a `--
+goose Down` section in `20260513000000_add_warranty_notification.sql` that
cleanly reverses the `ALTER TABLE entities ADD COLUMN
notify_warranty_expiration` change, so `goose down` can rollback the schema
using the same migration file.
🪄 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: 97ee3dce-806d-4a0f-826b-40f0f080bdce
⛔ Files ignored due to path filters (18)
backend/app/api/static/docs/docs.gois excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/openapi-3.yamlis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.jsonis excluded by!backend/app/api/static/docs/**backend/app/api/static/docs/swagger.yamlis excluded by!backend/app/api/static/docs/**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/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/runtime.gois excluded by!backend/internal/data/ent/**backend/internal/data/ent/schema/entity.gois excluded by!backend/internal/data/ent/**docs/public/api/openapi-3.0.jsonis excluded by!docs/public/api/**docs/public/api/openapi-3.0.yamlis excluded by!docs/public/api/**docs/public/api/swagger-2.0.jsonis excluded by!docs/public/api/**docs/public/api/swagger-2.0.yamlis excluded by!docs/public/api/**
📒 Files selected for processing (9)
backend/internal/core/services/service_background.gobackend/internal/data/migrations/postgres/20260513000000_add_warranty_notification.sqlbackend/internal/data/migrations/sqlite3/20260513000000_add_warranty_notification.sqlbackend/internal/data/repo/repo_entities.gobackend/internal/data/repo/repo_entities_test.godocs/src/content/docs/en/user-guide/notifiers.mdxfrontend/lib/api/types/data-contracts.tsfrontend/locales/en.jsonfrontend/pages/item/[id]/index/edit.vue
…lifetime warranty
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@frontend/pages/item/`[id]/index/edit.vue:
- Around line 291-295: The current watch on canNotifyWarrantyExpiration only
clears notifyWarrantyExpiration when the boolean flips, so stale invalid opt-ins
can survive hydration and still be sent by saveItem(). Update the logic in
edit.vue to also validate and clear item.value.notifyWarrantyExpiration whenever
the item is hydrated or changed, not just when canNotifyWarrantyExpiration
transitions to false. Use the existing watch(canNotifyWarrantyExpiration, ...)
alongside the item-loading/hydration path so the field is reset as soon as an
invalid warranty state is present.
🪄 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: 07d16ec1-a0f9-492b-9a44-5e91162fbee3
📒 Files selected for processing (1)
frontend/pages/item/[id]/index/edit.vue
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.
Adds a checkbox to an item's warranty details which allows the user to opt in to notifications 30 days before warranty expiration. Uses the same Shoutrrr flow as the existing maintenance reminders.
'Warranty Details' section
EDIT: Based on CodeRabbit review, I also gated the visibility of the checkbox based on 1) valid date, 2) NOT lifetime warranty. Since the notification couldn't fire in these instances anyway, CodeRabbit suggested updating the UI to match. Video of behavior below.
checkbox.hide.mov
Notifier settings
Test notification
What type of PR is this?
What this PR does / why we need it:
Warranty expiration dates are stored in Homebox but seem to be mostly unused, requiring the user to dig into each item's details to find out when the warranty expires. This PR gives user the option to receive a notification 30 days before warranty expiration. This allows them to file a claim, extend coverage, or replace the item before the window ends.
backend
frontend
Testing
To test this, I built it as a docker image and ran it on my live server.
Notes
I think there is room to extend the feature to allow the user to determine how many days before expiry they want a notification. But wanted to keep the scope of these changes as small as possible, and 30 days felt like a good starting point.
Code written with the help of Claude Opus.