Protect user's work #648
Protect user's work #648
Conversation
|
Sorry to bother you again @bdunne but we had an argue in our team whether the "quick and dirty" approach implemented in #644 is good enough for now or not. In the end I decided to fix it properly, even of we don't expect user to ever be needing this feature. Kindly ask you to review the two resulting PRs: What we basically do now is we don't just destroy all menus on server update, but we go extra mile and check which ones can actually be preserved. |
|
Travis will be red until ManageIQ/manageiq#19126 gets merged |
| to_keep, to_destroy = current_menus.partition { |menu| desired_menus.include?(menu) } | ||
| to_create = desired_menus.reject { |menu| current_menus.include?(menu) } | ||
| server.pxe_menus.where(:file_name => to_destroy).destroy_all | ||
| data['pxe_menus'] = server.pxe_menus.where(:file_name => to_keep) + create_pxe_menus(to_create.map { |f| { 'file_name' => f } }) |
There was a problem hiding this comment.
feels a bit heavy on business logic here, I wonder if this should not be provided by a model helper method that we'd leverage.
There was a problem hiding this comment.
I think you're right @abellotti, here'se the utility method ManageIQ/manageiq#19134
Currently, when user updates PXE Server details and submits form, all related PxeMenus and PxImages are discarded so she needs to run "Recreate Relationships" to inventory them again. This approach works but unfortunately user has option to manually edit PxeImages when they are inventoried, and by discarding them we also discard her work. With this commit we therefore go out of our way and double check which PxeMenus (and subsequent PxeImages) are *really* affected by update and only discard those. This way, if user only updates PxeServer's name, nothing else will be recreated hence user's manual work will be preserved. Fixes ManageIQ/manageiq#18940 Signed-off-by: Miha Pleško <[email protected]>
|
Checked commit xlab-si@c30aa65 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
|
replaced by #707 |
Currently, when user updates PXE Server details and submits form, all related PxeMenus and PxeImages are discarded so she needs to run "Recreate Relationships" to inventory them again.
This approach works but unfortunately user has option to manually edit PxeImages when they are inventoried, and by discarding them we also discard her work.
With this commit we therefore go out of our way and double check which PxeMenus (and subsequent PxeImages) are really affected by update and only discard those.
This way, if user only updates PxeServer's name, nothing else will be recreated hence user's manual work will be preserved.
Depends on ManageIQ/manageiq#19126
Depends on ManageIQ/manageiq#19134
Fixes ManageIQ/manageiq#18940
@miq-bot add_label bug,ivanchuk/yes
@miq-bot assign @bdunne
/cc @tadeboro @matejart @gberginc