Merged
Conversation
Owner
MarshallAsch
commented
Mar 20, 2026
MarshallAsch
commented
- live reload the code when running using the included the compose file instead of needing to rebuild the image
- docs: add admin settings tabs design spec
- docs: add admin settings tabs implementation plan
- feat: add Setting model and AppSettings type
- feat: add getSettings/updateSettings helpers
- feat: add settings API routes (GET, PATCH, test-email)
- fix: add userId guard and validate pressure against persisted settings
- feat: add settings sidebar layout with tab navigation
- feat: add notifications tab with SMTP status and test email
- feat: add inspection & maintenance settings tab
- feat: add cylinder defaults settings tab
- feat: rewrite settings page with tabbed layout
- feat: use configurable cron schedule from settings
- feat: use configurable service pressures from settings
- fix: settings tab switching and migration sync issues
- chore: remove the commented out db sync calls
- feat: add preview pages for the email notification templates
- feat: add email template previews and notification preferences link
kellenwiltshire
approved these changes
Mar 21, 2026
Collaborator
kellenwiltshire
left a comment
kellenwiltshire
left a comment
There was a problem hiding this comment.
Looks good, couple small things
src/app/settings/page.tsx
Outdated
| theme: u.theme, | ||
| role: u.role, | ||
| clientId: u.clientId ?? null, | ||
| clientName: (u as any).client?.name ?? null, |
Collaborator
There was a problem hiding this comment.
We should define the u type on line 67, which would be Profile
Comment on lines
+156
to
+172
| <input | ||
| type='number' | ||
| min={1} | ||
| step={1} | ||
| value={newPressure} | ||
| onChange={(e) => setNewPressure(e.target.value)} | ||
| placeholder='e.g. 3442' | ||
| className='border-border bg-background text-text w-32 rounded-md border px-3 py-1 text-sm outline-none focus:outline-none' | ||
| onKeyDown={(e) => { | ||
| if (e.key === 'Enter') handleAddPressure() | ||
| if (e.key === 'Escape') { | ||
| setShowAddInput(false) | ||
| setNewPressure('') | ||
| } | ||
| }} | ||
| autoFocus | ||
| /> |
Collaborator
There was a problem hiding this comment.
Can we use the NumberInput component here?
Owner
Author
There was a problem hiding this comment.
yup i forgot to ask it to re-use components where possible
… instead of needing to rebuild the image
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace double non-null assertion with explicit userId guard in PATCH - validateSettings now checks defaultServicePressure against the persisted allowedServicePressures list instead of SETTINGS_DEFAULTS Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace hardcoded '0 8 * * *' schedule with a per-minute check against cronHour/cronMinute from the settings table. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove hardcoded defaultValue and isIn validator from cylinder model - Add servicePressureOptions helper to FormConstants - CylinderModal accepts optional allowedServicePressures prop Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Move SETTINGS_TAB enum to shared tabs.ts (server component can't import from 'use client' module) - Remove incorrect 'use server' directive from page.tsx (it's a page component, not a Server Action) - Disable SequelizeAdapter auto-sync to prevent duplicate column errors during migrations Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add email preview API route with sample data for all 4 templates - Add preview links section to Notifications settings tab - Add "Manage notification preferences" footer link to welcome, hydro reminder, and visual reminder email templates Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
MarshallAsch
force-pushed
the
admin-settings-tabs
branch
from
528e0ba to
5a68c44
Compare
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
2 participants
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.