-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: main
Are you sure you want to change the base?
Conversation
- Model can change if more fields needs to be added
app/dbOperations/problems.go
Outdated
|
||
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") |
There was a problem hiding this comment.
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 =]
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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
}
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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() |
There was a problem hiding this comment.
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?
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.
model/problem.go
,dbOperations/problems.go
andhandler/problems.go
;How to test it
go build .
and./dont-panic-api
with a valid Gremlin database configuration.CREATE
It's a
POST
request with URLlocalhost:8080/api/problems
. Remember to put a valid body. Example: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 URLlocalhost:8080/api/problems
. To get a specific Problem, make a GET request with URLlocalhost: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 URLlocalhost: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 URLlocalhost: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.