Conversation
pheobeayo
requested review from
Teddy1792,
joelamouche and
oscarwroche
as code owners
joelamouche
left a comment
joelamouche
left a comment
There was a problem hiding this comment.
Overall looks good but I wasn't able to test, see comment
| id: string; | ||
| name: string; | ||
| description: string; | ||
| owner_address: string; |
There was a problem hiding this comment.
If you look at backend/src/application/dtos/project_dtos.rs you will see that the api doesn't return owner_address.
This is why when testing locally I get
Also if I look at backend/migrations/004_create_projects_table.sql I can see this field is not in the db either.
Please add owner_address in the backend to fix this pb.
Also, in the future, pls run and test locally your app before posting a PR.
There was a problem hiding this comment.
Okay, I will work on it
joelamouche
left a comment
joelamouche
left a comment
There was a problem hiding this comment.
I can't stress this enough: you need to test features locally before submitting a PR for review
|
|
||
| export type CreateProjectData = { | ||
| name: string; | ||
| description: string; |
There was a problem hiding this comment.
status is missing here but required in backend/src/application/dtos/project_dtos.rs whichy is why I'm getting this
joelamouche
left a comment
joelamouche
left a comment
There was a problem hiding this comment.
A little comment on db migrations
| description TEXT NOT NULL, | ||
| status VARCHAR(50) NOT NULL CHECK (status IN ('proposal', 'ongoing', 'rejected')), | ||
| creator VARCHAR(42) NOT NULL, | ||
| owner_address VARCHAR(42) GENERATED ALWAYS AS (creator) STORED, |
There was a problem hiding this comment.
Be careful not to modify existing migrations because this will cause a bug when deploying to prod. Always add new migration files
There was a problem hiding this comment.
Adding a new migration was responsible for the errors, that was why the checks were not passing. But if I need to add new ones, I will have to work on it.
2 participants
fixes #166