Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TeamDetail.MembersをTeamに移した #484

Merged
merged 31 commits into from
Oct 26, 2023
Merged

Conversation

0214sh7
Copy link
Member

@0214sh7 0214sh7 commented Mar 7, 2023

Copy link
Member

@ras0q ras0q left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

一部レビューしました
テストがコンパイルに失敗してそうです

interfaces/handler/contest.go Outdated Show resolved Hide resolved
interfaces/handler/contest.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 21, 2023

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Files Coverage Δ
domain/user.go 71.73% <ø> (ø)
interfaces/handler/contest.go 99.00% <100.00%> (+1.04%) ⬆️
interfaces/handler/user.go 100.00% <100.00%> (ø)
interfaces/repository/user_impl.go 97.36% <100.00%> (ø)
util/mockdata/handler.go 99.33% <100.00%> (+0.04%) ⬆️
interfaces/repository/contest_impl.go 96.86% <82.60%> (-2.43%) ⬇️

... and 2 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

Copy link
Member

@ras0q ras0q left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

感謝🙌
codecov見るとGetのどのテストもメンバーが入ってなさそうなので適宜メンバー入れてくれると助かります🙏
あとlintの修正もお願いします!

util/mockdata/handler.go Outdated Show resolved Hide resolved
util/mockdata/handler.go Outdated Show resolved Hide resolved
util/mockdata/handler.go Outdated Show resolved Hide resolved
util/mockdata/handler.go Outdated Show resolved Hide resolved
@ras0q
Copy link
Member

ras0q commented Apr 18, 2023

失敗してるテスト見るとわかるんですが、membersがnilになってるとこは空配列にして欲しいです🙏

@ras0q
Copy link
Member

ras0q commented Jul 20, 2023

@0214sh7 ここMembers追加し忘れてます

for _, v := range contestTeamUserBelongings {
if userID == v.UserID {
ct := v.ContestTeam
contestsMap[ct.ContestID].Teams = append(contestsMap[ct.ContestID].Teams, &domain.ContestTeam{
ID: ct.ID,
ContestID: ct.ContestID,
Name: ct.Name,
Result: ct.Result,
})
}
}

domain/contest.go Outdated Show resolved Hide resolved
util/mockdata/handler.go Outdated Show resolved Hide resolved
Copy link
Member

@ras0q ras0q left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

見ました
いくつか書きました

interfaces/handler/oapi_types_gen.go Outdated Show resolved Hide resolved
util/mockdata/handler.go Outdated Show resolved Hide resolved
interfaces/handler/user.go Outdated Show resolved Hide resolved
Copy link
Member

@ras0q ras0q left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

いくつか🙏

interfaces/repository/contest_impl.go Outdated Show resolved Hide resolved
interfaces/repository/contest_impl.go Outdated Show resolved Hide resolved
interfaces/repository/contest_impl.go Outdated Show resolved Hide resolved
interfaces/repository/contest_impl_test.go Outdated Show resolved Hide resolved
interfaces/repository/contest_impl.go Outdated Show resolved Hide resolved
interfaces/repository/contest_impl_test.go Outdated Show resolved Hide resolved
@0214sh7
Copy link
Member Author

0214sh7 commented Aug 20, 2023

マージまじでしんどすぎてマージ・マジ・マジーロ

@0214sh7
Copy link
Member Author

0214sh7 commented Sep 26, 2023

makeUserNameMapくんが何者なのかよくわかってません、何を引数とするmapですかこれ

nameMap, err := r.makeUserNameMap()

func (r *ContestRepository) makeUserNameMap() (map[string]string, error) {
users, err := r.portal.GetAll()
if err != nil {
return nil, err
}
mp := make(map[string]string, len(users))
for _, v := range users {
mp[v.TraQID] = v.RealName
}
return mp, nil
}

@ras0q
Copy link
Member

ras0q commented Sep 29, 2023

makeUserNameMapくんが何者なのかよくわかってません、何を引数とするmapですかこれ

nameMap, err := r.makeUserNameMap()

func (r *ContestRepository) makeUserNameMap() (map[string]string, error) {
users, err := r.portal.GetAll()
if err != nil {
return nil, err
}
mp := make(map[string]string, len(users))
for _, v := range users {
mp[v.TraQID] = v.RealName
}
return mp, nil
}

traQ ID = user.Nameですね

go.mod Outdated Show resolved Hide resolved
interfaces/repository/contest_impl.go Outdated Show resolved Hide resolved
Name: random.AlphaNumeric(),
Result: random.AlphaNumeric(),
},
Members: make([]*domain.User, 0),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここメンバー1つでもいいので入れ他方がよさそう

Name: random.AlphaNumeric(),
Result: random.AlphaNumeric(),
},
Members: make([]*domain.User, 0),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここも

Comment on lines 96 to 100
contestTeams := make([]schema.ContestTeam, 0, len(contest.ContestTeams))
for _, team := range contest.ContestTeams {
contestTeams = append(contestTeams, newContestTeam(team.ID, team.Name, team.Result))
contestTeams = append(contestTeams, newContestTeam(team.ID, team.Name, team.Result, []schema.User{}))
}
res := newContestDetail(newContest(contest.ID, contest.Name, contest.TimeStart, contest.TimeEnd), contest.Link, contest.Description, contestTeams)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

作成時はチームがないのでここら辺の記述はいらないかも

Name: random.AlphaNumeric(),
Result: random.AlphaNumeric(),
},
Members: make([]*domain.User, 0),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここらへんも

Copy link
Member

@ras0q ras0q left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

いいと思います!感謝

@ras0q ras0q merged commit dd01dc2 into main Oct 26, 2023
9 checks passed
@ras0q ras0q deleted the breaking/move_members_into_team branch October 26, 2023 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants