Removed comment logic from memo_service #4678
Closed
+1,605
−963
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 17d9b53. Revert
There was a problem hiding this comment.
Pull Request Overview
This PR refactors memo comment functionality by removing it from memo_service and moving it to a dedicated MemoCommentService, thereby reducing the responsibilities of memo_service and complying with the Single Responsibility Principle.
- Removed CreateMemoComment and ListMemoComments operations from memo_service.
- Introduced MemoCommentService in both TypeScript proto definitions and server API.
- Updated client imports and MemoEditor usage to call memoCommentServiceClient instead of memoServiceClient.
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/types/proto/api/v1/memo_service.ts | Removed memo comment request/response messages and corresponding service method definitions. |
| server/router/api/v1/memo_service.go | Removed Create/List memo comment handler functions. |
| server/router/api/v1/memo_comment_service.go | New implementation for memo comment creation and listing. |
| web/src/components/MemoEditor/index.tsx | Updated import and changed service call from memoServiceClient to memoCommentServiceClient; modified fallback logic for memo visibility. |
| proto/api/v1/memo_service.proto & proto/api/v1/memo_comment_service.proto | Adjusted proto definitions and HTTP annotations for the new memo comment operations. |
| GRPC related generated files | Removed memo comment endpoints from MemoService GRPC stubs and added them in MemoCommentService stubs. |
Comments suppressed due to low confidence (1)
web/src/components/MemoEditor/index.tsx:104
- In the MemoEditor, the fallback for memo visibility has been changed from 'PRIVATE' to 'PUBLIC' when disallowPublicVisibility is true. Please verify if this change is intentional or if it may inadvertently expose comments publicly.
visibility = "PUBLIC";
|
@boojack Can I get some guidance on the failing check? I ran the commands to format. |
|
@nathankrieger Please follow the import order sample in https://github.com/usememos/memos/blob/main/server/router/api/v1/memo_service.go#L22 The order should be as follows: // built-in packages
"context"
"fmt"
// third-party packages
"github.com/lithammer/shortuuid/v4"
"github.com/pkg/errors"
// local packages
"github.com/usememos/memos/server/runner/memopayload"
"github.com/usememos/memos/store" |
johnnyjoygh
requested changes
May 9, 2025
|
@nathankrieger There still has linter error, any updates? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #4677
Moves memo comment logic to its own service, shortening memo_service and aligning better with SRP