Skip to content

feat(backend): list submissions by hackathon ID - #367

Merged
sabinem merged 4 commits into
mainfrom
feat/list-subs-by-hackathon
Oct 23, 2026
Merged

sabinem merged 4 commits into
mainfrom
feat/list-subs-by-hackathon

Conversation

@marftn

@marftn marftn commented Oct 20, 2026

Copy link
Copy Markdown
Contributor
  • ListSubmissions now takes either a hackathon_id or a team_id as parameter
  • The response ordering is documented in the proto file
  • Tests are added

Closes #332

@marftn
marftn requested a review from sabinem October 20, 2026 15:46

@sabinem sabinem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@marftn Looks fine overall, I have only 2 small change requests.

}
switch req.GetValue().(type) {
case *msgs.ListSubmissionsRequest_TeamId:
teamID := uuid.MustParse(req.GetTeamId()) // Guarded upstream.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fix: uuid.MustParse so far it is not used in any service and I would not start to use it here:

  • it creates a dependency on the API to provide the check
  • in case the check is removed there the backend would crash (effect out of proportion)
teamID, err := uuid.Parse(req.GetTeamId())
if err != nil {
	return nil, status.Errorf(codes.InvalidArgument, "invalid team_id: %v", err)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair point. We used MustParse in Custodian/DAC Portal so I thought having it here with tests would make sense for readability and avoid "dead code", but it's true that we use Parse everywhere else so I'll put it back.

query = query.Where(entsubmission.HasTeamWith(entteam.IDEQ(teamID)))

case *msgs.ListSubmissionsRequest_HackathonId:
hackathonID := uuid.MustParse(req.GetHackathonId()) // Guarded upstream.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fix: Same as above.

@marftn
marftn requested a review from sabinem October 22, 2026 16:44

@sabinem sabinem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@marftn Just one todo: add changelog entry.

@sabinem sabinem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@marftn redecided this: it does not need a changelog entry as there is no change for the user.

@sabinem
sabinem merged commit 324b400 into main Oct 23, 2026
4 checks passed
@sabinem
sabinem deleted the feat/list-subs-by-hackathon branch October 23, 2026 11:59
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.

backend: list submissions

2 participants