diff --git a/docs/design/multi-spaces.md b/docs/design/multi-spaces.md index 758d8d32..0a690bbf 100644 --- a/docs/design/multi-spaces.md +++ b/docs/design/multi-spaces.md @@ -93,8 +93,8 @@ Distribution is derived rather than separately configured: - A Space feed first requires active membership, then returns readable memos assigned to that Space that do not have a `COMMENT` relation. Membership does not make another author's assigned `PRIVATE` memo readable. - Direct memo reads evaluate each memo independently, including comments. - A comment or conversation query requires its context memo to be readable, then filters every replying memo by that memo's own audience. -- A `COMMENT` or `REFERENCE` relation and its snippet are returned only when both endpoints are readable. -- Reactions are readable whenever their memo is readable. +- A `COMMENT` or `REFERENCE` relation and its snippet are returned only when both endpoints are readable. A readable comment's `parent` field still identifies its context memo; this identity does not grant access. Clients fetch it independently and render an unavailable context on permission denial or not-found. Bearer-share responses omit this field. +- Reactions are readable whenever their memo is readable. An active reaction creator can withdraw their own reaction even after losing memo access or Space membership. - Public profiles and other public surfaces continue to use `PUBLIC`, not Space placement. ### Participation and governance diff --git a/proto/api/v1/memo_service.proto b/proto/api/v1/memo_service.proto index 0353351a..4d8410d3 100644 --- a/proto/api/v1/memo_service.proto +++ b/proto/api/v1/memo_service.proto @@ -258,8 +258,9 @@ message Memo { // Output only. The computed properties of the memo. Property property = 15 [(google.api.field_behavior) = OUTPUT_ONLY]; - // Output only. The readable context memo of this COMMENT relation, if any. - // This is omitted unless the caller may independently read both memos. + // Output only. The context memo of this COMMENT relation, if any. + // Its identity is returned even when the caller cannot read the parent. + // Fetch the parent independently; this field does not grant read access. // Format: memos/{memo} optional string parent = 16 [ (google.api.field_behavior) = OUTPUT_ONLY, diff --git a/proto/gen/api/v1/memo_service.pb.go b/proto/gen/api/v1/memo_service.pb.go index acd921e9..b8f2df57 100644 --- a/proto/gen/api/v1/memo_service.pb.go +++ b/proto/gen/api/v1/memo_service.pb.go @@ -246,8 +246,9 @@ type Memo struct { Reactions []*Reaction `protobuf:"bytes,14,rep,name=reactions,proto3" json:"reactions,omitempty"` // Output only. The computed properties of the memo. Property *Memo_Property `protobuf:"bytes,15,opt,name=property,proto3" json:"property,omitempty"` - // Output only. The readable context memo of this COMMENT relation, if any. - // This is omitted unless the caller may independently read both memos. + // Output only. The context memo of this COMMENT relation, if any. + // Its identity is returned even when the caller cannot read the parent. + // Fetch the parent independently; this field does not grant read access. // Format: memos/{memo} Parent *string `protobuf:"bytes,16,opt,name=parent,proto3,oneof" json:"parent,omitempty"` // Output only. The snippet of the memo content. Plain text only. diff --git a/proto/gen/openapi.yaml b/proto/gen/openapi.yaml index 613ef741..baadf66e 100644 --- a/proto/gen/openapi.yaml +++ b/proto/gen/openapi.yaml @@ -3997,8 +3997,9 @@ components: readOnly: true type: string description: |- - Output only. The readable context memo of this COMMENT relation, if any. - This is omitted unless the caller may independently read both memos. + Output only. The context memo of this COMMENT relation, if any. + Its identity is returned even when the caller cannot read the parent. + Fetch the parent independently; this field does not grant read access. Format: memos/{memo} snippet: readOnly: true diff --git a/server/router/api/v1/memo_service_converter.go b/server/router/api/v1/memo_service_converter.go index 988c26f7..720ecd46 100644 --- a/server/router/api/v1/memo_service_converter.go +++ b/server/router/api/v1/memo_service_converter.go @@ -66,15 +66,11 @@ func (s *APIV1Service) convertMemoFromStoreWithCreators(ctx context.Context, mem memoMessage.Location = convertLocationFromStore(memo.Payload.Location) } + // Parent identity is part of a readable comment's context. It grants no + // access to the parent; clients resolve it independently and handle denial. if memo.ParentUID != nil { - contextMemo, err := s.Store.GetMemo(ctx, &store.FindMemo{UID: memo.ParentUID}) - if err != nil { - return nil, errors.Wrap(err, "failed to resolve comment context") - } - if contextMemo != nil && s.checkMemoReadAccess(ctx, contextMemo) == nil { - parentName := buildMemoName(*memo.ParentUID) - memoMessage.Parent = &parentName - } + parentName := buildMemoName(*memo.ParentUID) + memoMessage.Parent = &parentName } // Reactions have no independent audience and are readable whenever this diff --git a/server/router/api/v1/test/memo_move_test.go b/server/router/api/v1/test/memo_move_test.go new file mode 100644 index 00000000..1349b65a --- /dev/null +++ b/server/router/api/v1/test/memo_move_test.go @@ -0,0 +1,112 @@ +package test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/fieldmaskpb" + + apiv1 "github.com/usememos/memos/proto/gen/api/v1" + "github.com/usememos/memos/store" +) + +func TestMoveMemoPreservesIndependentConversation(t *testing.T) { + ctx := context.Background() + ts := NewTestService(t) + defer ts.Cleanup() + author, err := ts.CreateRegularUser(ctx, "move-author") + require.NoError(t, err) + commenter, err := ts.CreateRegularUser(ctx, "move-commenter") + require.NoError(t, err) + reader, err := ts.CreateRegularUser(ctx, "move-reader") + require.NoError(t, err) + authorCtx := ts.CreateUserContext(ctx, author.ID) + commenterCtx := ts.CreateUserContext(ctx, commenter.ID) + readerCtx := ts.CreateUserContext(ctx, reader.ID) + source, err := ts.Store.CreateSpace(ctx, &store.Space{UID: "move-source", Title: "Source"}, author.ID) + require.NoError(t, err) + target, err := ts.Store.CreateSpace(ctx, &store.Space{UID: "move-target", Title: "Target"}, author.ID) + require.NoError(t, err) + for _, member := range []*store.SpaceMember{ + {SpaceID: source.ID, UserID: commenter.ID, Role: store.SpaceMemberRoleUser}, + {SpaceID: target.ID, UserID: reader.ID, Role: store.SpaceMemberRoleUser}, + } { + _, err := ts.InviteAndAcceptSpaceMember(ctx, member, author.ID) + require.NoError(t, err) + } + sourceName, targetName := "spaces/"+source.UID, "spaces/"+target.UID + reference, err := ts.Service.CreateMemo(authorCtx, &apiv1.CreateMemoRequest{Memo: &apiv1.Memo{ + Content: "source reference", Space: &sourceName, Visibility: apiv1.Visibility_SPACE, + }}) + require.NoError(t, err) + attachment, err := ts.Service.CreateAttachment(authorCtx, &apiv1.CreateAttachmentRequest{ + Attachment: &apiv1.Attachment{Filename: "proposal.txt", Type: "text/plain", Content: []byte("proposal attachment")}, + }) + require.NoError(t, err) + original, err := ts.Service.CreateMemo(authorCtx, &apiv1.CreateMemoRequest{Memo: &apiv1.Memo{ + Content: "original memo", Space: &sourceName, Visibility: apiv1.Visibility_SPACE, + Attachments: []*apiv1.Attachment{attachment}, + Relations: []*apiv1.MemoRelation{{Type: apiv1.MemoRelation_REFERENCE, RelatedMemo: &apiv1.MemoRelation_Memo{Name: reference.Name}}}, + }}) + require.NoError(t, err) + comment, err := ts.Service.CreateMemoComment(commenterCtx, &apiv1.CreateMemoCommentRequest{ + Name: original.Name, Comment: &apiv1.Memo{Content: "source comment", Space: &sourceName, Visibility: apiv1.Visibility_SPACE}, + }) + require.NoError(t, err) + reaction, err := ts.Service.UpsertMemoReaction(commenterCtx, &apiv1.UpsertMemoReactionRequest{ + Name: original.Name, Reaction: &apiv1.Reaction{ReactionType: "👍"}, + }) + require.NoError(t, err) + + moved, err := ts.Service.UpdateMemo(authorCtx, &apiv1.UpdateMemoRequest{ + Memo: &apiv1.Memo{Name: original.Name, Space: &targetName, Visibility: apiv1.Visibility_SPACE}, + UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"space", "visibility"}}, + }) + require.NoError(t, err) + require.Equal(t, original.Name, moved.Name) + require.Equal(t, original.Creator, moved.Creator) + require.Equal(t, original.CreateTime, moved.CreateTime) + require.Equal(t, original.UpdateTime, moved.UpdateTime) + require.Equal(t, original.Content, moved.Content) + require.Len(t, moved.Attachments, 1) + require.Equal(t, original.Attachments, moved.Attachments) + require.Len(t, moved.Relations, 2, "the author in both Spaces retains references and comments") + require.Len(t, moved.Reactions, 1) + require.Equal(t, reaction.Name, moved.Reactions[0].Name) + + _, err = ts.Service.GetMemo(commenterCtx, &apiv1.GetMemoRequest{Name: original.Name}) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + readableComment, err := ts.Service.GetMemo(commenterCtx, &apiv1.GetMemoRequest{Name: comment.Name}) + require.NoError(t, err) + require.Equal(t, original.Name, readableComment.GetParent(), "parent identity survives lost parent access") + require.Equal(t, sourceName, readableComment.GetSpace()) + require.Equal(t, comment.Visibility, readableComment.Visibility) + require.Equal(t, comment.Content, readableComment.Content) + require.Empty(t, readableComment.Relations, "the inaccessible parent must not appear in relation previews") + + for _, viewer := range []context.Context{authorCtx, readerCtx} { + comments, err := ts.Service.ListMemoComments(viewer, &apiv1.ListMemoCommentsRequest{Name: original.Name}) + require.NoError(t, err) + if viewer == authorCtx { + require.Len(t, comments.Memos, 1) + } else { + require.Empty(t, comments.Memos) + } + } + destinationMemo, err := ts.Service.GetMemo(readerCtx, &apiv1.GetMemoRequest{Name: original.Name}) + require.NoError(t, err) + require.Empty(t, destinationMemo.Relations) + require.Len(t, destinationMemo.Reactions, 1, "existing reactions are visible to new readers") + _, err = ts.Service.GetMemo(readerCtx, &apiv1.GetMemoRequest{Name: comment.Name}) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + _, err = ts.Service.DeleteMemoReaction(readerCtx, &apiv1.DeleteMemoReactionRequest{Name: reaction.Name}) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + _, err = ts.Service.DeleteMemoReaction(commenterCtx, &apiv1.DeleteMemoReactionRequest{Name: reaction.Name}) + require.NoError(t, err, "the creator can withdraw a reaction without parent access") + reactions, err := ts.Service.ListMemoReactions(readerCtx, &apiv1.ListMemoReactionsRequest{Name: original.Name}) + require.NoError(t, err) + require.Empty(t, reactions.Reactions) +} diff --git a/server/router/api/v1/test/memo_service_test.go b/server/router/api/v1/test/memo_service_test.go index 81588bd1..66879c03 100644 --- a/server/router/api/v1/test/memo_service_test.go +++ b/server/router/api/v1/test/memo_service_test.go @@ -1051,11 +1051,11 @@ func TestGetMemoCommentUsesMemoLocalReadAccessAndConcealsContext(t *testing.T) { commentName := createdComment.Name publicComment, err := ts.Service.GetMemo(ctx, &apiv1.GetMemoRequest{Name: commentName}) require.NoError(t, err) - require.Empty(t, publicComment.GetParent(), "unreadable context must not be exposed") + require.Equal(t, parent.Name, publicComment.GetParent(), "parent identity remains available without granting access") otherComment, err := ts.Service.GetMemo(otherCtx, &apiv1.GetMemoRequest{Name: commentName}) require.NoError(t, err) - require.Empty(t, otherComment.GetParent(), "relation context requires both endpoints to be readable") + require.Equal(t, parent.Name, otherComment.GetParent(), "parent identity is independent of parent readability") comment, err := ts.Service.GetMemo(ownerCtx, &apiv1.GetMemoRequest{Name: commentName}) require.NoError(t, err) diff --git a/server/router/api/v1/test/reaction_service_test.go b/server/router/api/v1/test/reaction_service_test.go index 1156ab5a..afa02ee3 100644 --- a/server/router/api/v1/test/reaction_service_test.go +++ b/server/router/api/v1/test/reaction_service_test.go @@ -149,7 +149,7 @@ func TestUpsertMemoReactionRevalidatesSpaceParticipation(t *testing.T) { require.NoError(t, ts.Store.DeleteSpaceMember(ctx, &store.DeleteSpaceMember{SpaceID: space.ID, UserID: member.ID}, owner.ID)) _, err = ts.Service.DeleteMemoReaction(memberCtx, &apiv1.DeleteMemoReactionRequest{Name: memberReaction.Name}) - require.Equal(t, codes.PermissionDenied, status.Code(err)) + require.NoError(t, err, "the creator may withdraw a reaction after leaving") _, err = ts.Service.UpsertMemoReaction(memberCtx, &apiv1.UpsertMemoReactionRequest{ Name: memoName, Reaction: &apiv1.Reaction{ReactionType: "🔥"}, @@ -162,8 +162,7 @@ func TestUpsertMemoReactionRevalidatesSpaceParticipation(t *testing.T) { Role: store.SpaceMemberRoleUser, }, owner.ID) require.NoError(t, err) - _, err = ts.Service.DeleteMemoReaction(memberCtx, &apiv1.DeleteMemoReactionRequest{Name: memberReaction.Name}) - require.NoError(t, err) + _, err = ts.Service.UpsertMemoReaction(memberCtx, &apiv1.UpsertMemoReactionRequest{ Name: memoName, Reaction: &apiv1.Reaction{ReactionType: "🔥"}, diff --git a/store/db/mysql/reaction.go b/store/db/mysql/reaction.go index 195ecd92..470da86c 100644 --- a/store/db/mysql/reaction.go +++ b/store/db/mysql/reaction.go @@ -157,7 +157,7 @@ func (d *DB) deleteReactionAsCreator(ctx context.Context, delete *store.DeleteRe } defer func() { _ = tx.Rollback() }() if delete.Policy != nil { - if err := validateMySQLReactionWritePolicy(ctx, tx, &store.Reaction{ + if err := validateMySQLReactionDeletePolicy(ctx, tx, &store.Reaction{ CreatorID: *delete.ActorUserID, MemoID: *delete.MemoID, Policy: delete.Policy, diff --git a/store/db/mysql/reaction_policy.go b/store/db/mysql/reaction_policy.go index 90dc16c6..093f5335 100644 --- a/store/db/mysql/reaction_policy.go +++ b/store/db/mysql/reaction_policy.go @@ -21,6 +21,18 @@ func validateMySQLReactionWritePolicy(ctx context.Context, tx *sql.Tx, reaction return store.ValidateReactionWriteParticipation(reaction, participation) } +func validateMySQLReactionDeletePolicy(ctx context.Context, tx *sql.Tx, reaction *store.Reaction) error { + policy := reaction.Policy + participation, err := loadMySQLMemoParticipation(ctx, tx, reaction.MemoID, policy.ActorUserID) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return store.ErrReactionMemoNotFound + } + return errors.Wrap(err, "failed to read reaction participation") + } + return store.ValidateReactionWithdrawal(reaction, participation) +} + func mysqlSpaceMemberActive(ctx context.Context, tx *sql.Tx, spaceID, userID int32) (bool, error) { var role store.SpaceMemberRole err := tx.QueryRowContext(ctx, `SELECT role FROM space_member diff --git a/store/db/postgres/reaction.go b/store/db/postgres/reaction.go index c4e7d17f..cbb0b0c1 100644 --- a/store/db/postgres/reaction.go +++ b/store/db/postgres/reaction.go @@ -156,7 +156,7 @@ func (d *DB) deleteReactionAsCreator(ctx context.Context, delete *store.DeleteRe } defer func() { _ = tx.Rollback() }() if delete.Policy != nil { - if err := validatePostgresReactionWritePolicy(ctx, tx, &store.Reaction{ + if err := validatePostgresReactionDeletePolicy(ctx, tx, &store.Reaction{ CreatorID: *delete.ActorUserID, MemoID: *delete.MemoID, Policy: delete.Policy, diff --git a/store/db/postgres/reaction_policy.go b/store/db/postgres/reaction_policy.go index 29273212..7e5976fd 100644 --- a/store/db/postgres/reaction_policy.go +++ b/store/db/postgres/reaction_policy.go @@ -20,3 +20,15 @@ func validatePostgresReactionWritePolicy(ctx context.Context, tx *sql.Tx, reacti } return store.ValidateReactionWriteParticipation(reaction, participation) } + +func validatePostgresReactionDeletePolicy(ctx context.Context, tx *sql.Tx, reaction *store.Reaction) error { + policy := reaction.Policy + participation, err := readPostgresMemoParticipation(ctx, tx, reaction.MemoID, policy.ActorUserID) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return store.ErrReactionMemoNotFound + } + return errors.Wrap(err, "failed to read reaction participation") + } + return store.ValidateReactionWithdrawal(reaction, participation) +} diff --git a/store/db/sqlite/reaction.go b/store/db/sqlite/reaction.go index 902af571..d9a12d46 100644 --- a/store/db/sqlite/reaction.go +++ b/store/db/sqlite/reaction.go @@ -154,7 +154,7 @@ func (d *DB) deleteReactionAsCreator(ctx context.Context, delete *store.DeleteRe } defer func() { _ = tx.Rollback() }() if delete.Policy != nil { - if err := validateSQLiteReactionWritePolicy(ctx, tx, &store.Reaction{ + if err := validateSQLiteReactionDeletePolicy(ctx, tx, &store.Reaction{ CreatorID: *delete.ActorUserID, MemoID: *delete.MemoID, Policy: delete.Policy, diff --git a/store/db/sqlite/reaction_policy.go b/store/db/sqlite/reaction_policy.go index de8fdf20..38f5e8dd 100644 --- a/store/db/sqlite/reaction_policy.go +++ b/store/db/sqlite/reaction_policy.go @@ -21,6 +21,18 @@ func validateSQLiteReactionWritePolicy(ctx context.Context, tx dbExecutor, react return store.ValidateReactionWriteParticipation(reaction, participation) } +func validateSQLiteReactionDeletePolicy(ctx context.Context, tx dbExecutor, reaction *store.Reaction) error { + policy := reaction.Policy + participation, err := loadSQLiteMemoParticipation(ctx, tx, reaction.MemoID, policy.ActorUserID) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return store.ErrReactionMemoNotFound + } + return errors.Wrap(err, "failed to read reaction participation") + } + return store.ValidateReactionWithdrawal(reaction, participation) +} + func sqliteSpaceMemberActive(ctx context.Context, tx dbExecutor, spaceID, userID int32) (bool, error) { var role store.SpaceMemberRole err := tx.QueryRowContext(ctx, `SELECT role FROM space_member diff --git a/store/reaction_policy.go b/store/reaction_policy.go index 1f3ff0a5..1e54db78 100644 --- a/store/reaction_policy.go +++ b/store/reaction_policy.go @@ -25,6 +25,16 @@ func validateReactionWritePolicy(reaction *Reaction) error { // ValidateReactionWriteParticipation applies the reaction actor identity and // memo-local participation rules to state loaded by the write transaction. func ValidateReactionWriteParticipation(reaction *Reaction, snapshot *MemoCommentAuthorizationSnapshot) error { + if err := ValidateReactionWithdrawal(reaction, snapshot); err != nil { + return err + } + return ValidateMemoCommentAuthorization(snapshot) +} + +// ValidateReactionWithdrawal allows an active actor to withdraw their own reaction +// without requiring continued access to or participation in the memo. The driver +// also verifies ownership of the stored reaction in the same transaction. +func ValidateReactionWithdrawal(reaction *Reaction, snapshot *MemoCommentAuthorizationSnapshot) error { if err := validateReactionWritePolicy(reaction); err != nil { return err } @@ -40,5 +50,5 @@ func ValidateReactionWriteParticipation(reaction *Reaction, snapshot *MemoCommen if snapshot.ContextID != reaction.MemoID { return ErrMemoMutationConflict } - return ValidateMemoCommentAuthorization(snapshot) + return nil } diff --git a/store/test/reaction_policy_test.go b/store/test/reaction_policy_test.go index a357d30a..f746b3f8 100644 --- a/store/test/reaction_policy_test.go +++ b/store/test/reaction_policy_test.go @@ -136,7 +136,7 @@ func TestReactionWritePolicyRejectsInactiveActorAndMemo(t *testing.T) { require.ErrorIs(t, err, store.ErrMemoSpaceNotWritable) } -func TestDeleteReactionAtomicallyEnforcesCreatorAndParticipation(t *testing.T) { +func TestDeleteReactionAtomicallyEnforcesCreatorWithoutParticipation(t *testing.T) { ctx := context.Background() ts := NewTestingStore(ctx, t) defer ts.Close() @@ -168,14 +168,8 @@ func TestDeleteReactionAtomicallyEnforcesCreatorAndParticipation(t *testing.T) { err = ts.DeleteReaction(ctx, &store.DeleteReaction{ ID: &reaction.ID, MemoID: &memo.ID, ActorUserID: &member.ID, Policy: reactionWritePolicy(member.ID), }) - require.ErrorIs(t, err, store.ErrMemoSpaceMembershipRequired) - requireReactionPresent(ctx, t, ts, reaction.ID) + require.NoError(t, err, "creators can withdraw reactions after leaving the Space") - _, err = createSpaceMemberForTest(ctx, ts, &store.SpaceMember{SpaceID: space.ID, UserID: member.ID, Role: store.SpaceMemberRoleUser}, owner.ID) - require.NoError(t, err) - require.NoError(t, ts.DeleteReaction(ctx, &store.DeleteReaction{ - ID: &reaction.ID, MemoID: &memo.ID, ActorUserID: &member.ID, Policy: reactionWritePolicy(member.ID), - })) requireReactionMissing(ctx, t, ts, reaction.ID) } diff --git a/web/src/components/AppSidebar/AppSidebar.tsx b/web/src/components/AppSidebar/AppSidebar.tsx index be91ed05..06496cd5 100644 --- a/web/src/components/AppSidebar/AppSidebar.tsx +++ b/web/src/components/AppSidebar/AppSidebar.tsx @@ -269,6 +269,8 @@ const MemoDetailSidebarContent = () => { { @@ -36,6 +39,7 @@ const MemoActionMenu = (props: MemoActionMenuProps) => { // Dialog state const [deleteDialogOpen, setDeleteDialogOpen] = useState(false); + const [moveDialogOpen, setMoveDialogOpen] = useState(false); // Derived state const isComment = Boolean(memo.parent); @@ -45,6 +49,7 @@ const MemoActionMenu = (props: MemoActionMenuProps) => { // Action handlers const { + canMove, handleTogglePinMemoBtnClick, handleEditMemoClick, handleToggleMemoStatusClick, @@ -123,26 +128,39 @@ const MemoActionMenu = (props: MemoActionMenuProps) => { )} - {/* Write actions (non-readonly) */} - {!readonly && ( - <> - {/* Archive/Restore (non-comment) */} - {!isComment && ( - - {isArchived ? : } - {isArchived ? t("common.restore") : t("common.archive")} - - )} + {!readonly && !isComment && ( + + {isArchived ? : } + {isArchived ? t("common.restore") : t("common.archive")} + + )} - {/* Delete */} - - - {t("common.delete")} - - + {(canMove || !readonly) && ( + + + + {t("common.more")} + + + {canMove && ( + setMoveDialogOpen(true)}> + + {t("memo.move.title")} + + )} + {!readonly && ( + + + {t("common.delete")} + + )} + + )} + {moveDialogOpen && } + {/* Delete confirmation dialog */} void }) { + const t = useTranslate(); + const currentUser = useCurrentUser(); + const { data: spaces = [], isPending: loadingSpaces, error: spacesError, refetch } = useSpaces(currentUser?.name); + const { mutateAsync: updateMemo, isPending } = useUpdateMemo(); + const [destination, setDestination] = useState(memo.space || UNASSIGNED); + const [visibility, setVisibility] = useState(memo.visibility); + const [error, setError] = useState(""); + const nextSpace = destination === UNASSIGNED ? undefined : destination; + const duplicateTitles = getDuplicateSpaceTitles(spaces); + const spaceLabel = (space: (typeof spaces)[number]) => + duplicateTitles.has(space.title) ? `${space.title} (${extractSpaceUidFromName(space.name)})` : space.title; + const selectedSpace = spaces.find((space) => space.name === destination); + const destinationLabel = nextSpace + ? selectedSpace + ? spaceLabel(selectedSpace) + : extractSpaceUidFromName(nextSpace) + : t("memo.move.unassigned"); + const visibilityOption = getVisibilityOption(visibility); + const canSubmit = + currentUser?.name === memo.creator && (memo.space || undefined) !== nextSpace && (!nextSpace || !!selectedSpace) && !isPending; + + const handleSubmit = async (event: FormEvent) => { + event.preventDefault(); + if (!canSubmit) return; + setError(""); + try { + await updateMemo({ + update: { name: memo.name, space: nextSpace ?? "", visibility }, + updateMask: memo.visibility === Visibility.SPACE || visibility !== memo.visibility ? ["space", "visibility"] : ["space"], + }); + toast.success(t("memo.move.success")); + onOpenChange(false); + } catch (error) { + setError(getErrorMessage(error)); + void refetch(); + } + }; + + return ( + { + if (!isPending) onOpenChange(open); + }} + > + +
+ + {t("memo.move.title")} + {t("memo.move.description")} + +
+ + + {loadingSpaces &&

{t("space.loading")}

} + {spacesError && ( +
+ {t("space.load-error")}{" "} + +
+ )} +
+
+ + +

