Skip to content
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

User crud #17

Merged
merged 6 commits into from
Feb 14, 2024
Merged

User crud #17

merged 6 commits into from
Feb 14, 2024

Conversation

capture120
Copy link
Contributor

@capture120 capture120 commented Feb 8, 2024

Description

Link to Ticket

Created user CRUD routes for GET, POST, PUT, DELETE. Fixed user schema to match unique/non-unique constraints. Updated go types/enums to allow null values.

How Has This Been Tested?

Verified route functionality using Postman.


Checklist

  • I have performed a self-review of my code
  • I have reached out to another developer to review my code
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes

@@ -22,7 +22,7 @@ func main() {
dsn := "host=localhost user=user password=pwd dbname=algo port=5434 sslmode=disable"
db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{})
if err != nil {
panic("Failed to connect to database")
panic("Failed to connect to database")
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason it added so much spacing? I'm wondering if you're set on tab spacing instead of space

@@ -1,17 +1,19 @@
-- init.sql
DROP TABLE IF EXISTS users;

CREATE TYPE risk_tolerance_enum AS ENUM ('low', 'medium', 'high');
CREATE TYPE risk_tolerance_enum AS ENUM ('low', 'medium', 'high', '');
Copy link
Contributor

Choose a reason for hiding this comment

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

We recently made the goals enum a table, I think this one makes sense as an enum but @aniamisiorek what are your thoughts here?

return nil, err
}

// Save the updated user back to the database
Copy link
Contributor

Choose a reason for hiding this comment

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

Going to check this, but does this update the row entry or save a new entry in the DB?

Copy link
Contributor

Choose a reason for hiding this comment

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

checked it does not add the entry and keep the old one all clear.

Copy link
Contributor

@leoRysing leoRysing left a comment

Choose a reason for hiding this comment

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

Looks to be all good, just check the Delete User case since something might be up there

userRoutes.POST("/", userController.CreateUser)
userRoutes.GET("/:id", userController.GetUserById)
userRoutes.PUT("/:id", userController.UpdateUserById)
userRoutes.DELETE("/:id", userController.DeleteUserById)
Copy link
Contributor

Choose a reason for hiding this comment

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

Was delete working for you It wasn't for me on my computer. Everything else is working good awesome job.

Copy link
Contributor

@leoRysing leoRysing Feb 9, 2024

Choose a reason for hiding this comment

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

Ok update this is what I'm currently getting when trying to delete;

2024/02/08 19:18:55 /Users/leroyshaigorodsky/Documents/GitHub/Algo/backend/src/services/user.go:59 ERROR: update or delete on table "users" violates foreign key constraint "scores_user_id_fkey" on table "scores" (SQLSTATE 23503) - I think that table will need the cascade thing too (could be good to make sure that tables that reference this table will have the ON DELETE CASCADE sql stuff to make sure that those rows get deleted when a user gets deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Delete was working for me, here is the request I made to the route:
image

While working through this ticket I had issues with schemas but they should be resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

if its working lets merge it in🕺

@leoRysing leoRysing self-requested a review February 14, 2024 02:18
@capture120 capture120 merged commit 64ce724 into main Feb 14, 2024
3 checks passed
@capture120 capture120 deleted the user-crud branch February 14, 2024 04:05
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.

2 participants