-
-
Notifications
You must be signed in to change notification settings - Fork 159
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
added DeleteOperation to delete action #1497
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! Thanks for starting another PR. This one will require changing how the action is executed.
Currently when you generate the action, you'll get generated code like this:
UserQuery.find(user_id).delete
flash.success = "Deleted the record"
redirect Index
But that just calls delete
directly on the model. Instead it should generate the proper DeleteOperation
and implement that. It'll end up generating code like this:
record = UserQuery.find(user_id)
User::DeleteOperation.destroy(record) do |operation, deleted|
flash.success = "Deleted the record"
redirect Index
end
You'll just have to figure out the dynamic calls that will generate the proper code, and then how to test that the correct code is generated for the correct model.
tasks/gen/templates/resource/actions/{{folder_name}}/delete.cr.ecr
Outdated
Show resolved
Hide resolved
Thank you @jwoertink! 💪 Really appreciate the guidance, it helps me understand how crystal and lucky is working 🙇 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Just one small change left that I just thought of, but the rest of it looks good. Thanks for doing this. I also noticed a few other things that I will open up some other issues on shortly 😄
tasks/gen/templates/resource/actions/{{folder_name}}/delete.cr.ecr
Outdated
Show resolved
Hide resolved
Thank you! 😃 Can't wait to give them a try 😁 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉
Purpose
Fixes #1487
Description
added DeleteOperation to delete action
@jwoertink I am pretty sure that it is not as easy as that change that I've added but I would like to work on it so if it is not enough then could you please give me more guidance and direct me where I can find information to fulfil this issue? 🙂
Checklist
crystal tool format spec src
./script/setup
./script/test