Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions apps/web/src/components/app/Layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
import { MacroMcpSetupModal } from '@app/features/integrations/mcp-setup/MacroMcpSetupModal';
import { Paywall } from '@app/features/paywall/Paywall';
import { PropertyEditorModal } from '@app/features/property/editor/PropertyEditorModal';
import { CreateReminderModal } from '@app/features/reminders/CreateReminderModal';
import { ReminderComposerModal } from '@app/features/reminders/ReminderComposerModal';
import { useOnboardingV4Flag } from '@app/features/setup/flow/useOnboardingV4Flag';
import { GlobalShareModal } from '@app/features/sharing/global-share-modal/GlobalShareModal';
import { IosShareSheet } from '@app/features/sharing/ios-share-sheet/IosShareSheet';
Expand Down Expand Up @@ -463,7 +463,7 @@ function LayoutInner(props: RouteSectionProps) {
key={ENABLE_REMINDERS_FLAG}
enabledOverride={ENABLE_REMINDERS_OVERRIDE}
>
<CreateReminderModal />
<ReminderComposerModal />
</ShowFeatureFlag>
<Show when={isAddInboxDialogOpen()}>
<AddInboxDialog />
Expand Down
1 change: 1 addition & 0 deletions apps/web/src/features/next-soup/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export { makeCopyEntityIdAction } from './make-copy-entity-id-action';
export { makeCopyLinkAction } from './make-copy-link-action';
export { makeCreateReminderAction } from './make-create-reminder-action';
export { makeDeleteAction } from './make-delete-action';
export { makeEditReminderAction } from './make-edit-reminder-action';
export { makeFavoriteAction } from './make-favorite-action';
export { makeHideCompanyAction } from './make-hide-company-action';
export { makeMarkDoneAction } from './make-mark-done-action';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,218 @@
import type { EntityData, ReminderEntity } from '@entity';
import { beforeEach, describe, expect, it, vi } from 'vitest';

const mocks = vi.hoisted(() => ({
openReminderEditor: vi.fn(),
remindersEnabled: true,
cachedPreview: undefined as { rawName: string; access: string } | undefined,
}));

vi.mock('@app/features/reminders/reminder-composer', () => ({
openReminderEditor: mocks.openReminderEditor,
}));

// The reference name comes from the preview cache the row already populated.
vi.mock('@queries/preview', () => ({
getCachedItemPreview: () => mocks.cachedPreview,
isAccessiblePreviewItem: (item: { access?: string } | undefined) =>
item?.access === 'access',
}));

// Spread the original so the other flags in this module keep working; only the
// reminders gate is driven by the tests. Under vitest MODE is not
// 'development', so the real ENABLE_REMINDERS would resolve to false.
vi.mock('@core/constant/featureFlags', async (importOriginal) => ({
...(await importOriginal<typeof import('@core/constant/featureFlags')>()),
ENABLE_REMINDERS: () => mocks.remindersEnabled,
}));

import { makeEditReminderAction } from './make-edit-reminder-action';

const NEXT_RUN = '2026-08-09T09:00:00.000Z';

const reminder = (overrides: Partial<ReminderEntity> = {}) =>
({
type: 'reminder',
id: 'rem-1',
name: 'Chase the contract',
description: 'Chase the contract',
ownerId: '',
scheduleType: 'once',
nextRunAt: NEXT_RUN,
enabled: true,
...overrides,
}) as ReminderEntity;

const entity = (type: EntityData['type'], id = 'e1') =>
({ type, id, name: 'Thing' }) as EntityData;

describe('makeEditReminderAction', () => {
beforeEach(() => {
vi.clearAllMocks();
mocks.remindersEnabled = true;
mocks.cachedPreview = undefined;
});

it('can run for a one-shot reminder', () => {
expect(makeEditReminderAction().canExecute(reminder())).toBe(true);
});

it('cannot run for anything that is not a reminder', () => {
const { canExecute } = makeEditReminderAction();

expect(canExecute(entity('document'))).toBe(false);
expect(canExecute(entity('email'))).toBe(false);
});

// The composer only speaks one-shot schedules, so editing a recurring
// reminder through it would quietly turn a cron into a single firing.
it('cannot run for a recurring reminder', () => {
const recurring = reminder({
scheduleType: 'recurring',
cron: '0 0 9 * * *',
timezone: 'UTC',
});

expect(makeEditReminderAction().canExecute(recurring)).toBe(false);
});

it('opens the editor with the reminder as the row knows it', () => {
makeEditReminderAction().execute([reminder()]);

expect(mocks.openReminderEditor).toHaveBeenCalledWith({
id: 'rem-1',
description: 'Chase the contract',
remindAt: new Date(NEXT_RUN),
completed: false,
});
});

// A reschedule has to clear the done flag, so the editor needs to know the
// reminder was completed — see reminderEditPatch.
it('reports a completed reminder as completed', () => {
makeEditReminderAction().execute([
reminder({ completedAt: '2026-08-08T10:00:00.000Z' }),
]);

expect(mocks.openReminderEditor).toHaveBeenCalledWith(
expect.objectContaining({ completed: true })
);
});

// Blanking the description in the editor means "name it after what it is
// about", exactly as it does when creating — so the name has to travel with
// the draft.
it('carries the reference name as the blank-description fallback', () => {
mocks.cachedPreview = { rawName: 'Q3 Contract', access: 'access' };

makeEditReminderAction().execute([
reminder({ referencedEntity: { id: 'doc-1', type: 'document' } }),
]);

expect(mocks.openReminderEditor).toHaveBeenCalledWith(
expect.objectContaining({ fallbackDescription: 'Q3 Contract' })
);
});

it('falls back to how lists label an unnamed reference', () => {
mocks.cachedPreview = { rawName: '', access: 'access' };

makeEditReminderAction().execute([
reminder({ referencedEntity: { id: 'thread-1', type: 'email' } }),
]);

expect(mocks.openReminderEditor).toHaveBeenCalledWith(
expect.objectContaining({ fallbackDescription: '(No Subject)' })
);
});

// Without a fallback the editor keeps the existing description, rather than
// renaming the reminder to a placeholder because a lookup missed.
it('carries no fallback for a standalone reminder', () => {
makeEditReminderAction().execute([reminder()]);

expect(mocks.openReminderEditor).toHaveBeenCalledWith(
expect.objectContaining({ fallbackDescription: undefined })
);
});

it('carries no fallback when the reference is not cached', () => {
mocks.cachedPreview = undefined;

makeEditReminderAction().execute([
reminder({ referencedEntity: { id: 'doc-1', type: 'document' } }),
]);

expect(mocks.openReminderEditor).toHaveBeenCalledWith(
expect.objectContaining({ fallbackDescription: undefined })
);
});

it('carries no fallback when the reference is inaccessible', () => {
mocks.cachedPreview = { rawName: 'Secret', access: 'no_access' };

makeEditReminderAction().execute([
reminder({ referencedEntity: { id: 'doc-1', type: 'document' } }),
]);

expect(mocks.openReminderEditor).toHaveBeenCalledWith(
expect.objectContaining({ fallbackDescription: undefined })
);
});

it('does nothing for an empty selection', () => {
makeEditReminderAction().execute([]);

expect(mocks.openReminderEditor).not.toHaveBeenCalled();
});

// The editor asks about one reminder's time, so a multi-select is not a
// batch — the menu only offers it for a single row.
it('uses only the first entity of a multi-selection', () => {
const first = reminder();

makeEditReminderAction().execute([first, reminder({ id: 'rem-2' })]);

expect(mocks.openReminderEditor).toHaveBeenCalledOnce();
expect(mocks.openReminderEditor).toHaveBeenCalledWith(
expect.objectContaining({ id: 'rem-1' })
);
});

it('does not open the editor for a non-reminder', () => {
makeEditReminderAction().execute([entity('document')]);

expect(mocks.openReminderEditor).not.toHaveBeenCalled();
});

it('cannot run when the reminders flag is off', () => {
mocks.remindersEnabled = false;

expect(makeEditReminderAction().canExecute(reminder())).toBe(false);
});

// execute re-checks the gate, so a command-menu entry left over from before
// the flag closed cannot still open the editor.
it('does not open the editor when the reminders flag is off', () => {
mocks.remindersEnabled = false;

makeEditReminderAction().execute([reminder()]);

expect(mocks.openReminderEditor).not.toHaveBeenCalled();
});

// The soup context menu and soup command menu both drive actions through
// executeWithSoup, so the action is unreachable from a list without it.
it('opens the editor when driven from a soup list', async () => {
await makeEditReminderAction().executeWithSoup(
[reminder()],
{} as Parameters<
ReturnType<typeof makeEditReminderAction>['executeWithSoup']
>[1]
);

expect(mocks.openReminderEditor).toHaveBeenCalledWith(
expect.objectContaining({ id: 'rem-1' })
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { openReminderEditor } from '@app/features/reminders/reminder-composer';
import { reminderDescriptionForReference } from '@app/features/reminders/reminder-schedule';
import { ENABLE_REMINDERS } from '@core/constant/featureFlags';
import type { EntityData, ReminderEntity } from '@entity';
import {
getCachedItemPreview,
isAccessiblePreviewItem,
} from '@queries/preview';
import type { SoupState } from '../create-soup-state';

/**
* The description this reminder would get if it were being created now, for a
* blank description to fall back to.
*
* Read from the preview cache rather than fetched: the row the editor was
* opened from has already rendered this name, so it is cached. A miss returns
* undefined and the editor keeps the existing description instead — better than
* blocking on a request to answer a question the user may not even ask.
*/
function fallbackDescriptionFor(entity: ReminderEntity): string | undefined {
const reference = entity.referencedEntity;
if (!reference) return undefined;

const cached = getCachedItemPreview(reference.id);
if (!cached || !isAccessiblePreviewItem(cached)) return undefined;

return reminderDescriptionForReference(cached.rawName, reference.type);
}

/**
* Edit an existing reminder — its description, its time, or both.
*
* `execute` opens the composer prefilled rather than writing anything: both
* answers come from the user, so there is nothing to do until that modal
* resolves. Single-entity only, like creating one.
*
* Recurring reminders are excluded. The composer only speaks one-shot
* schedules, so editing one through it would quietly turn a cron into a single
* firing. Nothing in the product creates a recurring reminder today, so this
* excludes nothing a user can actually reach.
*/
export const makeEditReminderAction = () => {
const canExecute = (entity: EntityData): boolean =>
ENABLE_REMINDERS() &&
entity.type === 'reminder' &&
entity.scheduleType === 'once';

const execute = (entities: EntityData[]) => {
const [entity] = entities;
// Re-checked rather than assumed: a stale command-menu entry could
// otherwise still fire against a row that has since changed.
if (!entity || entity.type !== 'reminder' || !canExecute(entity)) return;

openReminderEditor({
id: entity.id,
description: entity.description,
remindAt: new Date(entity.nextRunAt),
completed: entity.completedAt != null,
fallbackDescription: fallbackDescriptionFor(entity),
});
};

const executeWithSoup = async (entities: EntityData[], _soup: SoupState) => {
// Opening the composer doesn't change the list, so selection and focus are
// left where they are.
execute(entities);
};

return { canExecute, execute, executeWithSoup };
};
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
makeCopyLinkAction,
makeCreateReminderAction,
makeDeleteAction,
makeEditReminderAction,
makeFavoriteAction,
makeMarkDoneAction,
makeMarkNotDoneAction,
Expand Down Expand Up @@ -88,6 +89,7 @@ export const useEntityActionHotkeys = (

const copyEntityIdAction = makeCopyEntityIdAction();
const createReminderAction = makeCreateReminderAction();
const editReminderAction = makeEditReminderAction();

const shareAction = makeShareAction();

Expand Down Expand Up @@ -302,25 +304,48 @@ export const useEntityActionHotkeys = (
tags: [HotkeyTags.SelectionModification],
}).withGroup(group);

// Rename - 'r'
/**
* Whether 'r' should open the reminder editor rather than rename.
*
* The two are mutually exclusive rather than merely unlikely to overlap:
* `renameAction.canExecute` ends at `entity.ownerId === userId()`, and a
* reminder row's `ownerId` is always `''` while `userId()` is a macro id or
* undefined — so rename never claims a reminder, and sharing the key beats
* leaving 'r' dead on one. Its name is its description, which only the
* reminders API can change.
*/
const editsReminder = (): boolean => {
const entities = getEntitiesForAction();
return entities.length === 1 && editReminderAction.canExecute(entities[0]);
};

// Rename - 'r'. Edits the reminder instead when the row is one.
registerHotkey({
hotkey: ['r'],
hotkeyToken: TOKENS.entity.action.rename,
scopeId,
description: () => {
if (editsReminder()) return 'Edit reminder';
const count = getEntitiesForAction().length;
return count > 1 ? 'Rename items' : 'Rename item';
},
keyDownHandler: () => {
const entities = getEntitiesForAction();
if (entities.length === 0) return false;

if (editsReminder()) {
editReminderAction.executeWithSoup(entities, soup);
return true;
}

if (!entities.every(renameAction.canExecute)) return false;

renameAction.executeWithSoup(entities, soup);
return true;
},
condition: () => {
if (condition && !condition()) return false;
if (editsReminder()) return true;
const entities = getEntitiesForAction();
return entities.length > 0 && entities.every(renameAction.canExecute);
},
Expand Down
Loading
Loading