+ {visibility === Visibility.SPACE + ? t("memo.move.space-audience", { space: destinationLabel }) + : visibilityOption && t(visibilityOption.descriptionKey)} +

+
+

{t("memo.move.history")}

+ {error && ( +

+ {error} +

+ )} + + + + +
+ + + ); +} diff --git a/web/src/components/MemoActionMenu/hooks.ts b/web/src/components/MemoActionMenu/hooks.ts index 578034d9..b2dcd54e 100644 --- a/web/src/components/MemoActionMenu/hooks.ts +++ b/web/src/components/MemoActionMenu/hooks.ts @@ -4,6 +4,7 @@ import { useCallback } from "react"; import toast from "react-hot-toast"; import { useLocation } from "react-router-dom"; import { useInstance } from "@/contexts/InstanceContext"; +import useCurrentUser from "@/hooks/useCurrentUser"; import { memoKeys, useDeleteMemo, useUpdateMemo } from "@/hooks/useMemoQueries"; import useNavigateTo from "@/hooks/useNavigateTo"; import { userKeys } from "@/hooks/useUserQueries"; @@ -25,6 +26,8 @@ interface UseMemoActionHandlersOptions { export const useMemoActionHandlers = ({ memo, parentPage, onEdit, setDeleteDialogOpen }: UseMemoActionHandlersOptions) => { const t = useTranslate(); const location = useLocation(); + const currentUser = useCurrentUser(); + const canMove = memo.creator === currentUser?.name && !location.pathname.startsWith(ROUTES.SHARED_MEMO); const navigateTo = useNavigateTo(); const queryClient = useQueryClient(); const { profile } = useInstance(); @@ -153,6 +156,7 @@ export const useMemoActionHandlers = ({ memo, parentPage, onEdit, setDeleteDialo }, [memo.name, memo.parent, t, isInMemoDetailPage, parentPage, navigateTo, memoUpdatedCallback, deleteMemo, queryClient]); return { + canMove, handleTogglePinMemoBtnClick, handleEditMemoClick, handleToggleMemoStatusClick, diff --git a/web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx b/web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx index 95e8eaca..d8f8e518 100644 --- a/web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx +++ b/web/src/components/MemoDetailSidebar/MemoDetailSidebar.tsx @@ -19,6 +19,7 @@ import SidebarSection, { SIDEBAR_SECTION_STACK_CLASSES } from "@/components/AppS import { extractHeadings } from "@/components/MemoContent/pipeline"; import { getRelationBuckets, getRelationMemo } from "@/components/MemoMetadata/Relation/relationHelpers"; import { useResolvedRelationMemos } from "@/components/MemoMetadata/Relation/useResolvedRelationMemos"; +import MemoParentPlaceholder, { type MemoParentStatus } from "@/components/MemoParentPlaceholder"; import { createMemoNavigationState } from "@/components/MemoView/navigation"; import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger } from "@/components/ui/dropdown-menu"; import { useInstance } from "@/contexts/InstanceContext"; @@ -36,6 +37,8 @@ import MemoSharePanel from "./MemoSharePanel"; interface Props { memo: Memo; parentMemo?: Memo; + parentStatus?: MemoParentStatus; + onParentRetry?: () => void; parentPage?: string; hasExplicitOrigin?: boolean; commentCount?: number; @@ -86,6 +89,8 @@ const BacklinkRow = ({ const MemoDetailSidebar = ({ memo, parentMemo, + parentStatus, + onParentRetry, parentPage, hasExplicitOrigin = false, commentCount, @@ -120,7 +125,7 @@ const MemoDetailSidebar = ({ ? [] : referenced.flatMap((relation) => { const relatedMemo = getRelationMemo(relation, "referenced"); - return relatedMemo?.name && !relatedMemo.snippet ? [relatedMemo.name] : []; + return relatedMemo?.name ? [relatedMemo.name] : []; }), [forceReadonly, referenced], ); @@ -159,7 +164,7 @@ const MemoDetailSidebar = ({ const parentSnippet = parentMemo ? normalizeSnippet(parentMemo.snippet || parentMemo.content || parentMemo.name) : ""; const showComments = !forceReadonly && commentCount !== undefined && commentCount > 0; const showOnThisMemo = headings.length > 1 || showComments; - const showConnections = !forceReadonly && (!!parentMemo || referenced.length > 0); + const showConnections = !forceReadonly && (!!parentMemo || !!parentStatus || referenced.length > 0); const handleCopyLink = () => { const host = (profile.instanceUrl || window.location.origin).replace(/\/+$/, ""); @@ -210,7 +215,8 @@ const MemoDetailSidebar = ({ {showConnections && ( - {parentMemo && ( + {parentStatus && } + {!parentStatus && parentMemo && ( { const relatedMemo = getRelationMemo(relation, "referenced"); + if (relatedMemo && resolvedMemos[relatedMemo.name] === null) return null; return ( = ({ relations, onRelationsChange, parentPage, memoName }) => { const referenceRelations = useMemo(() => getEditorReferenceRelations(relations, memoName), [relations, memoName]); const relatedMemoNames = useMemo( - () => - referenceRelations.flatMap((relation) => - relation.relatedMemo?.name && !relation.relatedMemo.snippet ? [relation.relatedMemo.name] : [], - ), + () => referenceRelations.flatMap((relation) => (relation.relatedMemo?.name ? [relation.relatedMemo.name] : [])), [referenceRelations], ); const resolvedMemos = useResolvedRelationMemos(relatedMemoNames); @@ -68,6 +65,7 @@ const RelationListEditor: FC = ({ relations, onRelation > {referenceRelations.map((relation) => { const relatedMemo = relation.relatedMemo!; + if (resolvedMemos[relatedMemo.name] === null) return null; const memo = relatedMemo.snippet ? relatedMemo : resolvedMemos[relatedMemo.name] || relatedMemo; return handleDeleteRelation(memo.name)} parentPage={parentPage} />; })} diff --git a/web/src/components/MemoMetadata/Relation/RelationListView.tsx b/web/src/components/MemoMetadata/Relation/RelationListView.tsx index 3030080c..83ed19de 100644 --- a/web/src/components/MemoMetadata/Relation/RelationListView.tsx +++ b/web/src/components/MemoMetadata/Relation/RelationListView.tsx @@ -34,7 +34,7 @@ function RelationListView({ relations, currentMemoName, parentPage, className }: () => activeRelations.flatMap((relation) => { const memo = getRelationMemo(relation, direction); - return memo?.name && !memo.snippet ? [memo.name] : []; + return memo?.name ? [memo.name] : []; }), [activeRelations, direction], ); @@ -75,7 +75,7 @@ function RelationListView({ relations, currentMemoName, parentPage, className }: > {activeRelations.map((relation) => { const memo = getRelationMemo(relation, direction); - if (!memo) { + if (!memo || resolvedMemos[memo.name] === null) { return null; } return ( diff --git a/web/src/components/MemoMetadata/Relation/useResolvedRelationMemos.ts b/web/src/components/MemoMetadata/Relation/useResolvedRelationMemos.ts index 60bb683e..2f95cbfd 100644 --- a/web/src/components/MemoMetadata/Relation/useResolvedRelationMemos.ts +++ b/web/src/components/MemoMetadata/Relation/useResolvedRelationMemos.ts @@ -1,54 +1,24 @@ import { create } from "@bufbuild/protobuf"; -import { useQueryClient } from "@tanstack/react-query"; -import { useEffect, useMemo, useState } from "react"; +import { useQueries, useQueryClient } from "@tanstack/react-query"; import { findMemoInCollectionQueries, memoDetailQueryOptions } from "@/hooks/useMemoQueries"; -import { MemoRelation_Memo, MemoRelation_MemoSchema } from "@/types/proto/api/v1/memo_service_pb"; +import { type MemoRelation_Memo, MemoRelation_MemoSchema } from "@/types/proto/api/v1/memo_service_pb"; export const useResolvedRelationMemos = (memoNames: string[], options?: { enabled?: boolean }) => { - const queryClient = useQueryClient(); - const [resolvedMemos, setResolvedMemos] = useState>({}); - const enabled = options?.enabled ?? true; - - const missingMemoNames = useMemo(() => { - return Array.from(new Set(memoNames)).filter((name) => name && !resolvedMemos[name]); - }, [memoNames, resolvedMemos]); - - useEffect(() => { - if (!enabled || missingMemoNames.length === 0) { - return; + const client = useQueryClient(); + const names = Array.from(new Set(memoNames.filter(Boolean))); + const queries = useQueries({ + queries: names.map((name) => ({ + ...memoDetailQueryOptions(name), + enabled: options?.enabled ?? true, + initialData: () => findMemoInCollectionQueries(client, name, true), + })), + }); + const resolved: Record = {}; + queries.forEach((query, index) => { + if (query.data === null) resolved[names[index]] = null; + else if (query.data) { + resolved[names[index]] = create(MemoRelation_MemoSchema, { name: query.data.name, snippet: query.data.snippet }); } - - let cancelled = false; - - void (async () => { - try { - const memos = await Promise.all( - missingMemoNames.map(async (name) => { - const memo = findMemoInCollectionQueries(queryClient, name) ?? (await queryClient.fetchQuery(memoDetailQueryOptions(name))); - return create(MemoRelation_MemoSchema, { name: memo.name, snippet: memo.snippet }); - }), - ); - - if (cancelled) { - return; - } - - setResolvedMemos((prev) => { - const next = { ...prev }; - for (const memo of memos) { - next[memo.name] = memo; - } - return next; - }); - } catch { - // Keep existing relation data when snippet hydration fails. - } - })(); - - return () => { - cancelled = true; - }; - }, [enabled, missingMemoNames, queryClient]); - - return resolvedMemos; + }); + return resolved; }; diff --git a/web/src/components/MemoParentPlaceholder.tsx b/web/src/components/MemoParentPlaceholder.tsx new file mode 100644 index 00000000..32a799d4 --- /dev/null +++ b/web/src/components/MemoParentPlaceholder.tsx @@ -0,0 +1,19 @@ +import { Button } from "@/components/ui/button"; +import { useTranslate } from "@/utils/i18n"; + +export type MemoParentStatus = "loading" | "unavailable" | "error"; + +export default function MemoParentPlaceholder({ status, onRetry }: { status: MemoParentStatus; onRetry?: () => void }) { + const t = useTranslate(); + return ( +
+

{t(status === "loading" ? "memo.parent-loading" : status === "error" ? "memo.parent-load-error" : "memo.parent-unavailable")}

+ {status === "unavailable" &&

{t("memo.parent-unavailable-description")}

} + {status === "error" && onRetry && ( + + )} +
+ ); +} diff --git a/web/src/contexts/AppSidebarContext.tsx b/web/src/contexts/AppSidebarContext.tsx index 64213502..3d97018f 100644 --- a/web/src/contexts/AppSidebarContext.tsx +++ b/web/src/contexts/AppSidebarContext.tsx @@ -1,5 +1,6 @@ import { createContext, type ReactNode, useCallback, useContext, useEffect, useMemo, useRef, useState } from "react"; import { useLocation } from "react-router-dom"; +import type { MemoParentStatus } from "@/components/MemoParentPlaceholder"; import type { PrimaryMemoScope } from "@/lib/memo-views"; import type { Memo } from "@/types/proto/api/v1/memo_service_pb"; @@ -9,6 +10,8 @@ export type InboxFilter = "all" | "unread" | "archived"; export interface MemoDetailSidebarDescriptor { memo: Memo; parentMemo?: Memo; + parentStatus?: MemoParentStatus; + onParentRetry?: () => void; from?: string; hasExplicitOrigin?: boolean; commentCount?: number; diff --git a/web/src/hooks/useMemoQueries.ts b/web/src/hooks/useMemoQueries.ts index 412ed34c..fecb5c2b 100644 --- a/web/src/hooks/useMemoQueries.ts +++ b/web/src/hooks/useMemoQueries.ts @@ -1,5 +1,6 @@ import { create } from "@bufbuild/protobuf"; import { FieldMaskSchema } from "@bufbuild/protobuf/wkt"; +import { Code, ConnectError } from "@connectrpc/connect"; import { type InfiniteData, type QueryClient, @@ -13,6 +14,7 @@ import { memoServiceClient } from "@/connect"; import { attachmentKeys } from "@/hooks/useAttachmentQueries"; import { userKeys } from "@/hooks/useUserQueries"; import { DEFAULT_LIST_MEMOS_PAGE_SIZE } from "@/lib/constants"; +import { shouldRetry } from "@/lib/query-client"; import type { ListMemosRequest, ListMemosResponse, Memo } from "@/types/proto/api/v1/memo_service_pb"; import { ListMemoCommentsRequestSchema, ListMemosRequestSchema, MemoSchema } from "@/types/proto/api/v1/memo_service_pb"; @@ -30,10 +32,47 @@ export const memoKeys = { export const memoDetailQueryOptions = (name: string) => queryOptions({ queryKey: memoKeys.detail(name), - queryFn: () => memoServiceClient.getMemo({ name }), + queryFn: async ({ client, signal }) => { + try { + return await memoServiceClient.getMemo({ name }, { signal }); + } catch (error) { + if (!isMemoUnavailableError(error)) throw error; + // Store a content-free result, replacing any formerly readable detail. + // Remove copies and snippets so collection fallbacks cannot revive them. + discardUnavailableMemo(client, name); + return null; + } + }, + retry: shouldRetry, staleTime: 1000 * 10, + // A previous denial cannot establish access on a new visit, even while fresh. + refetchOnMount: (query) => (query.state.data === null ? "always" : true), }); +export function isMemoUnavailableError(error: unknown): boolean { + return error instanceof ConnectError && (error.code === Code.PermissionDenied || error.code === Code.NotFound); +} + +function discardUnavailableMemo(client: QueryClient, name: string) { + const stripRelations = (memo: Memo): Memo => { + const relations = memo.relations.filter((relation) => relation.memo?.name !== name && relation.relatedMemo?.name !== name); + return relations.length === memo.relations.length ? memo : { ...memo, relations }; + }; + const stripList = (data: ListMemosResponse): ListMemosResponse => { + const memos = data.memos.filter((memo) => memo.name !== name).map(stripRelations); + return memos.length === data.memos.length && memos.every((memo, index) => memo === data.memos[index]) ? data : { ...data, memos }; + }; + for (const [key, data] of client.getQueriesData({ queryKey: memoKeys.all })) { + let next = data; + if (isMemoListResponse(data)) next = stripList(data); + else if (isInfiniteMemoListData(data)) { + const pages = data.pages.map(stripList); + if (pages.some((page, index) => page !== data.pages[index])) next = { ...data, pages }; + } else if (key[1] === "detail" && data && "name" in data && data.name !== name) next = stripRelations(data); + if (next !== data) client.setQueryData(key, next); + } +} + type MemoPatch = Partial & Pick; type MemoCollectionQueryData = ListMemosResponse | InfiniteData; @@ -109,8 +148,11 @@ function findMemoInQueryData(data: unknown, name: string): Memo | undefined { return undefined; } -export function findMemoInCollectionQueries(queryClient: QueryClient, name: string): Memo | undefined { - for (const [, data] of queryClient.getQueriesData({ queryKey: memoKeys.all })) { +export function findMemoInCollectionQueries(queryClient: QueryClient, name: string, freshOnly = false): Memo | undefined { + if (queryClient.getQueryData(memoKeys.detail(name)) === null) return undefined; + for (const [key, data] of queryClient.getQueriesData({ queryKey: memoKeys.all })) { + const state = queryClient.getQueryState(key); + if (freshOnly && (!state || state.isInvalidated || state.dataUpdatedAt < Date.now() - 10_000)) continue; const memo = findMemoInQueryData(data, name); if (memo) { return memo; @@ -155,10 +197,11 @@ export function useInfiniteMemos(request: Partial = {}, option } export function useMemo(name: string, options?: { enabled?: boolean }) { - return useQuery({ + const query = useQuery({ ...memoDetailQueryOptions(name), enabled: options?.enabled ?? true, }); + return { ...query, data: query.data ?? undefined, isUnavailable: query.data === null }; } function isHTTPURL(url: string): boolean { @@ -217,7 +260,8 @@ export function useUpdateMemo() { }); return memo; }, - onMutate: async ({ update }) => { + onMutate: async ({ update, updateMask }) => { + if (updateMask.includes("space")) return { previousMemo: undefined }; if (!update.name) { return { previousMemo: undefined }; } @@ -247,7 +291,10 @@ export function useUpdateMemo() { queryClient.invalidateQueries({ queryKey: memoKeys.all }); } }, - onSuccess: (updatedMemo) => { + onSuccess: (updatedMemo, { updateMask }) => { + if (updateMask.includes("space")) { + queryClient.invalidateQueries({ queryKey: memoKeys.all }); + } // Update cache with server response queryClient.setQueryData(memoKeys.detail(updatedMemo.name), updatedMemo); patchMemoInCollectionQueries(queryClient, updatedMemo); diff --git a/web/src/locales/az.json b/web/src/locales/az.json index 9a449a7b..2f157f32 100644 --- a/web/src/locales/az.json +++ b/web/src/locales/az.json @@ -349,6 +349,19 @@ "content-syntax": "Məzmun sintaksisi" }, "memo": { + "parent-loading": "İlkin memo yüklənir…", + "parent-load-error": "İlkin memonu yükləmək mümkün olmadı.", + "parent-unavailable": "İlkin memo əlçatan deyil", + "parent-unavailable-description": "Silinmiş ola bilər və ya artıq giriş icazəniz yoxdur.", + "move": { + "title": "Məkana köçür", + "description": "Bu memonun yerini və onu kimin oxuya biləcəyini seçin.", + "unassigned": "Təyin edilməyib", + "confirm": "Köçür", + "success": "Memo köçürüldü", + "space-audience": "{{space}} məkanının üzvləri bu memonu oxuya biləcəklər.", + "history": "Mövcud reaksiyalar bu memo ilə qalır. Şərhlər və əlaqəli memolar öz giriş qaydalarını saxlayır." + }, "archived-at": "Arxivlənib", "back-to": "{{source}} bölməsinə qayıt", "click-to-hide-sensitive-content": "Həssas məzmunu gizlətmək üçün klikləyin", diff --git a/web/src/locales/en.json b/web/src/locales/en.json index 9c793af0..36d8f242 100644 --- a/web/src/locales/en.json +++ b/web/src/locales/en.json @@ -349,6 +349,19 @@ "content-syntax": "Content syntax" }, "memo": { + "parent-loading": "Loading original memo…", + "parent-load-error": "Couldn’t load the original memo.", + "parent-unavailable": "Original memo unavailable", + "parent-unavailable-description": "It may have been deleted or you may no longer have access.", + "move": { + "title": "Move to Space", + "description": "Choose where this memo belongs and who can read it.", + "unassigned": "Unassigned", + "confirm": "Move", + "success": "Memo moved", + "space-audience": "Members of {{space}} will be able to read this memo.", + "history": "Existing reactions stay with this memo. Comments and linked memos keep their own access rules." + }, "archived-at": "Archived at", "back-to": "Back to {{source}}", "click-to-hide-sensitive-content": "Click to hide sensitive content", diff --git a/web/src/pages/MemoDetail.tsx b/web/src/pages/MemoDetail.tsx index 9a569f4a..b2c83cf6 100644 --- a/web/src/pages/MemoDetail.tsx +++ b/web/src/pages/MemoDetail.tsx @@ -4,6 +4,7 @@ import { useCallback, useEffect, useMemo as useReactMemo, useRef, useState } fro import { Link, Navigate, useLocation, useParams } from "react-router-dom"; import MemoCommentSection, { type MemoCommentSectionHandle } from "@/components/MemoCommentSection"; import { MentionResolutionProvider } from "@/components/MemoContent/MentionResolutionContext"; +import MemoParentPlaceholder, { type MemoParentStatus } from "@/components/MemoParentPlaceholder"; import MemoView, { type MemoViewHandle } from "@/components/MemoView"; import { computeCommentAmount } from "@/components/MemoView/MemoViewContext"; import { createMemoNavigationState, resolveMemoDetailOrigin } from "@/components/MemoView/navigation"; @@ -24,6 +25,8 @@ import { findMemoAnchorTarget } from "@/utils/markdown-manipulation"; const MemoSidebarRegistration = ({ memo, parentMemo, + parentStatus, + onParentRetry, from, hasExplicitOrigin, commentCount, @@ -35,6 +38,8 @@ const MemoSidebarRegistration = ({ }: { memo: Memo; parentMemo?: Memo; + parentStatus?: MemoParentStatus; + onParentRetry?: () => void; from: string; hasExplicitOrigin: boolean; commentCount?: number; @@ -50,6 +55,8 @@ const MemoSidebarRegistration = ({ setMemoDetail({ memo, parentMemo, + parentStatus, + onParentRetry, from, hasExplicitOrigin, commentCount, @@ -69,6 +76,8 @@ const MemoSidebarRegistration = ({ onEdit, onShareImageOpen, parentMemo, + parentStatus, + onParentRetry, readonly, setMemoDetail, ]); @@ -100,6 +109,8 @@ const MemoDetail = () => { data: memoFromDirect, error: directError, isLoading: directLoading, + isUnavailable: directUnavailable, + fetchStatus: directFetchStatus, } = useMemo(memoNameFromParams, { enabled: !isShareMode && !!memoNameFromParams }); const { data: memoFromShare, error: shareError, isLoading: shareLoading } = useSharedMemo(shareToken ?? "", { enabled: isShareMode }); @@ -121,10 +132,34 @@ const MemoDetail = () => { error: error as Error | null, }); - const { data: parentMemo } = useMemo(memo?.parent || "", { + const { + data: fetchedParent, + error: parentError, + isUnavailable: parentUnavailable, + refetch: refetchParent, + } = useMemo(memo?.parent || "", { enabled: !isShareMode && !!memo?.parent, }); + // Private parents return 401 to guests. The transport still owns session + // recovery; only a settled guest view treats that result as unavailable. + const guestParentDenied = + authInitialized && !currentUser && parentError instanceof ConnectError && parentError.code === Code.Unauthenticated; + const parentStatus: MemoParentStatus | undefined = + !isShareMode && memo?.parent + ? parentUnavailable || guestParentDenied + ? "unavailable" + : parentError + ? "error" + : !fetchedParent + ? "loading" + : undefined + : undefined; + const parentMemo = parentStatus ? undefined : fetchedParent; + const handleParentRetry = useCallback(() => { + void refetchParent(); + }, [refetchParent]); + const { data: comments = [], fetchNextPage: fetchNextComments, @@ -169,6 +204,10 @@ const MemoDetail = () => { el.scrollIntoView({ behavior: "smooth", block: "center" }); }, [hash, memo, memoName, comments]); + // Keep the query mounted while revalidating a cached denial. Redirecting + // earlier would abort the request that could confirm restored access. + if (!isShareMode && directUnavailable && directFetchStatus === "idle" && !directError) return ; + if (isShareMode) { const isNotFound = error instanceof ConnectError && (error.code === Code.NotFound || error.code === Code.Unauthenticated); if (isNotFound || (!isLoading && !memo)) { @@ -191,6 +230,8 @@ const MemoDetail = () => { { />
+ {!isShareMode && parentStatus && ( +
+ +
+ )} {!isShareMode && parentMemo && (
& { property?: Memo_Property | undefined; /** - * Output only. The readable context memo of this COMMENT relation, if any. - * This is omitted unless the caller may independently read both memos. + * Output only. The context memo of this COMMENT relation, if any. + * Its identity is returned even when the caller cannot read the parent. + * Fetch the parent independently; this field does not grant read access. * Format: memos/{memo} * * @generated from field: optional string parent = 16; diff --git a/web/tests/memo-access-transitions.test.tsx b/web/tests/memo-access-transitions.test.tsx new file mode 100644 index 00000000..2cb5fea7 --- /dev/null +++ b/web/tests/memo-access-transitions.test.tsx @@ -0,0 +1,105 @@ +import { create } from "@bufbuild/protobuf"; +import { Code, ConnectError } from "@connectrpc/connect"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act, renderHook, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { useResolvedRelationMemos } from "@/components/MemoMetadata/Relation/useResolvedRelationMemos"; +import { findMemoInCollectionQueries, memoKeys, useMemo, useUpdateMemo } from "@/hooks/useMemoQueries"; +import { ListMemosResponseSchema, MemoRelation_Type, MemoRelationSchema, MemoSchema } from "@/types/proto/api/v1/memo_service_pb"; + +const api = vi.hoisted(() => ({ getMemo: vi.fn(), updateMemo: vi.fn() })); +vi.mock("@/connect", () => ({ memoServiceClient: api })); +const setup = () => { + const client = new QueryClient({ defaultOptions: { queries: { retry: false, retryDelay: 0 }, mutations: { retry: false } } }); + const wrapper = ({ children }: { children: ReactNode }) => {children}; + return { client, wrapper }; +}; + +describe("memo access transitions", () => { + beforeEach(() => vi.resetAllMocks()); + + it.each([ + Code.PermissionDenied, + Code.NotFound, + ])("discards denied parent content and snippets while preserving the comment (%s)", async (code) => { + const { client, wrapper } = setup(); + const parent = create(MemoSchema, { name: "memos/parent", content: "secret", snippet: "secret" }); + const comment = create(MemoSchema, { + name: "memos/comment", + parent: parent.name, + content: "my comment", + relations: [ + create(MemoRelationSchema, { + type: MemoRelation_Type.COMMENT, + memo: { name: "memos/comment" }, + relatedMemo: { name: parent.name, snippet: "secret" }, + }), + ], + }); + client.setQueryData(memoKeys.detail(parent.name), parent, { updatedAt: 1 }); + client.setQueryData(memoKeys.detail(comment.name), comment); + client.setQueryData(memoKeys.list({}), create(ListMemosResponseSchema, { memos: [parent, comment] })); + api.getMemo.mockRejectedValue(new ConnectError("denied", code)); + const { result } = renderHook(() => useMemo(parent.name), { wrapper }); + await waitFor(() => expect(result.current.isUnavailable).toBe(true)); + expect(result.current.data).toBeUndefined(); + expect(client.getQueryData(memoKeys.detail(parent.name))).toBeNull(); + expect(findMemoInCollectionQueries(client, parent.name)).toBeUndefined(); + expect(client.getQueryData(memoKeys.detail(comment.name))).toMatchObject({ content: "my comment", parent: parent.name, relations: [] }); + expect(api.getMemo).toHaveBeenCalledOnce(); + api.getMemo.mockResolvedValue(parent); + await act(async () => { + await result.current.refetch(); + }); + await waitFor(() => expect(result.current.isUnavailable).toBe(false)); + expect(result.current.data?.content).toBe("secret"); + client.clear(); + }); + + it("resolves readable relations even when another relation is denied", async () => { + const { client, wrapper } = setup(); + api.getMemo.mockImplementation(({ name }) => + name === "memos/denied" + ? Promise.reject(new ConnectError("denied", Code.PermissionDenied)) + : Promise.resolve(create(MemoSchema, { name, snippet: "Readable" })), + ); + const { result } = renderHook(() => useResolvedRelationMemos(["memos/denied", "memos/readable"]), { wrapper }); + await waitFor(() => expect(result.current["memos/readable"]?.snippet).toBe("Readable")); + expect(result.current["memos/denied"]).toBeNull(); + client.clear(); + }); + + it("retains transient errors as retryable errors instead of marking a memo unavailable", async () => { + const { client, wrapper } = setup(); + api.getMemo.mockRejectedValue(new ConnectError("offline", Code.Unavailable)); + const { result } = renderHook(() => useMemo("memos/parent"), { wrapper }); + await waitFor(() => expect(result.current.isError).toBe(true)); + expect(result.current.isUnavailable).toBe(false); + expect(api.getMemo).toHaveBeenCalledTimes(2); + client.clear(); + }); + + it("does not optimistically move a memo before the server accepts it", async () => { + const { client, wrapper } = setup(); + const memo = create(MemoSchema, { name: "memos/moving", space: "spaces/a" }); + client.setQueryData(memoKeys.detail(memo.name), memo); + let reject!: (error: Error) => void; + api.updateMemo.mockImplementation( + () => + new Promise((_, fail) => { + reject = fail; + }), + ); + const { result } = renderHook(() => useUpdateMemo(), { wrapper }); + act(() => result.current.mutate({ update: { name: memo.name, space: "spaces/b" }, updateMask: ["space"] })); + await waitFor(() => expect(api.updateMemo).toHaveBeenCalledOnce()); + expect(client.getQueryData(memoKeys.detail(memo.name))).toEqual(memo); + await act(async () => { + reject(new ConnectError("denied", Code.PermissionDenied)); + }); + await waitFor(() => expect(result.current.isError).toBe(true)); + expect(client.getQueryData(memoKeys.detail(memo.name))).toEqual(memo); + client.clear(); + }); +}); diff --git a/web/tests/memo-action-menu.test.tsx b/web/tests/memo-action-menu.test.tsx index ea92eab3..bd4fbd75 100644 --- a/web/tests/memo-action-menu.test.tsx +++ b/web/tests/memo-action-menu.test.tsx @@ -1,11 +1,12 @@ import { create } from "@bufbuild/protobuf"; import { fireEvent, render, screen, waitFor } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import MemoActionMenu from "@/components/MemoActionMenu"; import { State } from "@/types/proto/api/v1/common_pb"; import { MemoSchema } from "@/types/proto/api/v1/memo_service_pb"; const handlers = vi.hoisted(() => ({ + canMove: true, handleTogglePinMemoBtnClick: vi.fn(), handleEditMemoClick: vi.fn(), handleToggleMemoStatusClick: vi.fn(), @@ -21,6 +22,10 @@ vi.mock("@/components/ConfirmDialog", () => ({ default: () => null, })); +vi.mock("@/components/MemoActionMenu/MemoMoveDialog", () => ({ + default: () =>
, +})); + vi.mock("@/components/MemoActionMenu/hooks", () => ({ useMemoActionHandlers: () => handlers, })); @@ -30,6 +35,35 @@ vi.mock("@/utils/i18n", () => ({ })); describe("MemoActionMenu", () => { + beforeEach(() => { + vi.clearAllMocks(); + handlers.canMove = true; + }); + + it.each(["move", "delete"])("places %s inside More while keeping frequent actions in the main menu", async (action) => { + render(); + fireEvent.click(screen.getByRole("button", { name: "common.more" })); + expect(await screen.findByRole("menuitem", { name: "common.edit" })).toBeInTheDocument(); + expect(screen.getByRole("menuitem", { name: "common.archive" })).toBeInTheDocument(); + expect(screen.queryByRole("menuitem", { name: "memo.move.title" })).not.toBeInTheDocument(); + expect(screen.queryByRole("menuitem", { name: "common.delete" })).not.toBeInTheDocument(); + + fireEvent.click(screen.getByRole("menuitem", { name: "common.more" })); + expect(await screen.findByRole("menuitem", { name: "memo.move.title" })).toBeInTheDocument(); + expect(screen.getByRole("menuitem", { name: "common.delete" })).toBeInTheDocument(); + fireEvent.click(screen.getByRole("menuitem", { name: action === "move" ? "memo.move.title" : "common.delete" })); + if (action === "move") expect(await screen.findByRole("dialog", { name: "Move to Space" })).toBeInTheDocument(); + else expect(handlers.handleDeleteMemoClick).toHaveBeenCalledOnce(); + }); + + it("omits More when neither action is available", async () => { + handlers.canMove = false; + render(); + fireEvent.click(screen.getByRole("button", { name: "common.more" })); + expect(await screen.findByRole("menuitem", { name: "common.copy" })).toBeInTheDocument(); + expect(screen.queryByRole("menuitem", { name: "common.more" })).not.toBeInTheDocument(); + }); + it("is a quiet compact control that takes the accent fill while open", async () => { const memo = create(MemoSchema, { name: "memos/1", state: State.NORMAL, pinned: false }); render(); diff --git a/web/tests/memo-action-navigation.test.tsx b/web/tests/memo-action-navigation.test.tsx index 6de53723..425d818f 100644 --- a/web/tests/memo-action-navigation.test.tsx +++ b/web/tests/memo-action-navigation.test.tsx @@ -25,6 +25,8 @@ vi.mock("@/hooks/useUserQueries", () => ({ userKeys: { stats: () => ["users", "stats"] }, })); +vi.mock("@/hooks/useCurrentUser", () => ({ default: () => ({ name: "users/alice" }) })); + vi.mock("@/contexts/InstanceContext", () => ({ useInstance: () => ({ profile: { instanceUrl: "" } }), })); diff --git a/web/tests/memo-detail-access.test.tsx b/web/tests/memo-detail-access.test.tsx new file mode 100644 index 00000000..37e552a4 --- /dev/null +++ b/web/tests/memo-detail-access.test.tsx @@ -0,0 +1,145 @@ +import { create } from "@bufbuild/protobuf"; +import { Code, ConnectError } from "@connectrpc/connect"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { MemoryRouter, Route, Routes } from "react-router-dom"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { memoKeys } from "@/hooks/useMemoQueries"; +import MemoDetail from "@/pages/MemoDetail"; +import { type Memo, MemoSchema } from "@/types/proto/api/v1/memo_service_pb"; + +const state = vi.hoisted(() => ({ + currentUser: undefined as { name: string } | undefined, + getMemo: vi.fn(), + listMemoComments: vi.fn(), + setMemoDetail: vi.fn(), + toastError: vi.fn(), +})); +vi.mock("@/connect", () => ({ memoServiceClient: state })); +vi.mock("@/contexts/AuthContext", () => ({ useAuth: () => ({ currentUser: state.currentUser, isInitialized: true }) })); +vi.mock("@/contexts/InstanceContext", () => ({ useInstance: () => ({ isInitialized: true }) })); +vi.mock("@/contexts/AppSidebarContext", () => ({ useAppSidebar: () => ({ setMemoDetail: state.setMemoDetail }) })); +vi.mock("@/components/MemoView", () => ({ default: ({ memo }: { memo: Memo }) =>
{memo.content}
})); +vi.mock("@/components/MemoView/MemoViewContext", () => ({ computeCommentAmount: () => 0 })); +vi.mock("@/components/MemoCommentSection", () => ({ default: () => null })); +vi.mock("@/components/MemoContent/MentionResolutionContext", () => ({ + MentionResolutionProvider: ({ children }: { children: ReactNode }) => children, +})); +vi.mock("@/utils/i18n", () => ({ useTranslate: () => (key: string) => key })); +vi.mock("react-hot-toast", () => ({ toast: { error: state.toastError } })); + +function renderDetail(client: QueryClient, name = "memos/restored") { + return render( + + + + } /> + not found
} /> + + + , + ); +} +const createClient = () => new QueryClient({ defaultOptions: { queries: { retry: false, retryDelay: 0 } } }); + +describe("memo detail access recovery", () => { + beforeEach(() => { + vi.clearAllMocks(); + state.currentUser = undefined; + state.listMemoComments.mockResolvedValue({ memos: [], nextPageToken: "" }); + }); + + it.each(["fresh", "stale"])("revalidates a %s cached denial before showing a restored memo", async (freshness) => { + const client = createClient(); + const name = "memos/restored"; + client.setQueryData(memoKeys.detail(name), null, { updatedAt: freshness === "fresh" ? Date.now() : 1 }); + let resolve!: (memo: Memo) => void; + let signal!: AbortSignal; + state.getMemo.mockImplementation((_, options) => { + signal = options.signal; + return new Promise((done) => { + resolve = done; + }); + }); + renderDetail(client); + await waitFor(() => expect(state.getMemo).toHaveBeenCalledOnce()); + expect(screen.queryByText("not found")).not.toBeInTheDocument(); + expect(signal.aborted).toBe(false); + await act(async () => resolve(create(MemoSchema, { name, content: "Restored memo" }))); + expect(await screen.findByText("Restored memo")).toBeInTheDocument(); + expect(screen.queryByText("not found")).not.toBeInTheDocument(); + client.clear(); + }); + + it.each([Code.PermissionDenied, Code.NotFound])("redirects only after revalidation confirms denial (%s)", async (code) => { + const client = createClient(); + client.setQueryData(memoKeys.detail("memos/restored"), null, { updatedAt: 1 }); + let reject!: (error: Error) => void; + state.getMemo.mockImplementation( + () => + new Promise((_, fail) => { + reject = fail; + }), + ); + renderDetail(client); + await waitFor(() => expect(state.getMemo).toHaveBeenCalledOnce()); + expect(screen.queryByText("not found")).not.toBeInTheDocument(); + await act(async () => reject(new ConnectError("denied", code))); + expect(await screen.findByText("not found")).toBeInTheDocument(); + client.clear(); + }); + + it("does not treat a failed revalidation as a renewed denial", async () => { + const client = createClient(); + client.setQueryData(memoKeys.detail("memos/restored"), null, { updatedAt: 1 }); + state.getMemo.mockRejectedValue(new ConnectError("offline", Code.Unavailable)); + renderDetail(client); + await waitFor(() => expect(state.toastError).toHaveBeenCalled()); + expect(screen.queryByText("not found")).not.toBeInTheDocument(); + client.clear(); + }); + + it.each([false, true])("classifies parent 401 according to the current viewer (signed in: %s)", async (signedIn) => { + if (signedIn) state.currentUser = { name: "users/alice" }; + const client = createClient(); + const comment = create(MemoSchema, { name: "memos/comment", content: "Public comment", parent: "memos/private-parent" }); + state.getMemo.mockImplementation(({ name }) => + name === comment.name ? Promise.resolve(comment) : Promise.reject(new ConnectError("authentication required", Code.Unauthenticated)), + ); + renderDetail(client, comment.name); + expect(await screen.findByText(signedIn ? "memo.parent-load-error" : "memo.parent-unavailable")).toBeInTheDocument(); + expect(screen.getByText("Public comment")).toBeInTheDocument(); + expect(screen.queryByText("not found")).not.toBeInTheDocument(); + expect(screen.queryByRole("link")).not.toBeInTheDocument(); + if (signedIn) expect(screen.getByRole("button", { name: "search.retry" })).toBeInTheDocument(); + else expect(screen.queryByRole("button", { name: "search.retry" })).not.toBeInTheDocument(); + expect(state.setMemoDetail).toHaveBeenLastCalledWith( + expect.objectContaining({ + parentStatus: signedIn ? "error" : "unavailable", + parentMemo: undefined, + }), + ); + client.clear(); + }); + + it("lets a guest retry a transient parent error", async () => { + const client = createClient(); + const comment = create(MemoSchema, { name: "memos/comment", content: "Public comment", parent: "memos/parent" }); + let parentAvailable = false; + state.getMemo.mockImplementation(({ name }) => + name === comment.name + ? Promise.resolve(comment) + : parentAvailable + ? Promise.resolve(create(MemoSchema, { name, content: "Parent memo" })) + : Promise.reject(new ConnectError("offline", Code.Unavailable)), + ); + renderDetail(client, comment.name); + const retry = await screen.findByRole("button", { name: "search.retry" }); + parentAvailable = true; + fireEvent.click(retry); + expect(await screen.findByRole("link", { name: "Parent memo" })).toBeInTheDocument(); + expect(screen.getByText("Public comment")).toBeInTheDocument(); + client.clear(); + }); +}); diff --git a/web/tests/memo-move-dialog.test.tsx b/web/tests/memo-move-dialog.test.tsx new file mode 100644 index 00000000..3ada27be --- /dev/null +++ b/web/tests/memo-move-dialog.test.tsx @@ -0,0 +1,91 @@ +import { create } from "@bufbuild/protobuf"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import MemoMoveDialog from "@/components/MemoActionMenu/MemoMoveDialog"; +import { MemoSchema, Visibility } from "@/types/proto/api/v1/memo_service_pb"; +import { SpaceSchema } from "@/types/proto/api/v1/space_service_pb"; + +const state = vi.hoisted(() => ({ update: vi.fn(), refetch: vi.fn(), user: "users/alice" })); +vi.mock("@/hooks/useCurrentUser", () => ({ default: () => ({ name: state.user }) })); +vi.mock("@/hooks/useMemoQueries", () => ({ useUpdateMemo: () => ({ mutateAsync: state.update, isPending: false }) })); +vi.mock("@/hooks/useSpaceQueries", () => ({ + useSpaces: () => ({ + data: [create(SpaceSchema, { name: "spaces/a", title: "Design" }), create(SpaceSchema, { name: "spaces/b", title: "Product" })], + isPending: false, + refetch: state.refetch, + }), +})); +vi.mock("@/utils/i18n", () => ({ useTranslate: () => (key: string) => key })); +vi.mock("react-hot-toast", () => ({ toast: { success: vi.fn() } })); + +const memo = create(MemoSchema, { name: "memos/original", creator: "users/alice", space: "spaces/a", visibility: Visibility.SPACE }); +const selectDestination = async (name: string) => { + fireEvent.click(screen.getByRole("combobox", { name: "space.current" })); + const option = await screen.findByRole("option", { name }); + fireEvent.pointerDown(option, { pointerType: "mouse" }); + fireEvent.click(option); +}; + +describe("Move to Space", () => { + beforeEach(() => { + vi.clearAllMocks(); + state.user = "users/alice"; + state.update.mockResolvedValue(memo); + }); + + it("requires a new destination and moves with explicit Space audience", async () => { + const close = vi.fn(); + render(); + expect(screen.getByRole("button", { name: "memo.move.confirm" })).toBeDisabled(); + await selectDestination("Product"); + fireEvent.click(screen.getByRole("button", { name: "memo.move.confirm" })); + await waitFor(() => expect(close).toHaveBeenCalledWith(false)); + expect(state.update).toHaveBeenCalledWith({ + update: { name: memo.name, space: "spaces/b", visibility: Visibility.SPACE }, + updateMask: ["space", "visibility"], + }); + }); + + it("defaults to Private when unassigning a Space-visible memo", async () => { + render(); + await selectDestination("memo.move.unassigned"); + fireEvent.click(screen.getByRole("button", { name: "memo.move.confirm" })); + await waitFor(() => + expect(state.update).toHaveBeenCalledWith({ + update: { name: memo.name, space: "", visibility: Visibility.PRIVATE }, + updateMask: ["space", "visibility"], + }), + ); + }); + + it("preserves non-Space audience while assigning a memo", async () => { + render(); + await selectDestination("Product"); + fireEvent.click(screen.getByRole("button", { name: "memo.move.confirm" })); + await waitFor(() => + expect(state.update).toHaveBeenCalledWith({ + update: { name: memo.name, space: "spaces/b", visibility: Visibility.PUBLIC }, + updateMask: ["space"], + }), + ); + }); + + it("keeps the dialog open and refreshes destinations when a move is rejected", async () => { + state.update.mockRejectedValue(new Error("Membership changed")); + const close = vi.fn(); + render(); + await selectDestination("Product"); + fireEvent.click(screen.getByRole("button", { name: "memo.move.confirm" })); + expect(await screen.findByRole("alert")).toHaveTextContent("Membership changed"); + expect(close).not.toHaveBeenCalled(); + expect(state.refetch).toHaveBeenCalledOnce(); + }); + + it("does not allow another user to submit a move", async () => { + state.user = "users/bob"; + render(); + await selectDestination("Product"); + expect(screen.getByRole("button", { name: "memo.move.confirm" })).toBeDisabled(); + expect(state.update).not.toHaveBeenCalled(); + }); +}); diff --git a/web/tests/memo-parent-placeholder.test.tsx b/web/tests/memo-parent-placeholder.test.tsx new file mode 100644 index 00000000..ffb29a41 --- /dev/null +++ b/web/tests/memo-parent-placeholder.test.tsx @@ -0,0 +1,20 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import MemoParentPlaceholder from "@/components/MemoParentPlaceholder"; + +vi.mock("@/utils/i18n", () => ({ useTranslate: () => (key: string) => key })); + +describe("unavailable parent", () => { + it("has no navigation or retry for unavailable content", () => { + render(); + expect(screen.getByText("memo.parent-unavailable")).toBeInTheDocument(); + expect(screen.queryByRole("link")).not.toBeInTheDocument(); + expect(screen.queryByRole("button")).not.toBeInTheDocument(); + }); + it("offers Retry for transient failures", () => { + const retry = vi.fn(); + render(); + fireEvent.click(screen.getByRole("button", { name: "search.retry" })); + expect(retry).toHaveBeenCalledOnce(); + }); +}); diff --git a/web/tests/query-deduplication.test.tsx b/web/tests/query-deduplication.test.tsx index 828a95ea..4e047b37 100644 --- a/web/tests/query-deduplication.test.tsx +++ b/web/tests/query-deduplication.test.tsx @@ -180,6 +180,6 @@ describe("query deduplication", () => { }); expect(clients.getMemo).toHaveBeenCalledTimes(1); - expect(clients.getMemo).toHaveBeenCalledWith({ name: missingMemo.name }); + expect(clients.getMemo).toHaveBeenCalledWith({ name: missingMemo.name }, { signal: expect.any(AbortSignal) }); }); });