Add condition to fix deletion of Default Container Image Rate #16792
Conversation
Fixing https://bugzilla.redhat.com/show_bug.cgi?id=1486658 Add condition to ensure_nondefault method in chargeback rate model to fix deletion of Default Container Image Rate in Cloud Intel -> Chargeback -> Rates so that the Rate can't be deleted and error message is displayed.
|
@miq-bot add_label bug, gaprindashvili/yes |
|
Checked commits hstastna/manageiq@ad0476e~...65ddd88 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
|
@martinpovolny @lpichler @h-kataria What do you think about this? Thanks a lot! ❇️ |
|
cc @zeari |
|
@miq-bot assign @gtanzillo |
| @@ -150,7 +150,7 @@ def ensure_unassigned | |||
| end | |||
|
|
|||
| def ensure_nondefault | |||
| if default? | |||
| if default? || description == 'Default Container Image Rate' | |||
There was a problem hiding this comment.
@hstastna few questions, can't description be edited, or have same chargeback rate with same description, or is description unique among chargeback rates?
There was a problem hiding this comment.
@gtanzillo @lpichler if "Default Container Image Rate" is a default rate that gets seeded from yml, why dont we change yml to populate default field as true here https://github.com/ManageIQ/manageiq/blob/master/db/fixtures/chargeback_rates.yml#L155
There was a problem hiding this comment.
@h-kataria If I click on Edit this Chargeback rate and try to change the description of 'Default Container Image Rate' and click on Save, the flash message occurs:
Can not change description of 'Default Container Image Rate'
If I try to add a new rate with the same description, the flash message occurs:
Description has already been taken
so I think this all works properly. I hope I understood your question properly 👼
There was a problem hiding this comment.
@hstastna i see that change was made here ManageIQ/manageiq-ui-classic#269 to not allow edit of Default Container Image chargeback rate. I still think it should have been marked as default rate to begin with. @gtanzillo @lpichler @dclarizio any comments?
There was a problem hiding this comment.
@h-kataria we added it here #13063 and what I remember we needed to distinguish this specific rate and it was possible only according to name. But yes it isn't the reason why we didn't set ChargebackRate#default to true in yaml. Maybe it was causing issues. Can you remember @zeari ?
There was a problem hiding this comment.
What we needed was a default-fallback rate, that if no rate is assigned that one is used.
We also needed the ability to edit it which wasnt possible for default rates.
Since default rates were not editable we made a special case of out "Default container image rate". The only place where that is used is in Chargeback for container images.
…delete_description Add condition to fix deletion of Default Container Image Rate (cherry picked from commit 164cd08) https://bugzilla.redhat.com/show_bug.cgi?id=1552260
|
Gaprindashvili backport details: |
|
@miq-bot add_label fine/yes |
…delete_description Add condition to fix deletion of Default Container Image Rate (cherry picked from commit 164cd08) https://bugzilla.redhat.com/show_bug.cgi?id=1568084
|
Fine backport details: |
…ge_Rate_delete_description Add condition to fix deletion of Default Container Image Rate (cherry picked from commit 164cd08) https://bugzilla.redhat.com/show_bug.cgi?id=1568084
Fixing https://bugzilla.redhat.com/show_bug.cgi?id=1486658
This PR has to be merged with ManageIQ/manageiq-ui-classic#3215
to completely fix the bug.
Summary:
Add condition to
ensure_nondefaultmethod in chargeback rate model to fix deletion of Default Container Image Rate in Cloud Intel -> Chargeback -> Rates so that the Rate can't be deleted and error message is properly displayed.Explanation:
Originally, checking if the selected rate is default (before its deletion), did not work well only if deleting rate(s) from a list of the rates. This was because of missing part of the condition for checking default rates here:
https://github.com/ManageIQ/manageiq/blob/master/app/models/chargeback_rate.rb#L153
Checking if the rate is default, worked only from rate details page. This was because of
https://github.com/ManageIQ/manageiq-ui-classic/blob/master/app/helpers/application_helper/button/chargeback_rate_remove.rb#L12
So I added the missing part of the condition here (this PR): https://github.com/ManageIQ/manageiq/compare/master...hstastna:Default_Container_Image_Rate_delete_description?expand=1#diff-7c998acd3d11c70ed830e216754f4b6bR153
I also edited
cb_rates_deletemethod in chargeback controller to be able to properly delete nondefault rates and also to show error messages about deleting default rates and list of the remaining rates - in a one step (more about this here ManageIQ/manageiq-ui-classic#3215)The bug is related to Compute Chargeback rates.