Adding the delete action to the building a web app guide#349
Adding the delete action to the building a web app guide#349carolyncole wants to merge 3 commits intohanami:mainfrom
Conversation
katafrakt
left a comment
katafrakt
left a comment
There was a problem hiding this comment.
Hey, nice work! I left few comments about the content.
timriley
left a comment
timriley
left a comment
There was a problem hiding this comment.
This is great, thank you @carolyncole! I've just left a couple of extra suggestions in addition to @katafrakt's. Once these are sorted, this will be good to merge :)
Thank you again! ❤️
| def handle(request, response) | ||
| result = book_repo.delete(request.params[:id]) | ||
|
|
||
| response.flash[:notice] = if !result.nil? |
There was a problem hiding this comment.
Regarding the logic in this action, I think we can just assume that all deletes will be successful. This means we can do away with the result conditional, and just jump straight to setting response.flash[:notice] = "Book deleted" straight away.
This would also remove the issue @katafrakt raised regarding the flash.now in the "else" side of the conditional.
|
|
||
| To delete a book we will only need one action, destroy. | ||
|
|
||
| First, let's update our routes. We could add `:delete` to our only list, but we can just remove the only list as we will have implemented all actions a resource provides ([:index, :show, :new, :create, :edit, :update, :delete]). |
There was a problem hiding this comment.
| First, let's update our routes. We could add `:delete` to our only list, but we can just remove the only list as we will have implemented all actions a resource provides ([:index, :show, :new, :create, :edit, :update, :delete]). | |
| First, let's update our routes. We could add `:delete` to our `only:` list, but we can just remove the this as we will have implemented all actions a resource provides ([:index, :show, :new, :create, :edit, :update, :delete]). |
A couple of tweaks to make the sentence a bit easier to read: distinguish "only" as a code argument for formatting it as so, and then referring to it as "this list" in the second half of the sentence. Just doing this because "only" is a common word in ordinary sentence construction.
Co-authored-by: Paweł Świątkowski <katafrakt@users.noreply.github.com>
|
@timriley @katafrakt Thanks for the feedback. I have updated the docs. |
3 participants
This is a follow on to #347