From 9f27c9ebec390f947165e165dcdc7fe76a2d46e8 Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Sat, 11 Mar 2023 00:15:00 +0900 Subject: [PATCH 01/14] fix variable names and add comment --- .../domain/service/slack_reaction_users.go | 7 ++++--- app/pkg/slack/hoge_test.go | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 app/pkg/slack/hoge_test.go diff --git a/app/internal/domain/service/slack_reaction_users.go b/app/internal/domain/service/slack_reaction_users.go index cab602d..c0a835f 100644 --- a/app/internal/domain/service/slack_reaction_users.go +++ b/app/internal/domain/service/slack_reaction_users.go @@ -68,18 +68,19 @@ func (s *slackReactionUsersService) ListUsersEmailByReaction(ctx context.Context if err != nil { return nil, err } - inviteUserIDs := s.getReactionUserIDs(ctx, msg.Reactions, reactionName) - inviteUserEmails, err := s.chunkedListUsersEmail(ctx, inviteUserIDs) + reactedUserIDs := s.getReactionUserIDs(ctx, msg.Reactions, reactionName) + reactedUserEmails, err := s.chunkedListUsersEmail(ctx, reactedUserIDs) if err != nil { return nil, err } - return inviteUserEmails, nil + return reactedUserEmails, nil } // getReactionUserIDs get reaction users by reactionName func (s *slackReactionUsersService) getReactionUserIDs(ctx context.Context, reactions []*model.SlackReaction, reactionName string) []string { var userIDs []string var targetReactions []*model.SlackReaction + // filtered reactions by reactionName for _, reaction := range reactions { rn := slack.ExtractReactionName(reaction.Name) if slack.RemoveSkinToneFromReaction(rn) == reactionName { diff --git a/app/pkg/slack/hoge_test.go b/app/pkg/slack/hoge_test.go new file mode 100644 index 0000000..4b3b8cf --- /dev/null +++ b/app/pkg/slack/hoge_test.go @@ -0,0 +1,17 @@ +/* + * Copyright 2023 Money Forward, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package slack From e12b7c5b8e668931690f03f5e3a4f6cf7735b6e9 Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Fri, 17 Mar 2023 10:06:49 +0900 Subject: [PATCH 02/14] add slackClient.GetReaction method' --- app/pkg/slack/client.go | 10 ++++++++++ app/pkg/slack/hoge_test.go | 17 ----------------- 2 files changed, 10 insertions(+), 17 deletions(-) delete mode 100644 app/pkg/slack/hoge_test.go diff --git a/app/pkg/slack/client.go b/app/pkg/slack/client.go index 18eddad..1a484ac 100644 --- a/app/pkg/slack/client.go +++ b/app/pkg/slack/client.go @@ -28,6 +28,7 @@ type Client interface { PostEphemeral(ctx context.Context, channelID, userID, ts, message string) error GetConversationReplies(ctx context.Context, channelID, ts string) ([]slack.Message, error) GetUsersInfo(ctx context.Context, userID ...string) (*[]slack.User, error) + GetReaction(ctx context.Context, channelID, ts string, full bool) ([]slack.ItemReaction, error) GetClient() *slack.Client GetAppUserID() string @@ -118,6 +119,15 @@ func (c *client) GetUsersInfo(ctx context.Context, userID ...string) (*[]slack.U return users, nil } +func (c *client) GetReaction(ctx context.Context, channelID, ts string, full bool) ([]slack.ItemReaction, error) { + return c.Client.GetReactionsContext(ctx, slack.ItemRef{ + Channel: channelID, + Timestamp: ts, + }, slack.GetReactionsParameters{ + Full: full, + }) +} + func (c *client) GetClient() *slack.Client { return c.Client } diff --git a/app/pkg/slack/hoge_test.go b/app/pkg/slack/hoge_test.go deleted file mode 100644 index 4b3b8cf..0000000 --- a/app/pkg/slack/hoge_test.go +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Copyright 2023 Money Forward, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package slack From e6cd1c88a82d06a6147d30adae7dcbea0713eb53 Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Fri, 17 Mar 2023 10:07:36 +0900 Subject: [PATCH 03/14] fix slackReactionUsersService.ListUsersEmailByReaction --- app/internal/domain/repository/slack.go | 11 ++++++++++ .../domain/service/slack_reaction_users.go | 20 +++++++++++++++++++ app/internal/model/slack_message.go | 2 ++ app/internal/repository/slack.go | 18 +++++++++++++++++ 4 files changed, 51 insertions(+) diff --git a/app/internal/domain/repository/slack.go b/app/internal/domain/repository/slack.go index 466f61e..7a830a9 100644 --- a/app/internal/domain/repository/slack.go +++ b/app/internal/domain/repository/slack.go @@ -24,8 +24,19 @@ import ( ) type SlackRepository interface { + // PostMessage send a message to a channel PostMessage(ctx context.Context, channelID, message, ts string) error + + // PostEphemeral sends an ephemeral message to user in a channel PostEphemeral(ctx context.Context, channelID, message, ts, userID string) error + + // GetParentMessage gets Slack message that started the thread GetParentMessage(ctx context.Context, channelID, ts string) (*model.SlackMessage, error) + + // ListUsersEmail fetches users email ListUsersEmail(ctx context.Context, userID []string) ([]*model.SlackUserEmail, error) + + // GetReactions fetches reactions + // full: if true always return the complete reaction list + GetReactions(ctx context.Context, channelID, ts string, full bool) ([]*model.SlackReaction, error) } diff --git a/app/internal/domain/service/slack_reaction_users.go b/app/internal/domain/service/slack_reaction_users.go index c0a835f..1671d8a 100644 --- a/app/internal/domain/service/slack_reaction_users.go +++ b/app/internal/domain/service/slack_reaction_users.go @@ -68,6 +68,12 @@ func (s *slackReactionUsersService) ListUsersEmailByReaction(ctx context.Context if err != nil { return nil, err } + if s.IsNeededMoreFetches(msg.Reactions) { + msg.Reactions, err = s.getFullReactions(ctx, channelID, ts) + if err != nil { + return nil, err + } + } reactedUserIDs := s.getReactionUserIDs(ctx, msg.Reactions, reactionName) reactedUserEmails, err := s.chunkedListUsersEmail(ctx, reactedUserIDs) if err != nil { @@ -76,6 +82,16 @@ func (s *slackReactionUsersService) ListUsersEmailByReaction(ctx context.Context return reactedUserEmails, nil } +// IsNeededMoreFetches returns true if more fetches is required +func (s *slackReactionUsersService) IsNeededMoreFetches(reactions []*model.SlackReaction) bool { + for _, r := range reactions { + if r.Count != len(r.UserIDs) { + return true + } + } + return false +} + // getReactionUserIDs get reaction users by reactionName func (s *slackReactionUsersService) getReactionUserIDs(ctx context.Context, reactions []*model.SlackReaction, reactionName string) []string { var userIDs []string @@ -95,3 +111,7 @@ func (s *slackReactionUsersService) getReactionUserIDs(ctx context.Context, reac } return slice.ToStringSet(userIDs) } + +func (s *slackReactionUsersService) getFullReactions(ctx context.Context, channelID, ts string) ([]*model.SlackReaction, error) { + return s.slackRepository.GetReactions(ctx, channelID, ts, true) +} diff --git a/app/internal/model/slack_message.go b/app/internal/model/slack_message.go index f4a4f2c..6c73dea 100644 --- a/app/internal/model/slack_message.go +++ b/app/internal/model/slack_message.go @@ -18,11 +18,13 @@ package model type SlackMessage struct { ChannelID string + Timestamp string Reactions []*SlackReaction } type SlackReaction struct { Name string + Count int UserIDs []string } diff --git a/app/internal/repository/slack.go b/app/internal/repository/slack.go index dc44f6b..ff2191b 100644 --- a/app/internal/repository/slack.go +++ b/app/internal/repository/slack.go @@ -64,10 +64,12 @@ func (r *slackRepository) GetParentMessage(ctx context.Context, channelID, ts st reactions = append(reactions, &model.SlackReaction{ Name: reaction.Name, UserIDs: reaction.Users, + Count: reaction.Count, }) } return &model.SlackMessage{ ChannelID: parentMessage.Channel, + Timestamp: parentMessage.Timestamp, Reactions: reactions, }, nil } @@ -93,3 +95,19 @@ func (r *slackRepository) ListUsersEmail(ctx context.Context, userID []string) ( } return slackUsers, nil } + +func (r *slackRepository) GetReactions(ctx context.Context, channelID, ts string, full bool) ([]*model.SlackReaction, error) { + reactions, err := r.client.GetReaction(ctx, channelID, ts, full) + if err != nil { + return nil, err + } + ret := make([]*model.SlackReaction, 0, len(reactions)) + for _, reaction := range reactions { + ret = append(ret, &model.SlackReaction{ + Name: reaction.Name, + UserIDs: reaction.Users, + Count: reaction.Count, + }) + } + return ret, nil +} From 416501f7fd1c1baa84bebe333506a39c662cc270 Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Fri, 17 Mar 2023 10:08:15 +0900 Subject: [PATCH 04/14] delete unused field --- app/internal/model/slack_message.go | 1 - 1 file changed, 1 deletion(-) diff --git a/app/internal/model/slack_message.go b/app/internal/model/slack_message.go index 6c73dea..aef9993 100644 --- a/app/internal/model/slack_message.go +++ b/app/internal/model/slack_message.go @@ -18,7 +18,6 @@ package model type SlackMessage struct { ChannelID string - Timestamp string Reactions []*SlackReaction } From 4784ab19f6b89db9bff2aa93ceaadc091133efbf Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Fri, 17 Mar 2023 10:08:57 +0900 Subject: [PATCH 05/14] delete unused field --- app/internal/repository/slack.go | 1 - 1 file changed, 1 deletion(-) diff --git a/app/internal/repository/slack.go b/app/internal/repository/slack.go index ff2191b..a6db4bd 100644 --- a/app/internal/repository/slack.go +++ b/app/internal/repository/slack.go @@ -69,7 +69,6 @@ func (r *slackRepository) GetParentMessage(ctx context.Context, channelID, ts st } return &model.SlackMessage{ ChannelID: parentMessage.Channel, - Timestamp: parentMessage.Timestamp, Reactions: reactions, }, nil } From f0cbdbd9dce7fed9332d062874a367571794cb95 Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Sat, 18 Mar 2023 15:29:35 +0900 Subject: [PATCH 06/14] change method name --- app/internal/domain/service/slack_reaction_users.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/internal/domain/service/slack_reaction_users.go b/app/internal/domain/service/slack_reaction_users.go index 1671d8a..0f837b8 100644 --- a/app/internal/domain/service/slack_reaction_users.go +++ b/app/internal/domain/service/slack_reaction_users.go @@ -68,7 +68,7 @@ func (s *slackReactionUsersService) ListUsersEmailByReaction(ctx context.Context if err != nil { return nil, err } - if s.IsNeededMoreFetches(msg.Reactions) { + if s.isReactionRefetchNeeded(msg.Reactions) { msg.Reactions, err = s.getFullReactions(ctx, channelID, ts) if err != nil { return nil, err @@ -82,8 +82,8 @@ func (s *slackReactionUsersService) ListUsersEmailByReaction(ctx context.Context return reactedUserEmails, nil } -// IsNeededMoreFetches returns true if more fetches is required -func (s *slackReactionUsersService) IsNeededMoreFetches(reactions []*model.SlackReaction) bool { +// isReactionRefetchNeeded returns true if more fetches is required +func (s *slackReactionUsersService) isReactionRefetchNeeded(reactions []*model.SlackReaction) bool { for _, r := range reactions { if r.Count != len(r.UserIDs) { return true From db34d6d6fdbe8aead9b82c4b3e9c565e2493c70d Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Sat, 18 Mar 2023 23:34:14 +0900 Subject: [PATCH 07/14] mock generate --- app/internal/domain/repository/mock/slack.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/app/internal/domain/repository/mock/slack.go b/app/internal/domain/repository/mock/slack.go index 02dc69a..62eeb6c 100644 --- a/app/internal/domain/repository/mock/slack.go +++ b/app/internal/domain/repository/mock/slack.go @@ -50,6 +50,21 @@ func (mr *MockSlackRepositoryMockRecorder) GetParentMessage(ctx, channelID, ts i return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetParentMessage", reflect.TypeOf((*MockSlackRepository)(nil).GetParentMessage), ctx, channelID, ts) } +// GetReactions mocks base method. +func (m *MockSlackRepository) GetReactions(ctx context.Context, channelID, ts string, full bool) ([]*model.SlackReaction, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetReactions", ctx, channelID, ts, full) + ret0, _ := ret[0].([]*model.SlackReaction) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetReactions indicates an expected call of GetReactions. +func (mr *MockSlackRepositoryMockRecorder) GetReactions(ctx, channelID, ts, full interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetReactions", reflect.TypeOf((*MockSlackRepository)(nil).GetReactions), ctx, channelID, ts, full) +} + // ListUsersEmail mocks base method. func (m *MockSlackRepository) ListUsersEmail(ctx context.Context, userID []string) ([]*model.SlackUserEmail, error) { m.ctrl.T.Helper() From 5f2bdb0d719c271b5b56401a9ca2a10a406d9a0b Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Sun, 19 Mar 2023 00:03:24 +0900 Subject: [PATCH 08/14] add testing --- .../service/slack_reaction_users_test.go | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/app/internal/domain/service/slack_reaction_users_test.go b/app/internal/domain/service/slack_reaction_users_test.go index 28a4b66..f50da2f 100644 --- a/app/internal/domain/service/slack_reaction_users_test.go +++ b/app/internal/domain/service/slack_reaction_users_test.go @@ -41,10 +41,12 @@ func Test_slackReactionUsersService_ListUsersEmailByReaction(t *testing.T) { { Name: "join", UserIDs: []string{"user01", "user02"}, + Count: 2, }, { Name: "reactionSample", UserIDs: []string{"user02", "user03"}, + Count: 2, }, }, } @@ -76,6 +78,58 @@ func Test_slackReactionUsersService_ListUsersEmailByReaction(t *testing.T) { {ID: "user03", Email: "user03@example.com"}, }, }, + { + name: "OK: There is difference between len(Reaction.UserIDs) and Reaction.Count", + args: args{ + channelID: "sampleCID", ts: "sampleTs", reactionName: "reactionSample", + }, + prepare: func(msr *mock_repository.MockSlackRepository) { + gomock.InOrder( + msr.EXPECT().GetParentMessage(gomock.Any(), "sampleCID", "sampleTs").Return( + &model.SlackMessage{ + ChannelID: "sampleCID", + Reactions: []*model.SlackReaction{ + { + Name: "join", + UserIDs: []string{"user01", "user02"}, + Count: 2, + }, + { + Name: "reactionSample", + UserIDs: []string{"user02", "user03"}, + Count: 4, // number of counts that need to be re-fetched + }, + }, + }, nil), + msr.EXPECT().GetReactions(gomock.Any(), "sampleCID", "sampleTs", true).Return( + []*model.SlackReaction{ + { + Name: "join", + UserIDs: []string{"user01", "user02"}, + Count: 2, + }, + { + Name: "reactionSample", + UserIDs: []string{"user02", "user03", "user04", "user05"}, // fully fetched + Count: 4, + }, + }, nil), + msr.EXPECT().ListUsersEmail(gomock.Any(), []string{"user02", "user03", "user04", "user05"}).Return( + []*model.SlackUserEmail{ + {ID: "user02", Email: "user02@example.com"}, + {ID: "user03", Email: "user03@example.com"}, + {ID: "user04", Email: "user04@example.com"}, + {ID: "user05", Email: "user05@example.com"}, + }, nil), + ) + }, + want: []*model.SlackUserEmail{ + {ID: "user02", Email: "user02@example.com"}, + {ID: "user03", Email: "user03@example.com"}, + {ID: "user04", Email: "user04@example.com"}, + {ID: "user05", Email: "user05@example.com"}, + }, + }, { name: "NG: error in GetParentMessage", args: args{ @@ -104,6 +158,35 @@ func Test_slackReactionUsersService_ListUsersEmailByReaction(t *testing.T) { }, wantErr: true, }, + { + name: "NG: error in GetReactions", + args: args{ + channelID: "sampleCID", ts: "sampleTs", reactionName: "reactionSample", + }, + prepare: func(msr *mock_repository.MockSlackRepository) { + gomock.InOrder( + msr.EXPECT().GetParentMessage(gomock.Any(), "sampleCID", "sampleTs").Return( + &model.SlackMessage{ + ChannelID: "sampleCID", + Reactions: []*model.SlackReaction{ + { + Name: "join", + UserIDs: []string{"user01", "user02"}, + Count: 2, + }, + { + Name: "reactionSample", + UserIDs: []string{"user02", "user03"}, + Count: 4, // number of counts that need to be re-fetched + }, + }, + }, nil), + msr.EXPECT().GetReactions(gomock.Any(), "sampleCID", "sampleTs", true).Return( + nil, errors.New("sample_error")), + ) + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From e8136cf6fa2403425ebd95a4c6c50716c2b61d43 Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Sun, 19 Mar 2023 14:30:52 +0900 Subject: [PATCH 09/14] fix condition --- app/internal/domain/service/slack_reaction_users.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/internal/domain/service/slack_reaction_users.go b/app/internal/domain/service/slack_reaction_users.go index 0f837b8..9598438 100644 --- a/app/internal/domain/service/slack_reaction_users.go +++ b/app/internal/domain/service/slack_reaction_users.go @@ -85,7 +85,7 @@ func (s *slackReactionUsersService) ListUsersEmailByReaction(ctx context.Context // isReactionRefetchNeeded returns true if more fetches is required func (s *slackReactionUsersService) isReactionRefetchNeeded(reactions []*model.SlackReaction) bool { for _, r := range reactions { - if r.Count != len(r.UserIDs) { + if r.Count > len(r.UserIDs) { return true } } From 0ba9d0eaa3df50a233ee5bebb384694a5cd4e796 Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Sun, 19 Mar 2023 14:32:55 +0900 Subject: [PATCH 10/14] add comment --- app/internal/domain/service/slack_reaction_users.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/internal/domain/service/slack_reaction_users.go b/app/internal/domain/service/slack_reaction_users.go index 9598438..b95450c 100644 --- a/app/internal/domain/service/slack_reaction_users.go +++ b/app/internal/domain/service/slack_reaction_users.go @@ -83,6 +83,7 @@ func (s *slackReactionUsersService) ListUsersEmailByReaction(ctx context.Context } // isReactionRefetchNeeded returns true if more fetches is required +// reactions[*].Count may be greater than len(reactions[*].UserIDs), at which point a fetch is required. func (s *slackReactionUsersService) isReactionRefetchNeeded(reactions []*model.SlackReaction) bool { for _, r := range reactions { if r.Count > len(r.UserIDs) { From a5856d223d6d53503b443efa724e5b2f9e841305 Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Sun, 19 Mar 2023 14:48:13 +0900 Subject: [PATCH 11/14] fix implementation --- app/internal/domain/repository/slack.go | 4 -- .../domain/service/slack_reaction_users.go | 21 ------ app/internal/repository/slack.go | 65 ++++++++++--------- 3 files changed, 33 insertions(+), 57 deletions(-) diff --git a/app/internal/domain/repository/slack.go b/app/internal/domain/repository/slack.go index 7a830a9..67715fe 100644 --- a/app/internal/domain/repository/slack.go +++ b/app/internal/domain/repository/slack.go @@ -35,8 +35,4 @@ type SlackRepository interface { // ListUsersEmail fetches users email ListUsersEmail(ctx context.Context, userID []string) ([]*model.SlackUserEmail, error) - - // GetReactions fetches reactions - // full: if true always return the complete reaction list - GetReactions(ctx context.Context, channelID, ts string, full bool) ([]*model.SlackReaction, error) } diff --git a/app/internal/domain/service/slack_reaction_users.go b/app/internal/domain/service/slack_reaction_users.go index b95450c..c0a835f 100644 --- a/app/internal/domain/service/slack_reaction_users.go +++ b/app/internal/domain/service/slack_reaction_users.go @@ -68,12 +68,6 @@ func (s *slackReactionUsersService) ListUsersEmailByReaction(ctx context.Context if err != nil { return nil, err } - if s.isReactionRefetchNeeded(msg.Reactions) { - msg.Reactions, err = s.getFullReactions(ctx, channelID, ts) - if err != nil { - return nil, err - } - } reactedUserIDs := s.getReactionUserIDs(ctx, msg.Reactions, reactionName) reactedUserEmails, err := s.chunkedListUsersEmail(ctx, reactedUserIDs) if err != nil { @@ -82,17 +76,6 @@ func (s *slackReactionUsersService) ListUsersEmailByReaction(ctx context.Context return reactedUserEmails, nil } -// isReactionRefetchNeeded returns true if more fetches is required -// reactions[*].Count may be greater than len(reactions[*].UserIDs), at which point a fetch is required. -func (s *slackReactionUsersService) isReactionRefetchNeeded(reactions []*model.SlackReaction) bool { - for _, r := range reactions { - if r.Count > len(r.UserIDs) { - return true - } - } - return false -} - // getReactionUserIDs get reaction users by reactionName func (s *slackReactionUsersService) getReactionUserIDs(ctx context.Context, reactions []*model.SlackReaction, reactionName string) []string { var userIDs []string @@ -112,7 +95,3 @@ func (s *slackReactionUsersService) getReactionUserIDs(ctx context.Context, reac } return slice.ToStringSet(userIDs) } - -func (s *slackReactionUsersService) getFullReactions(ctx context.Context, channelID, ts string) ([]*model.SlackReaction, error) { - return s.slackRepository.GetReactions(ctx, channelID, ts, true) -} diff --git a/app/internal/repository/slack.go b/app/internal/repository/slack.go index a6db4bd..96cc0c7 100644 --- a/app/internal/repository/slack.go +++ b/app/internal/repository/slack.go @@ -18,7 +18,8 @@ package repository import ( "context" - "fmt" + + slack2 "github.com/slack-go/slack" "github.com/moneyforward/auriga/app/pkg/slack" @@ -55,24 +56,40 @@ func (r *slackRepository) GetParentMessage(ctx context.Context, channelID, ts st return nil, err } } + if len(msgs) <= 0 { + return nil, errors.New("number of messages is zero") + } + parentMessage := msgs[0] + var reactions []*model.SlackReaction + if r.isIncompleteReaction(parentMessage.Reactions) { + // get full reactions + parentMessage.Reactions, err = r.client.GetReaction(ctx, channelID, ts, true) + if err != nil { + return nil, err + } + } + for _, reaction := range parentMessage.Reactions { + reactions = append(reactions, &model.SlackReaction{ + Name: reaction.Name, + UserIDs: reaction.Users, + Count: reaction.Count, + }) + } + return &model.SlackMessage{ + ChannelID: parentMessage.Channel, + Reactions: reactions, + }, nil +} - if len(msgs) > 0 { - fmt.Printf("%#v \n", msgs[0]) - parentMessage := msgs[0] - var reactions []*model.SlackReaction - for _, reaction := range parentMessage.Reactions { - reactions = append(reactions, &model.SlackReaction{ - Name: reaction.Name, - UserIDs: reaction.Users, - Count: reaction.Count, - }) +// isIncompleteReaction returns true if more fetches is required +// reactions[*].Count may be greater than len(reactions[*].Users), at which point a fetch is required. +func (r *slackRepository) isIncompleteReaction(reactions []slack2.ItemReaction) bool { + for _, reaction := range reactions { + if reaction.Count > len(reaction.Users) { + return true } - return &model.SlackMessage{ - ChannelID: parentMessage.Channel, - Reactions: reactions, - }, nil } - return nil, errors.New("number of messages is zero") + return false } func (r *slackRepository) ListUsersEmail(ctx context.Context, userID []string) ([]*model.SlackUserEmail, error) { @@ -94,19 +111,3 @@ func (r *slackRepository) ListUsersEmail(ctx context.Context, userID []string) ( } return slackUsers, nil } - -func (r *slackRepository) GetReactions(ctx context.Context, channelID, ts string, full bool) ([]*model.SlackReaction, error) { - reactions, err := r.client.GetReaction(ctx, channelID, ts, full) - if err != nil { - return nil, err - } - ret := make([]*model.SlackReaction, 0, len(reactions)) - for _, reaction := range reactions { - ret = append(ret, &model.SlackReaction{ - Name: reaction.Name, - UserIDs: reaction.Users, - Count: reaction.Count, - }) - } - return ret, nil -} From 049520c6114f2b9a9fb7067cb38742f31b4b7453 Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Sun, 19 Mar 2023 14:49:32 +0900 Subject: [PATCH 12/14] delete testing --- app/internal/domain/repository/mock/slack.go | 15 ---- .../service/slack_reaction_users_test.go | 81 ------------------- 2 files changed, 96 deletions(-) diff --git a/app/internal/domain/repository/mock/slack.go b/app/internal/domain/repository/mock/slack.go index 62eeb6c..02dc69a 100644 --- a/app/internal/domain/repository/mock/slack.go +++ b/app/internal/domain/repository/mock/slack.go @@ -50,21 +50,6 @@ func (mr *MockSlackRepositoryMockRecorder) GetParentMessage(ctx, channelID, ts i return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetParentMessage", reflect.TypeOf((*MockSlackRepository)(nil).GetParentMessage), ctx, channelID, ts) } -// GetReactions mocks base method. -func (m *MockSlackRepository) GetReactions(ctx context.Context, channelID, ts string, full bool) ([]*model.SlackReaction, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetReactions", ctx, channelID, ts, full) - ret0, _ := ret[0].([]*model.SlackReaction) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetReactions indicates an expected call of GetReactions. -func (mr *MockSlackRepositoryMockRecorder) GetReactions(ctx, channelID, ts, full interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetReactions", reflect.TypeOf((*MockSlackRepository)(nil).GetReactions), ctx, channelID, ts, full) -} - // ListUsersEmail mocks base method. func (m *MockSlackRepository) ListUsersEmail(ctx context.Context, userID []string) ([]*model.SlackUserEmail, error) { m.ctrl.T.Helper() diff --git a/app/internal/domain/service/slack_reaction_users_test.go b/app/internal/domain/service/slack_reaction_users_test.go index f50da2f..dbbcbf5 100644 --- a/app/internal/domain/service/slack_reaction_users_test.go +++ b/app/internal/domain/service/slack_reaction_users_test.go @@ -78,58 +78,6 @@ func Test_slackReactionUsersService_ListUsersEmailByReaction(t *testing.T) { {ID: "user03", Email: "user03@example.com"}, }, }, - { - name: "OK: There is difference between len(Reaction.UserIDs) and Reaction.Count", - args: args{ - channelID: "sampleCID", ts: "sampleTs", reactionName: "reactionSample", - }, - prepare: func(msr *mock_repository.MockSlackRepository) { - gomock.InOrder( - msr.EXPECT().GetParentMessage(gomock.Any(), "sampleCID", "sampleTs").Return( - &model.SlackMessage{ - ChannelID: "sampleCID", - Reactions: []*model.SlackReaction{ - { - Name: "join", - UserIDs: []string{"user01", "user02"}, - Count: 2, - }, - { - Name: "reactionSample", - UserIDs: []string{"user02", "user03"}, - Count: 4, // number of counts that need to be re-fetched - }, - }, - }, nil), - msr.EXPECT().GetReactions(gomock.Any(), "sampleCID", "sampleTs", true).Return( - []*model.SlackReaction{ - { - Name: "join", - UserIDs: []string{"user01", "user02"}, - Count: 2, - }, - { - Name: "reactionSample", - UserIDs: []string{"user02", "user03", "user04", "user05"}, // fully fetched - Count: 4, - }, - }, nil), - msr.EXPECT().ListUsersEmail(gomock.Any(), []string{"user02", "user03", "user04", "user05"}).Return( - []*model.SlackUserEmail{ - {ID: "user02", Email: "user02@example.com"}, - {ID: "user03", Email: "user03@example.com"}, - {ID: "user04", Email: "user04@example.com"}, - {ID: "user05", Email: "user05@example.com"}, - }, nil), - ) - }, - want: []*model.SlackUserEmail{ - {ID: "user02", Email: "user02@example.com"}, - {ID: "user03", Email: "user03@example.com"}, - {ID: "user04", Email: "user04@example.com"}, - {ID: "user05", Email: "user05@example.com"}, - }, - }, { name: "NG: error in GetParentMessage", args: args{ @@ -158,35 +106,6 @@ func Test_slackReactionUsersService_ListUsersEmailByReaction(t *testing.T) { }, wantErr: true, }, - { - name: "NG: error in GetReactions", - args: args{ - channelID: "sampleCID", ts: "sampleTs", reactionName: "reactionSample", - }, - prepare: func(msr *mock_repository.MockSlackRepository) { - gomock.InOrder( - msr.EXPECT().GetParentMessage(gomock.Any(), "sampleCID", "sampleTs").Return( - &model.SlackMessage{ - ChannelID: "sampleCID", - Reactions: []*model.SlackReaction{ - { - Name: "join", - UserIDs: []string{"user01", "user02"}, - Count: 2, - }, - { - Name: "reactionSample", - UserIDs: []string{"user02", "user03"}, - Count: 4, // number of counts that need to be re-fetched - }, - }, - }, nil), - msr.EXPECT().GetReactions(gomock.Any(), "sampleCID", "sampleTs", true).Return( - nil, errors.New("sample_error")), - ) - }, - wantErr: true, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { From 388794374eecda97abf1b0e5545c4a56c1cbb164 Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Tue, 21 Mar 2023 18:14:59 +0900 Subject: [PATCH 13/14] move reactions(var) --- app/internal/repository/slack.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/internal/repository/slack.go b/app/internal/repository/slack.go index 96cc0c7..bd10a11 100644 --- a/app/internal/repository/slack.go +++ b/app/internal/repository/slack.go @@ -60,7 +60,6 @@ func (r *slackRepository) GetParentMessage(ctx context.Context, channelID, ts st return nil, errors.New("number of messages is zero") } parentMessage := msgs[0] - var reactions []*model.SlackReaction if r.isIncompleteReaction(parentMessage.Reactions) { // get full reactions parentMessage.Reactions, err = r.client.GetReaction(ctx, channelID, ts, true) @@ -68,6 +67,7 @@ func (r *slackRepository) GetParentMessage(ctx context.Context, channelID, ts st return nil, err } } + var reactions []*model.SlackReaction for _, reaction := range parentMessage.Reactions { reactions = append(reactions, &model.SlackReaction{ Name: reaction.Name, From a63ad86b2b032375df3bef4028a464245e3c536b Mon Sep 17 00:00:00 2001 From: Sho Hirose Date: Tue, 21 Mar 2023 18:16:26 +0900 Subject: [PATCH 14/14] fix pkg name --- app/internal/repository/slack.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/internal/repository/slack.go b/app/internal/repository/slack.go index bd10a11..7d0545a 100644 --- a/app/internal/repository/slack.go +++ b/app/internal/repository/slack.go @@ -19,9 +19,9 @@ package repository import ( "context" - slack2 "github.com/slack-go/slack" + "github.com/slack-go/slack" - "github.com/moneyforward/auriga/app/pkg/slack" + pkgslack "github.com/moneyforward/auriga/app/pkg/slack" "github.com/moneyforward/auriga/app/internal/model" @@ -29,10 +29,10 @@ import ( ) type slackRepository struct { - client slack.Client + client pkgslack.Client } -func newSlackRepository(client slack.Client) *slackRepository { +func newSlackRepository(client pkgslack.Client) *slackRepository { return &slackRepository{ client: client, } @@ -50,7 +50,7 @@ func (r *slackRepository) PostEphemeral(ctx context.Context, channelID, message, func (r *slackRepository) GetParentMessage(ctx context.Context, channelID, ts string) (*model.SlackMessage, error) { msgs, err := r.client.GetConversationReplies(ctx, channelID, ts) if err != nil { - if errors.Is(err, slack.ErrThreadNotFound) { + if errors.Is(err, pkgslack.ErrThreadNotFound) { return nil, errThreadNotfound } else { return nil, err @@ -83,7 +83,7 @@ func (r *slackRepository) GetParentMessage(ctx context.Context, channelID, ts st // isIncompleteReaction returns true if more fetches is required // reactions[*].Count may be greater than len(reactions[*].Users), at which point a fetch is required. -func (r *slackRepository) isIncompleteReaction(reactions []slack2.ItemReaction) bool { +func (r *slackRepository) isIncompleteReaction(reactions []slack.ItemReaction) bool { for _, reaction := range reactions { if reaction.Count > len(reaction.Users) { return true @@ -95,7 +95,7 @@ func (r *slackRepository) isIncompleteReaction(reactions []slack2.ItemReaction) func (r *slackRepository) ListUsersEmail(ctx context.Context, userID []string) ([]*model.SlackUserEmail, error) { users, err := r.client.GetUsersInfo(ctx, userID...) if err != nil { - if errors.Is(err, slack.ErrUserNotFound) { + if errors.Is(err, pkgslack.ErrUserNotFound) { return nil, errUserNotFound } else { return nil, err