Skip to content

Add CRUD operations for Problems #3

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

joaopedroxavier
Copy link
Contributor

@joaopedroxavier joaopedroxavier commented Jul 13, 2021

Changes

This PR adds CRUD (CREATE, READ - or 'GET', UPDATE, and DELETE) basic functionality for 'Problem' vertices. Its implementation is VERY similar to other vertices that already exist so we can stay consistent.

  • Add model/problem.go, dbOperations/problems.go and handler/problems.go;
  • Add routing for the operations

How to test it

  1. go build . and ./dont-panic-api with a valid Gremlin database configuration.
  2. Open postman, and try the following routes to test each feature:
  • CREATE
    It's a POST request with URL localhost:8080/api/problems. Remember to put a valid body. Example:
{
  "name": "xavi-and-the-cookies",
  "subjects": [
    "dynamic-programming",
    "math"
  ],
  "difficulty": 2
}

The expected result of this request is that the Problem will be created if there isn't already any other Problem with the name you specified.

  • READ
    To fetch all problems, make a GET request with URL localhost:8080/api/problems. To get a specific Problem, make a GET request with URL localhost:8080/api/problems/{name}, being {name} the name of the Problem.
    The expected result is a json with the information from the problems you requested.

  • UPDATE
    It's a PUT request with URL localhost:8080/api/problems/{name}, being {name} the name of the Problem. Put a valid body that represents the updated Problem.
    The expected result is that the Problem information will be updated if there exists a Problem in the database with the name you specified.

  • DELETE
    It's a DELETE request with URL localhost:8080/api/problems/{name}, being {name} the name of the Problem.
    The expected result is that the Problem will be delete if there is a Problem in the database with the name you specified, or an error if there isn't.

@joaopedroxavier joaopedroxavier marked this pull request as ready for review July 13, 2021 20:22

func UpdateProblem(cosmos gremcos.Cosmos, problem modelStorage.Problem, name string) (modelStorage.Problem, error) {
if problem.Name != name {
return modelStorage.Problem{}, fmt.Errorf("can't change the property 'name' of the Problem")
Copy link
Member

Choose a reason for hiding this comment

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

É pra poder mudar o nome, esse é o principal objetivo dele não ser o id real =]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice! Agora vê, eu notei que dá pra fazer uma coisa:

adiciona um Problema chamado 'problema1'
adiciona outro Problema chamado 'problema2'
edita o problema2, e troca o nome dele pra 'problema1'

Isso vale? Ficaria 2 problemas com o mesmo nome. Isso traria alguns problemas depois, como por exemplo: Editar o Problema com nome 'problema1' iria editar os dois problemas, não dá pra saber qual dos dois era pra ser editado. Depois de fazer a query, esses dois problemas inclusive ficariam iguais, ia ficar basicamente uma duplicidade dele no db.

Vou mudar essa função. O que eu devia ter feito era 'checar se já existe um problema com o novo nome, se o nome estiver sendo modificado'

_, err := GetProblemByName(cosmos, problem.Name)

if err == nil {
return modelStorage.Problem{}, fmt.Errorf("there is already a problem with name %v", problem.Name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor: consider using typed errors, as they might make your functions easier to test and to reuse the error for other use cases:

type MyAwesomeError struct {
    SomeField string
}

func (e MyAwesomeError) Error() string {
    return fmt.Errorf("some error occurred and the field is %s", e.SomeField)
}

func (e MyAwesomeError) Is(err error) bool {
    _, ok := err.(MyAwesomeError)
    return ok
}

Copy link
Member

Choose a reason for hiding this comment

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

I added the tips to our notes so we can learn about them and replace in the code =]

res, err := cosmos.ExecuteQuery(query)
if err != nil {
fmt.Println("Failed to execute a gremlin command", query.String())
return problem, err
Copy link
Collaborator

Choose a reason for hiding this comment

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

another tip here that might improve debugability:
consider using xerrors. This way you can have a builtin "stacktrace" of errors.
when you build an error like:

// imagine that err is fmt.Errorf("some stuff happened")
return xerrors.Errorf("loading some stuff: %w", err)

the final error returns "loading some stuff: some stuff happened"
and the xerrors.Is(err, target error) call returns true if either the error is either of both, so it's also useful for testing

response := api.ResponseArray(res)
vertices, err := response.ToVertices()
vertices, _ := response.ToVertices()
Copy link
Collaborator

Choose a reason for hiding this comment

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

out of curiosity: why is the second return ignored?

@rebecacalazans rebecacalazans self-requested a review September 4, 2021 22:17
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