diff --git a/server/router/api/v1/instance_service.go b/server/router/api/v1/instance_service.go index c201455b..c2bc0911 100644 --- a/server/router/api/v1/instance_service.go +++ b/server/router/api/v1/instance_service.go @@ -210,6 +210,7 @@ func (s *APIV1Service) UpdateInstanceSetting(ctx context.Context, request *v1pb. return nil, status.Errorf(codes.FailedPrecondition, "instance setting %q is configured by the deployment", settingKeyString) } + applyInstanceSettingDefaults(request.Setting) // TODO: Apply update_mask if specified _ = request.UpdateMask diff --git a/server/router/api/v1/instance_service_validation.go b/server/router/api/v1/instance_service_validation.go index ac30035b..c0b56972 100644 --- a/server/router/api/v1/instance_service_validation.go +++ b/server/router/api/v1/instance_service_validation.go @@ -12,14 +12,25 @@ import ( v1pb "github.com/usememos/memos/proto/gen/api/v1" storepb "github.com/usememos/memos/proto/gen/store" + "github.com/usememos/memos/store" ) +func applyInstanceSettingDefaults(setting *v1pb.InstanceSetting) { + memoRelatedSetting := setting.GetMemoRelatedSetting() + if memoRelatedSetting == nil || memoRelatedSetting.ContentLengthLimit != 0 { + return + } + memoRelatedSetting.ContentLengthLimit = store.DefaultContentLengthLimit +} + func validateInstanceSetting(setting *v1pb.InstanceSetting) error { key, err := ExtractInstanceSettingKeyFromName(setting.Name) if err != nil { return err } switch key { + case storepb.InstanceSettingKey_MEMO_RELATED.String(): + return validateInstanceMemoRelatedSetting(setting.GetMemoRelatedSetting()) case storepb.InstanceSettingKey_TAGS.String(): return validateInstanceTagsSetting(setting.GetTagsSetting()) case storepb.InstanceSettingKey_ACCESS.String(): @@ -29,6 +40,16 @@ func validateInstanceSetting(setting *v1pb.InstanceSetting) error { } } +func validateInstanceMemoRelatedSetting(setting *v1pb.InstanceSetting_MemoRelatedSetting) error { + if setting == nil { + return errors.New("memo related setting is required") + } + if setting.ContentLengthLimit < store.DefaultContentLengthLimit { + return errors.Errorf("content_length_limit must be at least %d bytes", store.DefaultContentLengthLimit) + } + return nil +} + func validateInstanceAccessSetting(setting *v1pb.InstanceSetting_AccessSetting) error { if setting == nil { return errors.New("access setting is required") diff --git a/server/router/api/v1/test/instance_service_test.go b/server/router/api/v1/test/instance_service_test.go index 758b1d6e..9f564487 100644 --- a/server/router/api/v1/test/instance_service_test.go +++ b/server/router/api/v1/test/instance_service_test.go @@ -7,6 +7,8 @@ import ( "github.com/stretchr/testify/require" colorpb "google.golang.org/genproto/googleapis/type/color" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/fieldmaskpb" v1pb "github.com/usememos/memos/proto/gen/api/v1" @@ -466,6 +468,45 @@ func TestTestInstanceEmailSettingRequiresPasswordWhenSMTPIdentityChanges(t *test func TestUpdateInstanceSetting(t *testing.T) { ctx := context.Background() + t.Run("UpdateInstanceSetting - memo related content length limit", func(t *testing.T) { + ts := NewTestService(t) + defer ts.Cleanup() + + admin, err := ts.CreateHostUser(ctx, "memo-setting-admin") + require.NoError(t, err) + adminCtx := ts.CreateUserContext(ctx, admin.ID) + settingForLimit := func(limit int32) *v1pb.InstanceSetting { + return &v1pb.InstanceSetting{ + Name: "instance/settings/MEMO_RELATED", + Value: &v1pb.InstanceSetting_MemoRelatedSetting_{ + MemoRelatedSetting: &v1pb.InstanceSetting_MemoRelatedSetting{ + ContentLengthLimit: limit, + }, + }, + } + } + + updated, err := ts.Service.UpdateInstanceSetting(adminCtx, &v1pb.UpdateInstanceSettingRequest{Setting: settingForLimit(0)}) + require.NoError(t, err) + require.Equal(t, int32(8192), updated.GetMemoRelatedSetting().GetContentLengthLimit()) + got, err := ts.Service.GetInstanceSetting(ctx, &v1pb.GetInstanceSettingRequest{Name: "instance/settings/MEMO_RELATED"}) + require.NoError(t, err) + require.Equal(t, int32(8192), got.GetMemoRelatedSetting().GetContentLengthLimit()) + + _, err = ts.Service.UpdateInstanceSetting(adminCtx, &v1pb.UpdateInstanceSettingRequest{Setting: settingForLimit(8191)}) + require.Equal(t, codes.InvalidArgument, status.Code(err)) + + for _, limit := range []int32{8192, 16384} { + updated, err := ts.Service.UpdateInstanceSetting(adminCtx, &v1pb.UpdateInstanceSettingRequest{Setting: settingForLimit(limit)}) + require.NoError(t, err, "limit %d", limit) + require.Equal(t, limit, updated.GetMemoRelatedSetting().GetContentLengthLimit()) + + got, err := ts.Service.GetInstanceSetting(ctx, &v1pb.GetInstanceSettingRequest{Name: "instance/settings/MEMO_RELATED"}) + require.NoError(t, err, "limit %d", limit) + require.Equal(t, limit, got.GetMemoRelatedSetting().GetContentLengthLimit()) + } + }) + t.Run("UpdateInstanceSetting - access setting", func(t *testing.T) { ts := NewTestService(t) defer ts.Cleanup() diff --git a/web/src/components/Settings/MemoRelatedSettings.tsx b/web/src/components/Settings/MemoRelatedSettings.tsx index 4a39ac6f..0060d6e8 100644 --- a/web/src/components/Settings/MemoRelatedSettings.tsx +++ b/web/src/components/Settings/MemoRelatedSettings.tsx @@ -20,15 +20,35 @@ import { SettingList, SettingListItem, SettingPanel } from "./SettingList"; import SettingSection from "./SettingSection"; import useInstanceSettingUpdater, { buildInstanceSettingName } from "./useInstanceSettingUpdater"; +const MIN_CONTENT_LENGTH_LIMIT = 8 * 1024; +const MAX_CONTENT_LENGTH_LIMIT = 2_147_483_647; + +const parseContentLengthLimit = (value: string): number | undefined => { + if (value.trim() === "") { + return undefined; + } + + const parsed = Number(value); + if (!Number.isFinite(parsed) || !Number.isInteger(parsed)) { + return undefined; + } + if (parsed < MIN_CONTENT_LENGTH_LIMIT || parsed > MAX_CONTENT_LENGTH_LIMIT) { + return undefined; + } + return parsed; +}; + const MemoRelatedSettings = () => { const t = useTranslate(); const saveInstanceSetting = useInstanceSettingUpdater(); const { memoRelatedSetting: originalSetting } = useInstance(); const [memoRelatedSetting, setMemoRelatedSetting] = useState(originalSetting); + const [contentLengthLimitInput, setContentLengthLimitInput] = useState(String(originalSetting.contentLengthLimit)); const [editingReaction, setEditingReaction] = useState(""); useEffect(() => { setMemoRelatedSetting(originalSetting); + setContentLengthLimitInput(String(originalSetting.contentLengthLimit)); }, [originalSetting]); const updatePartialSetting = (partial: Partial) => { @@ -50,18 +70,34 @@ const MemoRelatedSettings = () => { }; const handleUpdateSetting = async () => { + const contentLengthLimit = parseContentLengthLimit(contentLengthLimitInput); + if (contentLengthLimit === undefined) { + toast.error( + t("setting.memo.content-length-limit-error", { + min: MIN_CONTENT_LENGTH_LIMIT, + max: MAX_CONTENT_LENGTH_LIMIT, + }), + ); + return; + } + if (memoRelatedSetting.reactions.length === 0) { toast.error(t("setting.memo.reactions-required")); return; } + const updatedMemoRelatedSetting = create(InstanceSetting_MemoRelatedSettingSchema, { + ...memoRelatedSetting, + contentLengthLimit, + }); + await saveInstanceSetting({ key: InstanceSetting_Key.MEMO_RELATED, setting: create(InstanceSettingSchema, { name: buildInstanceSettingName(InstanceSetting_Key.MEMO_RELATED), value: { case: "memoRelatedSetting", - value: memoRelatedSetting, + value: updatedMemoRelatedSetting, }, }), errorContext: "Update memo-related settings", @@ -87,11 +123,17 @@ const MemoRelatedSettings = () => { updatePartialSetting({ contentLengthLimit: Number(event.target.value) })} + min={MIN_CONTENT_LENGTH_LIMIT} + max={MAX_CONTENT_LENGTH_LIMIT} + step={1} + required + aria-label={t("setting.memo.content-length-limit")} + value={contentLengthLimitInput} + onChange={(event) => setContentLengthLimitInput(event.target.value)} /> - {t("setting.memo.bytes-unit")} + + {t("setting.memo.content-length-limit-minimum", { min: MIN_CONTENT_LENGTH_LIMIT })} + @@ -142,7 +184,10 @@ const MemoRelatedSettings = () => {
-
diff --git a/web/src/locales/en.json b/web/src/locales/en.json index 2c2c340f..d5c0f34e 100644 --- a/web/src/locales/en.json +++ b/web/src/locales/en.json @@ -651,6 +651,8 @@ "configured-reactions": "Configured reactions", "content-length-limit": "Content length limit (Byte)", "content-length-limit-description": "Maximum memo body size accepted by the server.", + "content-length-limit-error": "Content length limit must be a whole number between {{min}} and {{max}} bytes.", + "content-length-limit-minimum": "Minimum: {{min}} bytes", "double-click-edit-description": "Allow users to open memo editing by double-clicking a memo.", "editing-description": "Control memo editing behavior and server-side content limits.", "editing-title": "Editing", diff --git a/web/tests/memo-related-settings.test.tsx b/web/tests/memo-related-settings.test.tsx new file mode 100644 index 00000000..f6abdaf7 --- /dev/null +++ b/web/tests/memo-related-settings.test.tsx @@ -0,0 +1,90 @@ +import { create } from "@bufbuild/protobuf"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import MemoRelatedSettings from "@/components/Settings/MemoRelatedSettings"; +import { + type InstanceSetting, + InstanceSetting_Key, + InstanceSetting_MemoRelatedSettingSchema, +} from "@/types/proto/api/v1/instance_service_pb"; + +const mocks = vi.hoisted(() => ({ + instance: { + memoRelatedSetting: {}, + updateSetting: vi.fn(), + fetchSetting: vi.fn(), + }, + toastError: vi.fn(), + toastSuccess: vi.fn(), +})); + +vi.mock("@/contexts/InstanceContext", () => ({ + useInstance: () => mocks.instance, +})); + +vi.mock("@/utils/i18n", () => ({ + useTranslate: () => (key: string) => key, +})); + +vi.mock("react-hot-toast", () => ({ + toast: { + error: mocks.toastError, + success: mocks.toastSuccess, + }, +})); + +describe(" content length limit", () => { + beforeEach(() => { + mocks.instance.memoRelatedSetting = create(InstanceSetting_MemoRelatedSettingSchema, { + contentLengthLimit: 32_768, + enableDoubleClickEdit: false, + reactions: ["thumbs-up"], + }); + mocks.instance.updateSetting.mockReset().mockResolvedValue(undefined); + mocks.instance.fetchSetting.mockReset().mockResolvedValue(undefined); + mocks.toastError.mockReset(); + mocks.toastSuccess.mockReset(); + }); + + it.each([8_192, 16_384])("saves a valid %i-byte limit in the memo-related setting", async (contentLengthLimit) => { + render(); + + const input = screen.getByRole("spinbutton", { name: "setting.memo.content-length-limit" }); + expect(input).toHaveAttribute("min", "8192"); + expect(input).toHaveAttribute("max", "2147483647"); + expect(input).toHaveAttribute("step", "1"); + expect(screen.getByText("setting.memo.content-length-limit-minimum")).toBeVisible(); + + fireEvent.change(input, { target: { value: String(contentLengthLimit) } }); + fireEvent.click(screen.getByRole("button", { name: "common.save" })); + + await waitFor(() => expect(mocks.instance.updateSetting).toHaveBeenCalledTimes(1)); + const setting = mocks.instance.updateSetting.mock.calls[0][0] as InstanceSetting; + expect(setting.name).toBe("instance/settings/MEMO_RELATED"); + expect(setting.value.case).toBe("memoRelatedSetting"); + if (setting.value.case !== "memoRelatedSetting") { + throw new Error("Expected memo-related setting payload"); + } + expect(setting.value.value.contentLengthLimit).toBe(contentLengthLimit); + expect(setting.value.value.reactions).toEqual(["thumbs-up"]); + expect(mocks.instance.fetchSetting).toHaveBeenCalledWith(InstanceSetting_Key.MEMO_RELATED); + expect(mocks.toastError).not.toHaveBeenCalled(); + }); + + it.each([ + ["an empty value", ""], + ["a value below the minimum", "8191"], + ["a fractional value", "8192.5"], + ["a value above int32", "2147483648"], + ])("blocks %s before calling the update API", (_scenario, value) => { + render(); + + const input = screen.getByRole("spinbutton", { name: "setting.memo.content-length-limit" }); + fireEvent.change(input, { target: { value } }); + fireEvent.click(screen.getByRole("button", { name: "common.save" })); + + expect(mocks.instance.updateSetting).not.toHaveBeenCalled(); + expect(mocks.instance.fetchSetting).not.toHaveBeenCalled(); + expect(mocks.toastError).toHaveBeenCalledWith("setting.memo.content-length-limit-error"); + }); +});