fix(editor): preserve memo timestamp edits when closing popover
This commit is contained in:
parent
3d97b39f2d
commit
0716eaac2c
5 changed files with 300 additions and 12 deletions
|
|
@ -406,6 +406,9 @@ func (s *APIV1Service) UpdateMemo(ctx context.Context, request *v1pb.UpdateMemoR
|
||||||
} else if path == "update_time" {
|
} else if path == "update_time" {
|
||||||
updatedTsSec := time.Now().Unix()
|
updatedTsSec := time.Now().Unix()
|
||||||
if request.Memo.UpdateTime != nil {
|
if request.Memo.UpdateTime != nil {
|
||||||
|
if !request.Memo.UpdateTime.IsValid() {
|
||||||
|
return nil, status.Errorf(codes.InvalidArgument, "update_time is invalid")
|
||||||
|
}
|
||||||
updatedTsSec = request.Memo.UpdateTime.AsTime().Unix()
|
updatedTsSec = request.Memo.UpdateTime.AsTime().Unix()
|
||||||
}
|
}
|
||||||
update.UpdatedTs = &updatedTsSec
|
update.UpdatedTs = &updatedTsSec
|
||||||
|
|
|
||||||
123
server/router/api/v1/test/memo_timestamp_test.go
Normal file
123
server/router/api/v1/test/memo_timestamp_test.go
Normal file
|
|
@ -0,0 +1,123 @@
|
||||||
|
package test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"google.golang.org/grpc/codes"
|
||||||
|
"google.golang.org/grpc/status"
|
||||||
|
"google.golang.org/protobuf/types/known/fieldmaskpb"
|
||||||
|
"google.golang.org/protobuf/types/known/timestamppb"
|
||||||
|
|
||||||
|
apiv1 "github.com/usememos/memos/proto/gen/api/v1"
|
||||||
|
"github.com/usememos/memos/store"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestUpdateMemoTimestamps(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
ts := NewTestService(t)
|
||||||
|
defer ts.Cleanup()
|
||||||
|
user, err := ts.CreateRegularUser(ctx, "timestamp-owner")
|
||||||
|
require.NoError(t, err)
|
||||||
|
userCtx := ts.CreateUserContext(ctx, user.ID)
|
||||||
|
originalCreate := int64(1577836800)
|
||||||
|
originalUpdate := originalCreate + 3600
|
||||||
|
customCreate := originalCreate - 86400
|
||||||
|
customUpdate := originalUpdate - 86400
|
||||||
|
|
||||||
|
for _, tc := range []struct {
|
||||||
|
name string
|
||||||
|
paths []string
|
||||||
|
patch *apiv1.Memo
|
||||||
|
wantCreate int64
|
||||||
|
wantUpdate int64
|
||||||
|
autoUpdate bool
|
||||||
|
}{
|
||||||
|
{"creation only", []string{"create_time"}, &apiv1.Memo{CreateTime: ×tamppb.Timestamp{Seconds: customCreate}}, customCreate, originalUpdate, false},
|
||||||
|
{"modification only", []string{"update_time"}, &apiv1.Memo{UpdateTime: ×tamppb.Timestamp{Seconds: customUpdate}}, originalCreate, customUpdate, false},
|
||||||
|
{"both", []string{"create_time", "update_time"}, &apiv1.Memo{CreateTime: ×tamppb.Timestamp{Seconds: customCreate}, UpdateTime: ×tamppb.Timestamp{Seconds: customUpdate}}, customCreate, customUpdate, false},
|
||||||
|
{"with content", []string{"content", "create_time", "update_time"}, &apiv1.Memo{Content: "changed", CreateTime: ×tamppb.Timestamp{Seconds: customCreate}, UpdateTime: ×tamppb.Timestamp{Seconds: customUpdate}}, customCreate, customUpdate, false},
|
||||||
|
{"automatic modification", []string{"content", "update_time"}, &apiv1.Memo{Content: "changed"}, originalCreate, 0, true},
|
||||||
|
{"unmasked timestamps ignored", []string{"content"}, &apiv1.Memo{Content: "changed", CreateTime: ×tamppb.Timestamp{Seconds: customCreate}, UpdateTime: ×tamppb.Timestamp{Seconds: customUpdate}}, originalCreate, originalUpdate, false},
|
||||||
|
} {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
memo, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{Memo: &apiv1.Memo{
|
||||||
|
Content: "original", Visibility: apiv1.Visibility_PRIVATE,
|
||||||
|
CreateTime: ×tamppb.Timestamp{Seconds: originalCreate}, UpdateTime: ×tamppb.Timestamp{Seconds: originalUpdate},
|
||||||
|
}})
|
||||||
|
require.NoError(t, err)
|
||||||
|
tc.patch.Name = memo.Name
|
||||||
|
before := time.Now().Unix()
|
||||||
|
updated, err := ts.Service.UpdateMemo(userCtx, &apiv1.UpdateMemoRequest{
|
||||||
|
Memo: tc.patch, UpdateMask: &fieldmaskpb.FieldMask{Paths: tc.paths},
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, tc.wantCreate, updated.CreateTime.Seconds)
|
||||||
|
if tc.autoUpdate {
|
||||||
|
require.GreaterOrEqual(t, updated.UpdateTime.Seconds, before)
|
||||||
|
require.LessOrEqual(t, updated.UpdateTime.Seconds, time.Now().Unix())
|
||||||
|
} else {
|
||||||
|
require.Equal(t, tc.wantUpdate, updated.UpdateTime.Seconds)
|
||||||
|
}
|
||||||
|
// Verify both API reads and raw stored values, not just the update response.
|
||||||
|
fetched, err := ts.Service.GetMemo(userCtx, &apiv1.GetMemoRequest{Name: memo.Name})
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, updated.CreateTime, fetched.CreateTime)
|
||||||
|
require.Equal(t, updated.UpdateTime, fetched.UpdateTime)
|
||||||
|
id := parseMemoIDFromNameForTest(t, ts, memo.Name)
|
||||||
|
stored, err := ts.Store.GetMemo(ctx, &store.FindMemo{ID: &id})
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, updated.CreateTime.Seconds, stored.CreatedTs)
|
||||||
|
require.Equal(t, updated.UpdateTime.Seconds, stored.UpdatedTs)
|
||||||
|
wantContent := "original"
|
||||||
|
if tc.patch.Content != "" {
|
||||||
|
wantContent = tc.patch.Content
|
||||||
|
}
|
||||||
|
require.Equal(t, wantContent, stored.Content)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestUpdateMemoRejectsInvalidTimestamps(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
ts := NewTestService(t)
|
||||||
|
defer ts.Cleanup()
|
||||||
|
user, err := ts.CreateRegularUser(ctx, "invalid-timestamp-owner")
|
||||||
|
require.NoError(t, err)
|
||||||
|
userCtx := ts.CreateUserContext(ctx, user.ID)
|
||||||
|
memo, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{Memo: &apiv1.Memo{
|
||||||
|
Content: "original", Visibility: apiv1.Visibility_PRIVATE,
|
||||||
|
}})
|
||||||
|
require.NoError(t, err)
|
||||||
|
for _, path := range []string{"create_time", "update_time"} {
|
||||||
|
for _, timestamp := range []*timestamppb.Timestamp{
|
||||||
|
{Seconds: 253402300800},
|
||||||
|
{Nanos: -1},
|
||||||
|
{Nanos: 1000000000},
|
||||||
|
nil,
|
||||||
|
} {
|
||||||
|
if path == "update_time" && timestamp == nil {
|
||||||
|
continue // An omitted update_time explicitly requests the current time.
|
||||||
|
}
|
||||||
|
t.Run(path+"/"+timestamp.String(), func(t *testing.T) {
|
||||||
|
patch := &apiv1.Memo{Name: memo.Name, Content: "must not persist"}
|
||||||
|
if path == "create_time" {
|
||||||
|
patch.CreateTime = timestamp
|
||||||
|
} else {
|
||||||
|
patch.UpdateTime = timestamp
|
||||||
|
}
|
||||||
|
_, err := ts.Service.UpdateMemo(userCtx, &apiv1.UpdateMemoRequest{
|
||||||
|
Memo: patch, UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"content", path}},
|
||||||
|
})
|
||||||
|
require.Equal(t, codes.InvalidArgument, status.Code(err))
|
||||||
|
fetched, err := ts.Service.GetMemo(userCtx, &apiv1.GetMemoRequest{Name: memo.Name})
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, memo.Content, fetched.Content)
|
||||||
|
require.Equal(t, memo.CreateTime, fetched.CreateTime)
|
||||||
|
require.Equal(t, memo.UpdateTime, fetched.UpdateTime)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
51
store/test/memo_timestamp_test.go
Normal file
51
store/test/memo_timestamp_test.go
Normal file
|
|
@ -0,0 +1,51 @@
|
||||||
|
package test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
|
"github.com/usememos/memos/store"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestMemoTimestampUpdates(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
ctx := context.Background()
|
||||||
|
ts := NewTestingStore(ctx, t)
|
||||||
|
defer ts.Close()
|
||||||
|
user, err := createTestingHostUser(ctx, ts)
|
||||||
|
require.NoError(t, err)
|
||||||
|
originalCreate, originalUpdate := int64(1577836800), int64(1577840400)
|
||||||
|
created, updated := originalCreate-86400, originalUpdate-86400
|
||||||
|
|
||||||
|
for _, tc := range []struct {
|
||||||
|
name string
|
||||||
|
patch *store.UpdateMemo
|
||||||
|
wantCreate int64
|
||||||
|
wantUpdate int64
|
||||||
|
}{
|
||||||
|
{"creation-only", &store.UpdateMemo{CreatedTs: &created}, created, originalUpdate},
|
||||||
|
{"modification-only", &store.UpdateMemo{UpdatedTs: &updated}, originalCreate, updated},
|
||||||
|
{"both", &store.UpdateMemo{CreatedTs: &created, UpdatedTs: &updated}, created, updated},
|
||||||
|
{"unrelated-update", &store.UpdateMemo{Content: new("changed")}, originalCreate, originalUpdate},
|
||||||
|
} {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
memo, err := ts.CreateMemo(ctx, &store.Memo{
|
||||||
|
UID: tc.name, CreatorID: user.ID, Content: "original", Visibility: store.Private,
|
||||||
|
CreatedTs: originalCreate, UpdatedTs: originalUpdate,
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
tc.patch.ID = memo.ID
|
||||||
|
require.NoError(t, ts.UpdateMemo(ctx, tc.patch))
|
||||||
|
got, err := ts.GetMemo(ctx, &store.FindMemo{ID: &memo.ID})
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NotNil(t, got)
|
||||||
|
require.Equal(t, tc.wantCreate, got.CreatedTs)
|
||||||
|
require.Equal(t, tc.wantUpdate, got.UpdatedTs)
|
||||||
|
if tc.patch.Content != nil {
|
||||||
|
require.Equal(t, *tc.patch.Content, got.Content)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
@ -1,4 +1,4 @@
|
||||||
import { type FC, useRef, useState } from "react";
|
import { type FC, useId, useRef, useState } from "react";
|
||||||
import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover";
|
import { Popover, PopoverContent, PopoverTrigger } from "@/components/ui/popover";
|
||||||
import { useTranslate } from "@/utils/i18n";
|
import { useTranslate } from "@/utils/i18n";
|
||||||
import { useEditorContext, useEditorSelector } from "../state";
|
import { useEditorContext, useEditorSelector } from "../state";
|
||||||
|
|
@ -12,9 +12,12 @@ function formatDate(date: Date): string {
|
||||||
|
|
||||||
function parseDate(value: string): Date | undefined {
|
function parseDate(value: string): Date | undefined {
|
||||||
const match = value.match(/^(\d{4})-(\d{2})-(\d{2}) (\d{2}):(\d{2}):(\d{2})$/);
|
const match = value.match(/^(\d{4})-(\d{2})-(\d{2}) (\d{2}):(\d{2}):(\d{2})$/);
|
||||||
if (!match) return undefined;
|
if (!match || Number(match[1]) === 0) return undefined;
|
||||||
const date = new Date(Number(match[1]), Number(match[2]) - 1, Number(match[3]), Number(match[4]), Number(match[5]), Number(match[6]));
|
const date = new Date(0);
|
||||||
return Number.isNaN(date.getTime()) ? undefined : date;
|
date.setFullYear(Number(match[1]), Number(match[2]) - 1, Number(match[3]));
|
||||||
|
date.setHours(Number(match[4]), Number(match[5]), Number(match[6]), 0);
|
||||||
|
// Date normalizes overflowing fields (e.g. February 30) instead of rejecting them.
|
||||||
|
return formatDate(date) === value ? date : undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
const TimestampInput: FC<{
|
const TimestampInput: FC<{
|
||||||
|
|
@ -22,34 +25,38 @@ const TimestampInput: FC<{
|
||||||
date: Date | undefined;
|
date: Date | undefined;
|
||||||
onChange: (date: Date) => void;
|
onChange: (date: Date) => void;
|
||||||
}> = ({ label, date, onChange }) => {
|
}> = ({ label, date, onChange }) => {
|
||||||
|
const id = useId();
|
||||||
const initialValue = useRef(date ? formatDate(date) : "");
|
const initialValue = useRef(date ? formatDate(date) : "");
|
||||||
const [value, setValue] = useState(initialValue.current);
|
const [value, setValue] = useState(initialValue.current);
|
||||||
const [invalid, setInvalid] = useState(false);
|
const [invalid, setInvalid] = useState(false);
|
||||||
|
|
||||||
const handleBlur = () => {
|
const handleChange = (nextValue: string) => {
|
||||||
const parsed = parseDate(value);
|
setValue(nextValue);
|
||||||
if (parsed) {
|
const parsed = parseDate(nextValue);
|
||||||
setInvalid(false);
|
setInvalid(false);
|
||||||
|
if (parsed) {
|
||||||
|
// Closing the popover can unmount the input without firing blur.
|
||||||
|
// Keep valid edits in the editor store as soon as they are entered.
|
||||||
onChange(parsed);
|
onChange(parsed);
|
||||||
} else {
|
|
||||||
setInvalid(true);
|
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="space-y-1">
|
<div className="space-y-1">
|
||||||
<label className="text-xs font-medium text-muted-foreground">
|
<label htmlFor={id} className="text-xs font-medium text-muted-foreground">
|
||||||
{label}
|
{label}
|
||||||
{value !== initialValue.current && <span className="text-primary ml-0.5">*</span>}
|
{value !== initialValue.current && <span className="text-primary ml-0.5">*</span>}
|
||||||
</label>
|
</label>
|
||||||
<input
|
<input
|
||||||
|
id={id}
|
||||||
type="text"
|
type="text"
|
||||||
className="block w-full rounded-md border border-border bg-background px-2 py-1 text-sm font-mono data-[invalid=true]:border-destructive"
|
className="block w-full rounded-md border border-border bg-background px-2 py-1 text-sm font-mono data-[invalid=true]:border-destructive"
|
||||||
data-invalid={invalid}
|
data-invalid={invalid}
|
||||||
|
aria-invalid={invalid}
|
||||||
placeholder={DATETIME_FORMAT}
|
placeholder={DATETIME_FORMAT}
|
||||||
value={value}
|
value={value}
|
||||||
onChange={(e) => setValue(e.target.value)}
|
onChange={(e) => handleChange(e.target.value)}
|
||||||
onBlur={handleBlur}
|
onBlur={() => setInvalid(!parseDate(value))}
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
|
|
|
||||||
104
web/tests/memo-editor-timestamps.test.tsx
Normal file
104
web/tests/memo-editor-timestamps.test.tsx
Normal file
|
|
@ -0,0 +1,104 @@
|
||||||
|
import { create } from "@bufbuild/protobuf";
|
||||||
|
import { timestampFromDate } from "@bufbuild/protobuf/wkt";
|
||||||
|
import { act, fireEvent, render, screen, waitFor } from "@testing-library/react";
|
||||||
|
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
import { TimestampPopover } from "@/components/MemoEditor/components/TimestampPopover";
|
||||||
|
import { memoService } from "@/components/MemoEditor/services/memoService";
|
||||||
|
import { createInitialState, EditorProvider, useEditorContext } from "@/components/MemoEditor/state";
|
||||||
|
import { type Memo, MemoSchema } from "@/types/proto/api/v1/memo_service_pb";
|
||||||
|
|
||||||
|
const clients = vi.hoisted(() => ({ getMemo: vi.fn(), updateMemo: vi.fn() }));
|
||||||
|
vi.mock("@/connect", () => ({
|
||||||
|
attachmentServiceClient: { createAttachment: vi.fn() },
|
||||||
|
memoServiceClient: clients,
|
||||||
|
}));
|
||||||
|
vi.mock("@/utils/i18n", () => ({ useTranslate: () => (key: string) => key }));
|
||||||
|
|
||||||
|
const originalCreate = new Date(2020, 0, 1, 10, 20, 30);
|
||||||
|
const originalUpdate = new Date(2020, 0, 2, 11, 22, 33);
|
||||||
|
const customTime = new Date(2024, 1, 29, 12, 34, 56);
|
||||||
|
let editor: ReturnType<typeof useEditorContext>;
|
||||||
|
|
||||||
|
function Probe() {
|
||||||
|
editor = useEditorContext();
|
||||||
|
return <TimestampPopover />;
|
||||||
|
}
|
||||||
|
|
||||||
|
function renderEditor() {
|
||||||
|
const memo = create(MemoSchema, {
|
||||||
|
name: "memos/timestamps",
|
||||||
|
content: "Original content",
|
||||||
|
createTime: timestampFromDate(originalCreate),
|
||||||
|
updateTime: timestampFromDate(originalUpdate),
|
||||||
|
});
|
||||||
|
clients.getMemo.mockResolvedValue(memo);
|
||||||
|
render(
|
||||||
|
<EditorProvider initialEditorState={{ ...createInitialState(), ...memoService.fromMemo(memo) }}>
|
||||||
|
<Probe />
|
||||||
|
</EditorProvider>,
|
||||||
|
);
|
||||||
|
fireEvent.click(screen.getByRole("button"));
|
||||||
|
return memo;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("memo editor timestamps", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
clients.updateMemo.mockImplementation(async ({ memo }: { memo: Memo }) => memo);
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
[0, "createTime", "create_time"],
|
||||||
|
[1, "updateTime", "update_time"],
|
||||||
|
] as const)("saves %s after dismissing the focused input with Escape", async (index, field, path) => {
|
||||||
|
const memo = renderEditor();
|
||||||
|
const input = screen.getAllByRole("textbox")[index];
|
||||||
|
act(() => input.focus());
|
||||||
|
fireEvent.change(input, { target: { value: "2024-02-29 12:34:56" } });
|
||||||
|
fireEvent.keyDown(input, { key: "Escape" });
|
||||||
|
await waitFor(() => expect(screen.queryByRole("textbox")).not.toBeInTheDocument());
|
||||||
|
|
||||||
|
await memoService.save(editor.getState(), { memoName: memo.name });
|
||||||
|
|
||||||
|
expect(clients.updateMemo).toHaveBeenCalledOnce();
|
||||||
|
const request = clients.updateMemo.mock.calls[0][0];
|
||||||
|
expect(request.updateMask.paths).toEqual([path]);
|
||||||
|
expect(request.memo[field]).toEqual(timestampFromDate(customTime));
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
"2024-02-30 12:34:56",
|
||||||
|
"2024-13-01 12:34:56",
|
||||||
|
"2024-02-29 24:34:56",
|
||||||
|
"invalid",
|
||||||
|
])("rejects invalid local time %s without changing the saved timestamp", async (value) => {
|
||||||
|
const memo = renderEditor();
|
||||||
|
const input = screen.getAllByRole("textbox")[0];
|
||||||
|
fireEvent.change(input, { target: { value } });
|
||||||
|
fireEvent.blur(input);
|
||||||
|
expect(input).toHaveAttribute("data-invalid", "true");
|
||||||
|
expect(editor.getState().timestamps.createTime).toEqual(originalCreate);
|
||||||
|
await memoService.save(editor.getState(), { memoName: memo.name });
|
||||||
|
expect(clients.updateMemo).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("automatically updates the modification time for content edits", async () => {
|
||||||
|
const memo = renderEditor();
|
||||||
|
const state = { ...editor.getState(), content: "Changed content" };
|
||||||
|
await memoService.save(state, { memoName: memo.name });
|
||||||
|
const request = clients.updateMemo.mock.calls[0][0];
|
||||||
|
expect(request.updateMask.paths).toEqual(["content", "update_time"]);
|
||||||
|
expect(request.memo.updateTime).toBeUndefined();
|
||||||
|
expect(request.memo.createTime).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps the explicitly chosen modification time when content also changes", async () => {
|
||||||
|
const memo = renderEditor();
|
||||||
|
const input = screen.getAllByRole("textbox")[1];
|
||||||
|
fireEvent.change(input, { target: { value: "2024-02-29 12:34:56" } });
|
||||||
|
// Saving must see the edit even if the input has not blurred yet.
|
||||||
|
await memoService.save({ ...editor.getState(), content: "Changed content" }, { memoName: memo.name });
|
||||||
|
const request = clients.updateMemo.mock.calls[0][0];
|
||||||
|
expect(request.updateMask.paths).toEqual(["content", "update_time"]);
|
||||||
|
expect(request.memo.updateTime).toEqual(timestampFromDate(customTime));
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue