Skip to content

Implement handlers#49

Merged
dpordomingo merged 3 commits into
src-d:masterfrom
dpordomingo:implement-handlers
Jan 30, 2018
Merged

Implement handlers#49
dpordomingo merged 3 commits into
src-d:masterfrom
dpordomingo:implement-handlers

Conversation

@dpordomingo

@dpordomingo dpordomingo commented Jan 30, 2018

Copy link
Copy Markdown
Contributor

partially solves #3
supersedes #16

edd1807 Implement /experiment handlers

@dpordomingo dpordomingo requested a review from bzz January 30, 2018 15:04
@dpordomingo

dpordomingo commented Jan 30, 2018

Copy link
Copy Markdown
Contributor Author

@carlosms you can take this as a dependency of your work with the repositories till @bzz approves it

Comment thread server/handler/assignments.go Outdated
return nil, fmt.Errorf("wrong format in experiment ID sent; received %s", requestedExperimentID)
}

// TODO: fetch from user in session

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why don't use jwt.GetUserID? Middleware is already there.

assignments, err := repository.GetAssignmentsFor(userID, experimentID)
if err == repository.ErrNoAssignmentsInitialized {
if assignments, err = repository.CreateAssignmentsFor(userID, experimentID); err != nil {
return nil, fmt.Errorf("no available assignments")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the original error should be logged (ok to do in separate PR)

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.

I'll do so by #50

Comment thread server/handler/assignments.go Outdated
requestedAssignmentID := chi.URLParam(r, "assignmentId")
assignmentID, err := strconv.Atoi(requestedAssignmentID)
if err != nil {
return nil, fmt.Errorf("wrong format in assignment ID sent; received %s", requestedAssignmentID)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's not 500 but 404 or 400. Ok to do in separate PR.

@smacker

smacker commented Jan 30, 2018

Copy link
Copy Markdown
Contributor

also CI is failing with error

server/handler/experiments.go:8:2: cannot find package "github.com/src-d/code-annotation.bak/server/repository"

@dpordomingo

Copy link
Copy Markdown
Contributor Author

@smacker your comments were addressed 💃

@bzz bzz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

From the PR feedback:
- Expose right HTTP status codes as spotted by @smacker
@dpordomingo dpordomingo merged commit 9b03e09 into src-d:master Jan 30, 2018
@dpordomingo dpordomingo deleted the implement-handlers branch February 9, 2018 18:28
dpordomingo pushed a commit that referenced this pull request Mar 26, 2018
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.

3 participants