contrib/memos
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(memos): choose created or updated time for memos (#5894)

Browse Source
This commit is contained in:
boojack
2026-04-26 11:50:26 +08:00
committed by GitHub
parent 3949a252db
commit c268551a16
49 changed files with 484 additions and 485 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/router/api/v1/instance_service.go
-2
View File
@@ -325,7 +325,6 @@ func convertInstanceMemoRelatedSettingFromStore(setting *storepb.InstanceMemoRel
return nil
}
return &v1pb.InstanceSetting_MemoRelatedSetting{
DisplayWithUpdateTime: setting.DisplayWithUpdateTime,
ContentLengthLimit: setting.ContentLengthLimit,
EnableDoubleClickEdit: setting.EnableDoubleClickEdit,
Reactions: setting.Reactions,
@@ -337,7 +336,6 @@ func convertInstanceMemoRelatedSettingToStore(setting *v1pb.InstanceSetting_Memo
return nil
}
return &storepb.InstanceMemoRelatedSetting{
DisplayWithUpdateTime: setting.DisplayWithUpdateTime,
ContentLengthLimit: setting.ContentLengthLimit,
EnableDoubleClickEdit: setting.EnableDoubleClickEdit,
Reactions: setting.Reactions,
server/router/api/v1/memo_service.go
+17 -54
View File
@@ -54,25 +54,7 @@ func (s *APIV1Service) CreateMemo(ctx context.Context, request *v1pb.CreateMemoR
Visibility: convertVisibilityToStore(request.Memo.Visibility),
}
instanceMemoRelatedSetting, err := s.Store.GetInstanceMemoRelatedSetting(ctx)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to get instance memo related setting")
}
// Handle display_time first: if provided, use it to set the appropriate timestamp
// based on the instance setting (similar to UpdateMemo logic)
// Note: explicit create_time/update_time below will override this if provided
if request.Memo.DisplayTime != nil && request.Memo.DisplayTime.IsValid() {
displayTs := request.Memo.DisplayTime.AsTime().Unix()
if instanceMemoRelatedSetting.DisplayWithUpdateTime {
create.UpdatedTs = displayTs
} else {
create.CreatedTs = displayTs
}
}
// Set custom timestamps if provided in the request
// These take precedence over display_time
// Set custom timestamps if provided in the request.
if request.Memo.CreateTime != nil && request.Memo.CreateTime.IsValid() {
createdTs := request.Memo.CreateTime.AsTime().Unix()
create.CreatedTs = createdTs
@@ -196,7 +178,7 @@ func (s *APIV1Service) ListMemos(ctx context.Context, request *v1pb.ListMemosReq
return nil, status.Errorf(codes.InvalidArgument, "invalid order_by: %v", err)
}
} else {
// Default ordering by display_time desc
// Default ordering by create_time desc.
memoFind.OrderByTimeAsc = false
}
@@ -218,14 +200,6 @@ func (s *APIV1Service) ListMemos(ctx context.Context, request *v1pb.ListMemosReq
}
}
instanceMemoRelatedSetting, err := s.Store.GetInstanceMemoRelatedSetting(ctx)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to get instance memo related setting")
}
if instanceMemoRelatedSetting.DisplayWithUpdateTime {
memoFind.OrderByUpdatedTs = true
}
var limit, offset int
if request.PageToken != "" {
var pageToken v1pb.PageToken
@@ -312,15 +286,13 @@ func (s *APIV1Service) ListMemos(ctx context.Context, request *v1pb.ListMemosReq
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to list memo creators: %v", err)
}
conversionOptions := memoConversionOptions{displayWithUpdateTime: instanceMemoRelatedSetting.DisplayWithUpdateTime}
for _, memo := range memos {
memoName := fmt.Sprintf("%s%s", MemoNamePrefix, memo.UID)
reactions := reactionMap[memoName]
attachments := attachmentMap[memo.ID]
relations := relationMap[memo.ID]
memoMessage, err := s.convertMemoFromStoreWithCreatorsAndOptions(ctx, memo, reactions, attachments, relations, creatorMap, conversionOptions)
memoMessage, err := s.convertMemoFromStoreWithCreators(ctx, memo, reactions, attachments, relations, creatorMap)
if err != nil {
if stderrors.Is(err, errMemoCreatorNotFound) {
slog.Warn("Skipping memo with missing creator",
@@ -479,16 +451,7 @@ func (s *APIV1Service) UpdateMemo(ctx context.Context, request *v1pb.UpdateMemoR
}
update.UpdatedTs = &updatedTs
} else if path == "display_time" {
displayTs := request.Memo.DisplayTime.AsTime().Unix()
memoRelatedSetting, err := s.Store.GetInstanceMemoRelatedSetting(ctx)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to get instance memo related setting")
}
if memoRelatedSetting.DisplayWithUpdateTime {
update.UpdatedTs = &displayTs
} else {
update.CreatedTs = &displayTs
}
return nil, status.Errorf(codes.InvalidArgument, "display_time is not supported")
} else if path == "location" {
payload := memo.Payload
payload.Location = convertLocationToStore(request.Memo.Location)
@@ -817,12 +780,6 @@ func (s *APIV1Service) ListMemoComments(ctx context.Context, request *v1pb.ListM
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to list memo creators: %v", err)
}
instanceMemoRelatedSetting, err := s.Store.GetInstanceMemoRelatedSetting(ctx)
if err != nil {
return nil, status.Errorf(codes.Internal, "failed to get instance memo related setting")
}
conversionOptions := memoConversionOptions{displayWithUpdateTime: instanceMemoRelatedSetting.DisplayWithUpdateTime}
var memosResponse []*v1pb.Memo
for _, m := range memos {
memoName := memoIDToNameMap[m.ID]
@@ -830,7 +787,7 @@ func (s *APIV1Service) ListMemoComments(ctx context.Context, request *v1pb.ListM
attachments := attachmentMap[m.ID]
relations := relationMap[m.ID]
memoMessage, err := s.convertMemoFromStoreWithCreatorsAndOptions(ctx, m, reactions, attachments, relations, creatorMap, conversionOptions)
memoMessage, err := s.convertMemoFromStoreWithCreators(ctx, m, reactions, attachments, relations, creatorMap)
if err != nil {
if stderrors.Is(err, errMemoCreatorNotFound) {
slog.Warn("Skipping memo comment with missing creator",
@@ -939,7 +896,7 @@ func (s *APIV1Service) getMemoContentSnippet(content string) (string, error) {
// parseMemoOrderBy parses the order_by field and sets the appropriate ordering in memoFind.
// Follows AIP-132: supports comma-separated list of fields with optional "desc" suffix.
// Example: "pinned desc, display_time desc" or "create_time asc".
// Example: "pinned desc, create_time desc" or "update_time asc".
func (*APIV1Service) parseMemoOrderBy(orderBy string, memoFind *store.FindMemo) error {
if strings.TrimSpace(orderBy) == "" {
return errors.New("empty order_by")
@@ -950,6 +907,7 @@ func (*APIV1Service) parseMemoOrderBy(orderBy string, memoFind *store.FindMemo)
// Track if we've seen pinned field.
hasPinned := false
hasExplicitTimeField := false
for _, field := range fields {
parts := strings.Fields(strings.TrimSpace(field))
@@ -971,16 +929,21 @@ func (*APIV1Service) parseMemoOrderBy(orderBy string, memoFind *store.FindMemo)
hasPinned = true
memoFind.OrderByPinned = true
// Note: pinned is always DESC (true first) regardless of direction specified.
case "display_time", "create_time", "name":
case "create_time", "name":
// Only set if this is the first time field we encounter.
if !memoFind.OrderByUpdatedTs {
if !hasExplicitTimeField {
memoFind.OrderByTimeAsc = fieldDirection == "asc"
}
hasExplicitTimeField = true
case "update_time":
memoFind.OrderByUpdatedTs = true
memoFind.OrderByTimeAsc = fieldDirection == "asc"
// Only set if this is the first time field we encounter.
if !hasExplicitTimeField {
memoFind.OrderByUpdatedTs = true
memoFind.OrderByTimeAsc = fieldDirection == "asc"
}
hasExplicitTimeField = true
default:
return errors.Errorf("unsupported order field: %s, supported fields are: pinned, display_time, create_time, update_time, name", fieldName)
return errors.Errorf("unsupported order field: %s, supported fields are: pinned, create_time, update_time, name", fieldName)
}
}
server/router/api/v1/memo_service_converter.go
+8 -34
View File
@@ -20,10 +20,6 @@ var (
errReactionCreatorNotFound = stderrors.New("reaction creator not found")
)
type memoConversionOptions struct {
displayWithUpdateTime bool
}
func (s *APIV1Service) convertMemoFromStore(ctx context.Context, memo *store.Memo, reactions []*store.Reaction, attachments []*store.Attachment, relations []*v1pb.MemoRelation) (*v1pb.Memo, error) {
creatorMap, err := s.listUsersByID(ctx, []int32{memo.CreatorID})
if err != nil {
@@ -33,42 +29,20 @@ func (s *APIV1Service) convertMemoFromStore(ctx context.Context, memo *store.Mem
}
func (s *APIV1Service) convertMemoFromStoreWithCreators(ctx context.Context, memo *store.Memo, reactions []*store.Reaction, attachments []*store.Attachment, relations []*v1pb.MemoRelation, creatorMap map[int32]*store.User) (*v1pb.Memo, error) {
instanceMemoRelatedSetting, err := s.Store.GetInstanceMemoRelatedSetting(ctx)
if err != nil {
return nil, errors.Wrap(err, "failed to get instance memo related setting")
}
return s.convertMemoFromStoreWithCreatorsAndOptions(
ctx,
memo,
reactions,
attachments,
relations,
creatorMap,
memoConversionOptions{displayWithUpdateTime: instanceMemoRelatedSetting.DisplayWithUpdateTime},
)
}
func (s *APIV1Service) convertMemoFromStoreWithCreatorsAndOptions(ctx context.Context, memo *store.Memo, reactions []*store.Reaction, attachments []*store.Attachment, relations []*v1pb.MemoRelation, creatorMap map[int32]*store.User, options memoConversionOptions) (*v1pb.Memo, error) {
displayTs := memo.CreatedTs
if options.displayWithUpdateTime {
displayTs = memo.UpdatedTs
}
name := fmt.Sprintf("%s%s", MemoNamePrefix, memo.UID)
creator := creatorMap[memo.CreatorID]
if creator == nil {
return nil, errMemoCreatorNotFound
}
memoMessage := &v1pb.Memo{
Name: name,
State: convertStateFromStore(memo.RowStatus),
Creator: BuildUserName(creator.Username),
CreateTime: timestamppb.New(time.Unix(memo.CreatedTs, 0)),
UpdateTime: timestamppb.New(time.Unix(memo.UpdatedTs, 0)),
DisplayTime: timestamppb.New(time.Unix(displayTs, 0)),
Content: memo.Content,
Visibility: convertVisibilityFromStore(memo.Visibility),
Pinned: memo.Pinned,
Name: name,
State: convertStateFromStore(memo.RowStatus),
Creator: BuildUserName(creator.Username),
CreateTime: timestamppb.New(time.Unix(memo.CreatedTs, 0)),
UpdateTime: timestamppb.New(time.Unix(memo.UpdatedTs, 0)),
Content: memo.Content,
Visibility: convertVisibilityFromStore(memo.Visibility),
Pinned: memo.Pinned,
}
if memo.Payload != nil {
memoMessage.Tags = memo.Payload.Tags
server/router/api/v1/test/memo_service_test.go
+121 -28
View File
@@ -254,6 +254,120 @@ func TestListMemos(t *testing.T) {
require.Equal(t, "👍", userTwoReaction.ReactionType)
}
func TestListMemosTimeOrderBy(t *testing.T) {
ctx := context.Background()
ts := NewTestService(t)
defer ts.Cleanup()
user, err := ts.CreateHostUser(ctx, "time-order-user")
require.NoError(t, err)
userCtx := ts.CreateUserContext(ctx, user.ID)
memoEarlyCreateLateUpdate, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "early create late update",
Visibility: apiv1.Visibility_PRIVATE,
CreateTime: timestamppb.New(time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)),
UpdateTime: timestamppb.New(time.Date(2020, 1, 3, 0, 0, 0, 0, time.UTC)),
},
})
require.NoError(t, err)
memoMiddleCreateEarlyUpdate, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "middle create early update",
Visibility: apiv1.Visibility_PRIVATE,
CreateTime: timestamppb.New(time.Date(2020, 1, 2, 0, 0, 0, 0, time.UTC)),
UpdateTime: timestamppb.New(time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)),
},
})
require.NoError(t, err)
memoLateCreateMiddleUpdate, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "late create middle update",
Visibility: apiv1.Visibility_PRIVATE,
CreateTime: timestamppb.New(time.Date(2020, 1, 3, 0, 0, 0, 0, time.UTC)),
UpdateTime: timestamppb.New(time.Date(2020, 1, 2, 0, 0, 0, 0, time.UTC)),
},
})
require.NoError(t, err)
tests := []struct {
name string
orderBy string
wantNames []string
}{
{
name: "default create time",
orderBy: "",
wantNames: []string{
memoLateCreateMiddleUpdate.Name,
memoMiddleCreateEarlyUpdate.Name,
memoEarlyCreateLateUpdate.Name,
},
},
{
name: "explicit create time",
orderBy: "create_time desc",
wantNames: []string{
memoLateCreateMiddleUpdate.Name,
memoMiddleCreateEarlyUpdate.Name,
memoEarlyCreateLateUpdate.Name,
},
},
{
name: "explicit update time",
orderBy: "update_time desc",
wantNames: []string{
memoEarlyCreateLateUpdate.Name,
memoLateCreateMiddleUpdate.Name,
memoMiddleCreateEarlyUpdate.Name,
},
},
{
name: "pinned with explicit create time",
orderBy: "pinned desc, create_time desc",
wantNames: []string{
memoLateCreateMiddleUpdate.Name,
memoMiddleCreateEarlyUpdate.Name,
memoEarlyCreateLateUpdate.Name,
},
},
{
name: "explicit create time ascending",
orderBy: "create_time asc",
wantNames: []string{
memoEarlyCreateLateUpdate.Name,
memoMiddleCreateEarlyUpdate.Name,
memoLateCreateMiddleUpdate.Name,
},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
resp, err := ts.Service.ListMemos(userCtx, &apiv1.ListMemosRequest{
PageSize: 10,
OrderBy: test.orderBy,
})
require.NoError(t, err)
require.Len(t, resp.Memos, len(test.wantNames))
gotNames := make([]string, 0, len(resp.Memos))
for _, memo := range resp.Memos {
gotNames = append(gotNames, memo.Name)
}
require.Equal(t, test.wantNames, gotNames)
})
}
_, err = ts.Service.ListMemos(userCtx, &apiv1.ListMemosRequest{
PageSize: 10,
OrderBy: "display_time desc",
})
require.Error(t, err)
}
func TestListMemosSkipsReactionsWithMissingCreators(t *testing.T) {
ctx := context.Background()
@@ -431,7 +545,6 @@ func TestCreateMemoWithCustomTimestamps(t *testing.T) {
// Define custom timestamps (January 1, 2020)
customCreateTime := time.Date(2020, 1, 1, 12, 0, 0, 0, time.UTC)
customUpdateTime := time.Date(2020, 1, 2, 12, 0, 0, 0, time.UTC)
customDisplayTime := time.Date(2020, 1, 3, 12, 0, 0, 0, time.UTC)
// Test 1: Create a memo with custom create_time
memoWithCreateTime, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{
@@ -457,41 +570,21 @@ func TestCreateMemoWithCustomTimestamps(t *testing.T) {
require.NotNil(t, memoWithUpdateTime)
require.Equal(t, customUpdateTime.Unix(), memoWithUpdateTime.UpdateTime.AsTime().Unix(), "update_time should match the custom timestamp")
// Test 3: Create a memo with custom display_time
// Note: display_time is computed from either created_ts or updated_ts based on instance setting
// Since DisplayWithUpdateTime defaults to false, display_time maps to created_ts
memoWithDisplayTime, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "This memo has a custom display time",
Visibility: apiv1.Visibility_PRIVATE,
DisplayTime: timestamppb.New(customDisplayTime),
},
})
require.NoError(t, err)
require.NotNil(t, memoWithDisplayTime)
// Since DisplayWithUpdateTime is false by default, display_time sets created_ts
require.Equal(t, customDisplayTime.Unix(), memoWithDisplayTime.DisplayTime.AsTime().Unix(), "display_time should match the custom timestamp")
require.Equal(t, customDisplayTime.Unix(), memoWithDisplayTime.CreateTime.AsTime().Unix(), "create_time should also match since display_time maps to created_ts")
// Test 4: Create a memo with all custom timestamps
// When both display_time and create_time are provided, create_time takes precedence
// Test 3: Create a memo with all custom timestamps
memoWithAllTimestamps, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "This memo has all custom timestamps",
Visibility: apiv1.Visibility_PRIVATE,
CreateTime: timestamppb.New(customCreateTime),
UpdateTime: timestamppb.New(customUpdateTime),
DisplayTime: timestamppb.New(customDisplayTime),
Content: "This memo has all custom timestamps",
Visibility: apiv1.Visibility_PRIVATE,
CreateTime: timestamppb.New(customCreateTime),
UpdateTime: timestamppb.New(customUpdateTime),
},
})
require.NoError(t, err)
require.NotNil(t, memoWithAllTimestamps)
require.Equal(t, customCreateTime.Unix(), memoWithAllTimestamps.CreateTime.AsTime().Unix(), "create_time should match the custom timestamp")
require.Equal(t, customUpdateTime.Unix(), memoWithAllTimestamps.UpdateTime.AsTime().Unix(), "update_time should match the custom timestamp")
// display_time is computed from created_ts when DisplayWithUpdateTime is false
require.Equal(t, customCreateTime.Unix(), memoWithAllTimestamps.DisplayTime.AsTime().Unix(), "display_time should be derived from create_time")
// Test 5: Create a comment (memo relation) with custom timestamps
// Test 4: Create a comment (memo relation) with custom timestamps
parentMemo, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "This is the parent memo",
@@ -514,7 +607,7 @@ func TestCreateMemoWithCustomTimestamps(t *testing.T) {
require.NotNil(t, comment)
require.Equal(t, customCommentCreateTime.Unix(), comment.CreateTime.AsTime().Unix(), "comment create_time should match the custom timestamp")
// Test 6: Verify that memos without custom timestamps still get auto-generated ones
// Test 5: Verify that memos without custom timestamps still get auto-generated ones
memoWithoutTimestamps, err := ts.Service.CreateMemo(userCtx, &apiv1.CreateMemoRequest{
Memo: &apiv1.Memo{
Content: "This memo has auto-generated timestamps",
server/router/api/v1/user_service_stats.go
+5 -25
View File
@@ -5,7 +5,6 @@ import (
"fmt"
"time"
"github.com/pkg/errors"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/timestamppb"
@@ -55,11 +54,6 @@ func (s *APIV1Service) listUsernamesByID(ctx context.Context, userIDs []int32) (
}
func (s *APIV1Service) ListAllUserStats(ctx context.Context, _ *v1pb.ListAllUserStatsRequest) (*v1pb.ListAllUserStatsResponse, error) {
instanceMemoRelatedSetting, err := s.Store.GetInstanceMemoRelatedSetting(ctx)
if err != nil {
return nil, errors.Wrap(err, "failed to get instance memo related setting")
}
normalStatus := store.Normal
memoFind := &store.FindMemo{
// Exclude comments by default.
@@ -105,7 +99,7 @@ func (s *APIV1Service) ListAllUserStats(ctx context.Context, _ *v1pb.ListAllUser
userMemoStatMap[memo.CreatorID] = &v1pb.UserStats{
Name: "",
TagCount: make(map[string]int32),
MemoDisplayTimestamps: []*timestamppb.Timestamp{},
MemoCreatedTimestamps: []*timestamppb.Timestamp{},
PinnedMemos: []string{},
MemoTypeStats: &v1pb.UserStats_MemoTypeStats{
LinkCount: 0,
@@ -118,12 +112,7 @@ func (s *APIV1Service) ListAllUserStats(ctx context.Context, _ *v1pb.ListAllUser
stats := userMemoStatMap[memo.CreatorID]
// Add display timestamp
displayTs := memo.CreatedTs
if instanceMemoRelatedSetting.DisplayWithUpdateTime {
displayTs = memo.UpdatedTs
}
stats.MemoDisplayTimestamps = append(stats.MemoDisplayTimestamps, timestamppb.New(time.Unix(displayTs, 0)))
stats.MemoCreatedTimestamps = append(stats.MemoCreatedTimestamps, timestamppb.New(time.Unix(memo.CreatedTs, 0)))
// Count memo stats
stats.TotalMemoCount++
@@ -215,12 +204,7 @@ func (s *APIV1Service) GetUserStats(ctx context.Context, request *v1pb.GetUserSt
memoFind.VisibilityList = []store.Visibility{store.Public, store.Protected}
}
instanceMemoRelatedSetting, err := s.Store.GetInstanceMemoRelatedSetting(ctx)
if err != nil {
return nil, errors.Wrap(err, "failed to get instance memo related setting")
}
displayTimestamps := []*timestamppb.Timestamp{}
createdTimestamps := []*timestamppb.Timestamp{}
tagCount := make(map[string]int32)
linkCount := int32(0)
codeCount := int32(0)
@@ -246,11 +230,7 @@ func (s *APIV1Service) GetUserStats(ctx context.Context, request *v1pb.GetUserSt
totalMemoCount += int32(len(memos))
for _, memo := range memos {
displayTs := memo.CreatedTs
if instanceMemoRelatedSetting.DisplayWithUpdateTime {
displayTs = memo.UpdatedTs
}
displayTimestamps = append(displayTimestamps, timestamppb.New(time.Unix(displayTs, 0)))
createdTimestamps = append(createdTimestamps, timestamppb.New(time.Unix(memo.CreatedTs, 0)))
// Count different memo types based on content.
if memo.Payload != nil {
for _, tag := range memo.Payload.Tags {
@@ -281,7 +261,7 @@ func (s *APIV1Service) GetUserStats(ctx context.Context, request *v1pb.GetUserSt
userStats := &v1pb.UserStats{
Name: fmt.Sprintf("%s/stats", BuildUserName(user.Username)),
MemoDisplayTimestamps: displayTimestamps,
MemoCreatedTimestamps: createdTimestamps,
TagCount: tagCount,
PinnedMemos: pinnedMemos,
TotalMemoCount: totalMemoCount,