Skip to content

Fix update endpoint#24

Open
23nith wants to merge 8 commits intodevelopfrom
fix_update_endpoint
Open

Fix update endpoint#24
23nith wants to merge 8 commits intodevelopfrom
fix_update_endpoint

Conversation

@23nith
Copy link
Copy Markdown
Owner

@23nith 23nith commented Sep 22, 2022

No description provided.

@23nith 23nith marked this pull request as ready for review September 22, 2022 08:39
post '/add_list_item' => 'list_items#create'
# post '/add_list_item' => 'list_items#add_list_item'
post '/edit_list_item' => 'list_items#update'
get '/list_item/:id/edit' => 'list_items#edit'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

FYI you can also make use of resources :list_item - https://guides.rubyonrails.org/routing.html#resource-routing-the-rails-default

Comment on lines +35 to +37
def edit
@list_item = ListItem.find(params[:id])
end
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why do you need the edit endpoint? Rails already has a show endpoint by default right?

GET /list_item/:id

validates :book_id, presence: true
validates :user_id, presence: true
validates :rating, presence: true, numericality: { in: 1..5 }
validates :rating, presence: true, numericality: { in: 0..5 }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit

can make use of greater_than and less_than, instead of making rails iterate over a range of values. A bit more performant. But again , this is a nit. Performance is negligible

}, status: :unauthorized
end
render json: {
status: 200,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why do you need to include the status in the json object? Wouldn't HTTP status 200 already be enough for the frontend to know that it is successful?

@23nith 23nith force-pushed the fix_update_endpoint branch 2 times, most recently from 1cfae82 to 5a6d34e Compare September 25, 2025 05:04
@23nith 23nith force-pushed the fix_update_endpoint branch from 5a6d34e to e77fbb6 Compare September 25, 2025 05:12
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