feat(#2147): add reminder UI — email checkbox and settings dialog fields#11
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds automatic reminder fields to document/envelope schemas and editor UI: introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Thank you for creating your first Pull Request and for being a part of the open signing revolution! 💚🚀
Feel free to hop into our community in Discord
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx`:
- Around line 805-835: When the reminder toggle (meta.reminderEnabled) is
flipped, update the form state so disabling clears meta.reminderIntervalDays and
enabling sets a sensible default if it's currently undefined; call
form.setValue('meta.reminderIntervalDays', undefined) when meta.reminderEnabled
becomes false, and when it becomes true call
form.setValue('meta.reminderIntervalDays', 7) only if
form.getValues('meta.reminderIntervalDays') is undefined (also respect
envelopeHasBeenSent if the toggle should be inert when sent). Implement this in
the toggle's onChange handler or a useEffect watching
form.watch('meta.reminderEnabled'), referencing meta.reminderEnabled and
meta.reminderIntervalDays to locate the logic.
- Around line 789-798: The Switch's disabled prop uses non-reactive
form.getValues('meta.envelopeExpirationPeriod'), so replace that with a watched
value: call form.watch('meta.envelopeExpirationPeriod') at the component level
(similar to the existing emailSettings watch near the emailSettings extraction)
and use that watched expirationPeriod in the disabled expression alongside
envelopeHasBeenSent; update both occurrences where
getValues('meta.envelopeExpirationPeriod') is used so the Switch (and any other
UI) reacts immediately when the user toggles expiration.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f6e9667a-7f87-4aa6-8e00-4a7530979595
📒 Files selected for processing (7)
apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsxapps/remix/app/routes/embed+/v2+/authoring+/envelope.create._index.tsxapps/remix/app/routes/embed+/v2+/authoring+/envelope.edit.$id.tsxpackages/lib/types/document-meta.tspackages/lib/types/envelope-editor.tspackages/lib/types/envelope.tspackages/ui/components/document/document-email-checkboxes.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx`:
- Around line 152-159: The current superRefine only checks
data.meta.reminderIntervalDays but doesn't stop reminders when no expiration is
set; update the superRefine validator (the same block using superRefine) to also
check data.meta.reminderEnabled && !data.meta.expiresAt (or the exact expiration
field name used in the schema, e.g., data.meta.expiration) and call ctx.addIssue
with a custom ZodIssueCode and a message like "Expiration is required when
reminders are enabled", targeting the expiration path ['meta',
'<expirationFieldName>']; keep the existing interval check so both expiration
and reminderIntervalDays are required when reminderEnabled is true.
- Around line 814-844: The reminder interval input remains editable when the
expiration is unset; update the rendering/disabled logic in the
EnvelopeEditorSettingsDialog so the interval input respects expiration presence.
Specifically, change the condition around the FormField (currently using
form.watch('meta.reminderEnabled')) to also require a set expiration
(form.watch('meta.expiresAt')) or alternately add to the Input's disabled prop:
disabled={envelopeHasBeenSent || !form.watch('meta.expiresAt')}; target the
FormField with name "meta.reminderIntervalDays" and the Input element to ensure
the interval is either hidden or disabled when expiresAt is not 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c6435da8-52ba-42e2-9b7b-7cdb387d19a4
📒 Files selected for processing (1)
apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx
| }).superRefine((data, ctx) => { | ||
| if (data.meta.reminderEnabled && !data.meta.reminderIntervalDays) { | ||
| ctx.addIssue({ | ||
| code: z.ZodIssueCode.custom, | ||
| message: msg`Reminder interval is required when reminders are enabled`.id, | ||
| path: ['meta', 'reminderIntervalDays'], | ||
| }); | ||
| } |
There was a problem hiding this comment.
Enforce expiration as a required precondition for reminders
Line [153] validates interval presence but does not prevent meta.reminderEnabled = true when no expiration is set. This allows an invalid reminder configuration to be submitted.
🔧 Proposed fix
}).superRefine((data, ctx) => {
- if (data.meta.reminderEnabled && !data.meta.reminderIntervalDays) {
+ if (data.meta.reminderEnabled && !data.meta.envelopeExpirationPeriod) {
+ ctx.addIssue({
+ code: z.ZodIssueCode.custom,
+ message: msg`Set an expiration date to enable automatic reminders`.id,
+ path: ['meta', 'reminderEnabled'],
+ });
+ }
+
+ if (data.meta.reminderEnabled && data.meta.reminderIntervalDays == null) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: msg`Reminder interval is required when reminders are enabled`.id,
path: ['meta', 'reminderIntervalDays'],
});
}
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx`
around lines 152 - 159, The current superRefine only checks
data.meta.reminderIntervalDays but doesn't stop reminders when no expiration is
set; update the superRefine validator (the same block using superRefine) to also
check data.meta.reminderEnabled && !data.meta.expiresAt (or the exact expiration
field name used in the schema, e.g., data.meta.expiration) and call ctx.addIssue
with a custom ZodIssueCode and a message like "Expiration is required when
reminders are enabled", targeting the expiration path ['meta',
'<expirationFieldName>']; keep the existing interval check so both expiration
and reminderIntervalDays are required when reminderEnabled is true.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx (2)
154-161:⚠️ Potential issue | 🟠 MajorEnforce expiration as a required precondition for reminders in schema validation.
Line [155] only enforces interval presence, so
meta.reminderEnabled = truecan still be submitted with no expiration period.🔧 Proposed fix
.superRefine((data, ctx) => { - if (data.meta.reminderEnabled && !data.meta.reminderIntervalDays) { + if (data.meta.reminderEnabled && !data.meta.envelopeExpirationPeriod) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: msg`Set an expiration date to enable automatic reminders`.id, + path: ['meta', 'envelopeExpirationPeriod'], + }); + } + + if (data.meta.reminderEnabled && data.meta.reminderIntervalDays == null) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: msg`Reminder interval is required when reminders are enabled`.id, path: ['meta', 'reminderIntervalDays'], });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx` around lines 154 - 161, The current .superRefine only requires reminderIntervalDays when data.meta.reminderEnabled is true but does not require an expiration field; update the superRefine callback to also check that data.meta.expirationDays (or the project’s expiration field, e.g., data.meta.expirationDate) is present/valid when data.meta.reminderEnabled === true and, if missing, call ctx.addIssue with code z.ZodIssueCode.custom, a helpful message (e.g., "Expiration is required when reminders are enabled"), and path ['meta','expirationDays'] (or ['meta','expirationDate'] to match the actual field name) so both reminderIntervalDays and the expiration field are enforced as required preconditions for reminders.
813-843:⚠️ Potential issue | 🟠 MajorMake reminder interval rendering/disabled state depend on expiration.
Line [813] and Line [829] allow editing
meta.reminderIntervalDaysafter expiration is unset.🔧 Proposed fix
- {form.watch('meta.reminderEnabled') && ( + {form.watch('meta.reminderEnabled') && envelopeExpirationPeriod && ( <FormField control={form.control} name="meta.reminderIntervalDays" render={({ field }) => ( @@ <Input type="number" min={1} max={30} className="bg-background" - disabled={envelopeHasBeenSent} + disabled={envelopeHasBeenSent || !envelopeExpirationPeriod} value={field.value ?? ''}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx` around lines 813 - 843, The reminder interval input currently only checks form.watch('meta.reminderEnabled') and envelopeHasBeenSent; update the render condition and disabled state so the field also depends on whether an expiration is set. Change the conditional that renders meta.reminderIntervalDays to something like form.watch('meta.reminderEnabled') && Boolean(form.watch('meta.expiration')) (or the actual expiration field name used in this form), and set the Input's disabled prop to envelopeHasBeenSent || !Boolean(form.watch('meta.expiration')), ensuring meta.reminderIntervalDays cannot be edited when expiration is unset or the envelope has been sent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx`:
- Around line 154-161: The current .superRefine only requires
reminderIntervalDays when data.meta.reminderEnabled is true but does not require
an expiration field; update the superRefine callback to also check that
data.meta.expirationDays (or the project’s expiration field, e.g.,
data.meta.expirationDate) is present/valid when data.meta.reminderEnabled ===
true and, if missing, call ctx.addIssue with code z.ZodIssueCode.custom, a
helpful message (e.g., "Expiration is required when reminders are enabled"), and
path ['meta','expirationDays'] (or ['meta','expirationDate'] to match the actual
field name) so both reminderIntervalDays and the expiration field are enforced
as required preconditions for reminders.
- Around line 813-843: The reminder interval input currently only checks
form.watch('meta.reminderEnabled') and envelopeHasBeenSent; update the render
condition and disabled state so the field also depends on whether an expiration
is set. Change the conditional that renders meta.reminderIntervalDays to
something like form.watch('meta.reminderEnabled') &&
Boolean(form.watch('meta.expiration')) (or the actual expiration field name used
in this form), and set the Input's disabled prop to envelopeHasBeenSent ||
!Boolean(form.watch('meta.expiration')), ensuring meta.reminderIntervalDays
cannot be edited when expiration is unset or the envelope has been sent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 90787816-e094-4dfc-92ed-ab49c53770c8
📒 Files selected for processing (1)
apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx (2)
813-847:⚠️ Potential issue | 🟡 MinorInterval input should also respect expiration state.
When
envelopeExpirationPeriodis cleared after reminders were enabled, the interval input remains visible and editable while the switch is disabled. This creates an inconsistent UX where the user can edit a field that will fail server-side validation.🔧 Proposed fix
- {form.watch('meta.reminderEnabled') && ( + {form.watch('meta.reminderEnabled') && envelopeExpirationPeriod && ( <FormField control={form.control} name="meta.reminderIntervalDays"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx` around lines 813 - 847, The reminder interval input (FormField with name "meta.reminderIntervalDays") must also respect the envelope expiration state: change the visibility/availability check from only form.watch('meta.reminderEnabled') to require both the reminder toggle and a valid expiration (e.g., form.watch('meta.reminderEnabled') && !!form.watch('meta.envelopeExpirationPeriod')), or alternatively keep it visible but set the Input disabled when envelopeExpirationPeriod is missing; ensure you reference envelopeHasBeenSent and the existing field name "meta.reminderIntervalDays" so the field is either hidden or disabled when the expiration period is cleared to prevent client-side editing that would fail server validation.
154-162: 🧹 Nitpick | 🔵 TrivialAdd client-side validation for expiration requirement.
The
superRefineonly validatesreminderIntervalDayswhenreminderEnabledis true, but doesn't validate thatenvelopeExpirationPeriodis set. This creates a mismatch with the server-side schema (ZDocumentMetaCreateSchemaindocument-meta.ts), which validates both conditions.While the UI disables the switch when no expiration is set (line 804), adding the client-side validation would provide consistent error feedback and catch edge cases.
🔧 Proposed fix
.superRefine((data, ctx) => { + if (data.meta.reminderEnabled && !data.meta.envelopeExpirationPeriod) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: msg`Set an expiration date to enable automatic reminders`.id, + path: ['meta', 'reminderEnabled'], + }); + } + if (data.meta.reminderEnabled && !data.meta.reminderIntervalDays) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: msg`Reminder interval is required when reminders are enabled`.id, path: ['meta', 'reminderIntervalDays'], }); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx` around lines 154 - 162, The superRefine on the Zod schema only enforces reminderIntervalDays when data.meta.reminderEnabled is true; update the same superRefine block in envelope-editor-settings-dialog.tsx to also require data.meta.envelopeExpirationPeriod when reminderEnabled is true by adding a ctx.addIssue with code z.ZodIssueCode.custom, a clear message (e.g. "Expiration period is required when reminders are enabled"), and path ['meta','envelopeExpirationPeriod'] so the client-side validation matches the server ZDocumentMetaCreateSchema behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx`:
- Around line 813-847: The reminder interval input (FormField with name
"meta.reminderIntervalDays") must also respect the envelope expiration state:
change the visibility/availability check from only
form.watch('meta.reminderEnabled') to require both the reminder toggle and a
valid expiration (e.g., form.watch('meta.reminderEnabled') &&
!!form.watch('meta.envelopeExpirationPeriod')), or alternatively keep it visible
but set the Input disabled when envelopeExpirationPeriod is missing; ensure you
reference envelopeHasBeenSent and the existing field name
"meta.reminderIntervalDays" so the field is either hidden or disabled when the
expiration period is cleared to prevent client-side editing that would fail
server validation.
- Around line 154-162: The superRefine on the Zod schema only enforces
reminderIntervalDays when data.meta.reminderEnabled is true; update the same
superRefine block in envelope-editor-settings-dialog.tsx to also require
data.meta.envelopeExpirationPeriod when reminderEnabled is true by adding a
ctx.addIssue with code z.ZodIssueCode.custom, a clear message (e.g. "Expiration
period is required when reminders are enabled"), and path
['meta','envelopeExpirationPeriod'] so the client-side validation matches the
server ZDocumentMetaCreateSchema behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 03590a4f-1f10-47ba-8318-1ed051bce1eb
📒 Files selected for processing (4)
apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsxpackages/lib/types/document-email.tspackages/lib/types/document-meta.tspackages/trpc/server/envelope-router/distribute-envelope.types.ts
…ialog fields - Add ownerReminderDigest checkbox to DocumentEmailCheckboxes - Add reminderEnabled toggle and reminderIntervalDays interval input to envelope settings dialog General tab (disabled when no expiration set or document already sent) - Extend ZDocumentMetaCreateSchema, ZEnvelopeLiteSchema, and ZEnvelopeEditorSchema to include reminder fields - Fix null→undefined coercion for reminderIntervalDays in embedded authoring routes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…er settings - Replace form.getValues() with form.watch() for envelopeExpirationPeriod so the reminder switch disables reactively when the expiration period is cleared - Add superRefine cross-field validation requiring reminderIntervalDays when reminderEnabled is true, preventing silent misconfiguration Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… opt-in default - Extract ZDocumentMetaFieldsSchema so .pick() callers remain unbroken - Add .superRefine() to ZDocumentMetaCreateSchema: requires reminderIntervalDays and envelopeExpirationPeriod when reminderEnabled is true - Change ownerReminderDigest default from true to false (opt-in, not opt-out) - Replace Number() coercion with parseInt() in interval input handler - Update distribute-envelope.types.ts to use ZDocumentMetaFieldsSchema.pick() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
27d32af to
9cd32e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsx (1)
154-162:⚠️ Potential issue | 🟠 MajorEnforce expiration/reminder consistency in the form layer.
Line [154]-Line [162] validates interval only, but Line [813] still renders the interval whenever
reminderEnabledis true. If expiration is cleared, reminders can remain logically “on” in form state and submit into a backend validation failure.🔧 Suggested fix
.superRefine((data, ctx) => { - if (data.meta.reminderEnabled && !data.meta.reminderIntervalDays) { + if (data.meta.reminderEnabled && !data.meta.envelopeExpirationPeriod) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: msg`Set an expiration date to enable automatic reminders`.id, + path: ['meta', 'reminderEnabled'], + }); + } + + if (data.meta.reminderEnabled && data.meta.reminderIntervalDays == null) { ctx.addIssue({ code: z.ZodIssueCode.custom, message: msg`Reminder interval is required when reminders are enabled`.id, path: ['meta', 'reminderIntervalDays'], }); @@ - {form.watch('meta.reminderEnabled') && ( + {form.watch('meta.reminderEnabled') && envelopeExpirationPeriod && ( <FormField @@ - disabled={envelopeHasBeenSent} + disabled={envelopeHasBeenSent || !envelopeExpirationPeriod}Also applies to: 813-847
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/lib/types/document-email.ts`:
- Line 75: The change flips the default of ownerReminderDigest to false, which
breaks opt-out semantics; instead restore the legacy default of true for
ownerReminderDigest in the email settings schema so that missing/partial/invalid
emailSettings continue to enable digests unless explicitly set to false (update
the default for ownerReminderDigest wherever it’s defined in the
document-email.ts schema and any duplicate declaration around the second
occurrence so both places use true), ensuring the opt-out behavior remains
intact.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8c2c45c9-a9f2-4d3e-9e22-ef6103ef29e6
📒 Files selected for processing (9)
apps/remix/app/components/general/envelope-editor/envelope-editor-settings-dialog.tsxapps/remix/app/routes/embed+/v2+/authoring+/envelope.create._index.tsxapps/remix/app/routes/embed+/v2+/authoring+/envelope.edit.$id.tsxpackages/lib/types/document-email.tspackages/lib/types/document-meta.tspackages/lib/types/envelope-editor.tspackages/lib/types/envelope.tspackages/trpc/server/envelope-router/distribute-envelope.types.tspackages/ui/components/document/document-email-checkboxes.tsx
| 'Whether to send a digest email to the document owner when reminders are sent to unsigned recipients. Aggregates all pending documents for the team into a single email.', | ||
| ) | ||
| .default(true), | ||
| .default(false), |
There was a problem hiding this comment.
Default flip likely breaks “opt-out” semantics and legacy behavior
Line 75 and Line 118 switch ownerReminderDigest to default false, so any document with missing/partial/invalid emailSettings now silently disables digest emails. That is a backward-incompatible behavior shift and appears to conflict with the PR objective wording (“allow senders to opt out”).
Suggested fix (preserve opt-out behavior)
- .default(false),
+ .default(true),
...
- ownerReminderDigest: false,
+ ownerReminderDigest: true,Also applies to: 118-118
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/lib/types/document-email.ts` at line 75, The change flips the
default of ownerReminderDigest to false, which breaks opt-out semantics; instead
restore the legacy default of true for ownerReminderDigest in the email settings
schema so that missing/partial/invalid emailSettings continue to enable digests
unless explicitly set to false (update the default for ownerReminderDigest
wherever it’s defined in the document-email.ts schema and any duplicate
declaration around the second occurrence so both places use true), ensuring the
opt-out behavior remains intact.
Summary
ownerReminderDigestcheckbox toDocumentEmailCheckboxes(Email tab) so senders can opt out of the reminder digestreminderEnabledtoggle andreminderIntervalDaysinterval input (1–30 days) to the envelope settings dialog General tab, immediately after the Expiration fieldZDocumentMetaCreateSchema,ZEnvelopeLiteSchema, andZEnvelopeEditorSchemato carry the new fields through the tRPC layernull → undefinedcoercion forreminderIntervalDaysin embedded authoring routesTest plan
npm run buildpasses🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Behavior Changes