feat: transaction crud #1432
feat: transaction crud #1432
Conversation
mathnogueira
force-pushed
the
feat/transaction-crud
branch
2 times, most recently
from
6998ddf
to
6180a55
Compare
mathnogueira
force-pushed
the
feat/transaction-crud
branch
from
6348668
to
078df29
Compare
| step_number int NOT NULL, | ||
|
|
||
| CONSTRAINT transaction_steps_transactions_fk | ||
| FOREIGN KEY (transaction_id, transaction_version) REFERENCES transactions(id, "version") |
There was a problem hiding this comment.
don't you need an fk here to the tests table?
There was a problem hiding this comment.
Thing is that the PK of tests are id and version. But as we discussed a couple of days ago, we are always going to use the latest version available. So I decided to not keep the fk here
| { | ||
| "name": "test-transaction", | ||
| "description": "a transaction", | ||
| "steps": [] |
There was a problem hiding this comment.
it would be better to have a test here
| @@ -34,6 +34,139 @@ paths: | |||
| $ref: "./definition.yaml#/components/schemas/ExecuteDefinitionResponse" | |||
| 422: | |||
| description: "trying to create a test with an already existing ID" | |||
| /transactions: | |||
| get: | |||
There was a problem hiding this comment.
Based on the UI design, we are going to need one single endpoint that returns tests and transactions (should support search, pagination, etc.). We could keep the get /transactions endpoint and implement the unified one in a new PR.
There was a problem hiding this comment.
Yeah I remember that the idea was to have a single hub for both tests and transactions
There was a problem hiding this comment.
Hey @mathnogueira great job man!, I left some non-blocking comments!
| @@ -34,6 +34,139 @@ paths: | |||
| $ref: "./definition.yaml#/components/schemas/ExecuteDefinitionResponse" | |||
| 422: | |||
| description: "trying to create a test with an already existing ID" | |||
| /transactions: | |||
| get: | |||
There was a problem hiding this comment.
Yeah I remember that the idea was to have a single hub for both tests and transactions
|
|
||
| func (m Model) Transaction(ctx context.Context, in openapi.Transaction) (model.Transaction, error) { | ||
| tests := make([]model.Test, len(in.Steps)) | ||
| for i, testID := range in.Steps { |
There was a problem hiding this comment.
isn't it weird to have db queries a the mappers level? I think this should only set the default values (hydratation) to the object from the request and then the following logic of the controller can do the rest.
Maybe this is something we can address later on 😄
There was a problem hiding this comment.
I'll try to solve this in another PR
| "transaction_steps", | ||
| "transactions", | ||
| "test_runs", | ||
| "tests", |
There was a problem hiding this comment.
I forgot to add the environments table here, is it necessary?
There was a problem hiding this comment.
yes, it is. I'll add it
This PR...
Changes
Fixes
Checklist