feat: add registration form - #212
Conversation
4b64012 to
dd25151
Compare
dd25151 to
01a65d4
Compare
sabinem
left a comment
sabinem
left a comment
There was a problem hiding this comment.
@Panaetius Looks overall good. I left a few comments.
On further thought: we do need a selection type, as we are grouping by certain values such as affiliation, etc in the team assignments, so we need to be sure these values are normed.
|
|
||
| option go_package = "github.com/swissdatasciencecenter/hackagon/components/backend/internal/proto/hackathon/entities"; | ||
|
|
||
| message Answer { |
There was a problem hiding this comment.
fix: Answer needs a user_id since the user_id is optional in ListParticipantsAnswers, otherwise it will unclear who answered the question. You can add the the user_id either here or in the ListParticpiantAnswersResponse.
| // Participant answers (admin) | ||
| rpc ListParticipantAnswers(hackathon.messages.hackathon_svc.ListParticipantAnswersRequest) returns (hackathon.messages.hackathon_svc.ListParticipantAnswersResponse); |
There was a problem hiding this comment.
fix: The comment suggest that this service is admin only, but a participant should also have access to read his answers. Also waitlisted participants should be able to read what they answered.
There was a problem hiding this comment.
yes, that's what's intended, the comment is wrong.
| field.Time("updated_at"). | ||
| Default(time.Now).UpdateDefault(time.Now). | ||
| Comment("Timestamp of the last update."), | ||
| } |
There was a problem hiding this comment.
nitpicky: name this modified_at to align with the rest of the tables in the schema
| message Answer { | ||
| string question_id = 1; | ||
| string value = 2; | ||
| QuestionType type = 3 [(buf.validate.field).enum.defined_only = true]; |
There was a problem hiding this comment.
suggestion: remove the answer type here as it can be derived from the question?
| field.Enum("type"). | ||
| Values("text", "bool"). | ||
| Comment("The type of the question (for readability when reading answers)."), |
There was a problem hiding this comment.
suggestion: remove the answer type as this should be part of the question.
| (buf.validate.field).string.min_len = 1, | ||
| (buf.validate.field).string.max_len = 255 | ||
| ]; | ||
| optional hackathon.entities.QuestionType type = 4; |
There was a problem hiding this comment.
suggestion: refuse a type change once an answer exists for a question.
| edge.To("answers", Answer.Type). | ||
| Comment("Answers submitted by participants for this question."), |
There was a problem hiding this comment.
todo: add a OnDelete so it it is clear what happens to answers when a question is removed.
| func (Question) Indexes() []ent.Index { | ||
| return []ent.Index{ | ||
| index.Fields("key", "hackathon_id").Unique(), | ||
| index.Fields("order"), |
There was a problem hiding this comment.
fix: order is not unique, so for a deterministic ordering add a second field such as created_at.
|
|
||
| message SubmitAnswersRequest { | ||
| string hackathon_id = 1 [(buf.validate.field).string.uuid = true]; | ||
| repeated hackathon.entities.Answer answers = 2; |
There was a problem hiding this comment.
question: does this overwrite all previous answers or only the ones returned?
|
|
||
| option go_package = "github.com/swissdatasciencecenter/hackagon/components/backend/internal/proto/hackathon/messages/hackathon_svc"; | ||
|
|
||
| message JoinRequest { |
There was a problem hiding this comment.
question: what happens to the answers on a second join call? Or can it only be issued once? Did we consider this in the backend?
There was a problem hiding this comment.
second join call overwrites the first.
2 participants
No description provided.