feat: custom OOO reason#27943
Conversation
| onSuccess: (_data, variables) => { | ||
| showToast(t("custom_reason_deleted"), "success"); | ||
|
|
||
| const currentReasonId = getValues("reasonId"); |
There was a problem hiding this comment.
This is imp cause when we delete, even after invalidating the list, if the deleted item was previously selected and we click on save anyway, it will throw foreign key error as the data would not be existing by then
| export const outOfOfficeReasonList = async ({ ctx }: GetOptions) => { | ||
| const outOfOfficeReasons = await prisma.outOfOfficeReason.findMany({ | ||
| where: { | ||
| enabled: true, |
There was a problem hiding this comment.
making sure if X has created custom reasons, it does not get showed in Y's list. Only defaults and the user created reasons
| model OutOfOfficeReason { | ||
| id Int @id @default(autoincrement()) | ||
| emoji String | ||
| reason String @unique |
There was a problem hiding this comment.
this was important so that reasons which are created by users as custom, is avl for other users to create too. So made the combo of userId and reason as unique. When userId is null, then only the reasons will be unique and this will only happen in the global reasons (like what we had previously)
|
@bandhan-majumder can you address the Devin comment? |
| const watchedNotes = watch("notes"); | ||
| const hasValidNotes = Boolean(watchedNotes?.trim()); | ||
| // below pattern covers common emoji ranges | ||
| const emojiPattern = /^(?:[\u2700-\u27bf]|(?:\ud83c[\udde6-\uddff]){2}|[\ud800-\udbff][\udc00-\udfff]|[\u0023-\u0039]\ufe0f?\u20e3|\u3299|\u3297|\u303d|\u3030|\u24c2|\ud83c[\udd70-\udd71]|\ud83c[\udd7e-\udd7f]|\ud83c\udd8e|\ud83c[\udd91-\udd9a]|\ud83c[\udde6-\uddff]|\ud83c[\ude01-\ude02]|\ud83c\ude1a|\ud83c\ude2f|\ud83c[\ude32-\ude3a]|\ud83c[\ude50-\ude51]|\u203c|\u2049|[\u25aa-\u25ab]|\u25b6|\u25c0|[\u25fb-\u25fe]|\u00a9|\u00ae|\u2122|\u2139|\ud83c\udc04|[\u2600-\u26FF]|\u2b05|\u2b06|\u2b07|\u2b1b|\u2b1c|\u2b50|\u2b55|\u231a|\u231b|\u2328|\u23cf|[\u23e9-\u23f3]|[\u23f8-\u23fa]|\ud83c\udccf|\u2934|\u2935|[\u2190-\u21ff])+$/; |
There was a problem hiding this comment.
It covers common ranges. Thought about going with something similar to https://stackoverflow.com/a/64034562, but that required es6 or higher, was getting type issues, had to change. Current one works well.
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Yeah, fixed it. Also updated the description accordingly |
There was a problem hiding this comment.
Review: feat: custom OOO reason
Summary
This PR adds custom out-of-office reasons: users can create and delete their own reasons. Uniqueness is enforced per user via a composite unique on (reason, userId); system defaults use userId: null. The booking page shows the custom reason text when "show note publicly" is on.
Verdict: Approve with requested changes. A few bugs and project-rule fixes should be addressed before merge.
Critical / Bugs
1. Syntax error in CreateOrEditOutOfOfficeModal – emoji Input onChange
The emoji <Input> is missing the onChange prop opening. The diff shows:
<Input
value={customEmoji}
const value = e.target.value;
if (value !== "" && !emojiPattern.test(value)) return;
setCustomEmoji(value);
}}There should be an onChange handler. It should look like:
onChange={(e) => {
const value = e.target.value;
if (value !== "" && !emojiPattern.test(value)) return;
setCustomEmoji(value);
}}Without this, the file will not parse. Please fix.
2. Delete handler: Prisma delete where clause
In outOfOfficeDeleteReason.handler.ts you use:
await prisma.outOfOfficeReason.delete({
where: {
id: input.id,
userId: ctx.user.id,
},
});Prisma’s delete requires a where that matches a unique constraint. The schema has @@unique([reason, userId]), not @@unique([id, userId]), so where: { id, userId } is not a valid unique selector. Prisma may reject this at runtime.
Recommendation: Ensure ownership first, then delete by id only:
const reason = await prisma.outOfOfficeReason.findFirst({
where: { id: input.id, userId: ctx.user.id },
});
if (!reason) {
throw new TRPCError({ code: "NOT_FOUND", message: "Custom reason not found" });
}
// ... existing "in use" check ...
await prisma.outOfOfficeReason.delete({ where: { id: input.id } });3. Avoid as any in tests
outOfOfficeDeleteReason.handler.test.ts:mockResolvedValueOnce({} as any)andmockResolvedValueOnce([...] as any).- Prefer a minimal typed shape, e.g.
mockResolvedValueOnce({ id: 5, userId: 4 } as OutOfOfficeReason)or a small test fixture type, instead ofany.
4. Delete handler: error handling and codes
- Catching a generic
Errorand rethrowing withmessage: error.messagecan leak internal messages. Prefer mapping known cases (e.g. "Record not found") to user-facing messages and using a generic message for unexpected errors. - Use
NOT_FOUND(or similar) when the reason doesn’t exist or doesn’t belong to the user, and reserveBAD_REQUESTfor validation / "already in use" cases. That keeps the API clearer for the client.
Minor / Suggestions
5. Formatting in delete handler
- Use a space after
ifand before(:if (existingOutOfOfficeEntryWithCustomReason.length > 0). - Add a newline at end of file (common lint rule).
6. Create handler: translation locale
outOfOfficeCreateReason.handler.ts uses t(defaultReason.reason) to compare against system defaults. Confirm that getTranslation() (or equivalent) is called with the request/session locale so the "already exists as system default" check works for all languages (e.g. "Vacation" vs "Vacaciones"). If server-side i18n is not request-scoped, consider comparing by translation key (defaultReason.reason) instead of translated string where appropriate.
7. OutOfOfficeInSlots: trailing whitespace
The diff adds a blank line with trailing spaces. Remove trailing whitespace on that line.
8. Modal: outOfOfficeReasonList and ctx
Router correctly passes { ctx } to outOfOfficeReasonList; handler correctly accepts GetOptions and filters by userId: null and ctx.user.id. Looks good.
9. Migration
Composite unique on (reason, userId) and migration SQL (drop old unique, create new index) look correct. Note: in PostgreSQL, NULLs are considered distinct in unique indexes, so multiple rows with the same reason and userId = null would still be disallowed; your system defaults have distinct reason values, so this is correct.
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/trpc/server/routers/viewer/ooo/outOfOfficeDeleteReason.handler.ts">
<violation number="1" location="packages/trpc/server/routers/viewer/ooo/outOfOfficeDeleteReason.handler.ts:17">
P2: Delete in-use validation is scoped to current user only, but cross-user references to the same reasonId are possible via create/update path.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@sahitya-chandra applied the suggestions. |
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
What does this PR do?
It lets user allow creating their own custom out of office reason. Users can create, delete the custom reasons. If user tries to create a new reasons which exists (let it be default or the user previously already created), it won't allow as the reason is unique. Before we were having a unique constrain on the reasons but that will limit each user's ability to create their custom reasons. So, now, userId and the reason is compositely unique. If userId is null, that means that is default (current reasons). Added test, related trpc handler and migration to support the feature.
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo (if applicable):
Screencast.From.2026-02-14.01-47-26.mp4
Image Demo (if applicable):
Error messages
Error while creating same reason as default
Error while trying to delete a reason that is already in use
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist
Summary by cubic
Adds custom out-of-office reasons (emoji + short text) and shows them on the booking page when the user opts to show notes publicly. Meets Linear CAL-7199 by letting users create, select, and manage personal reasons.
New Features
Migration
Written for commit cb6bce3. Summary will update on new commits.