Skip to content
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

Allow to save multiple tag categories in chargeback controller #4310

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Jul 17, 2018

Behaviour:

Before:
It was possible to save just assignment for Tagged VMs and Instances only for ONE Tag Category

After:
It is possible to save it also for multiple categories:

screen shot 2018-07-18 at 10 36 29

Implementation details

@edit[:cb_assign][:tags] as you can see in
code(@edit[:cb_assign][cb_assign_key]) is bringing
selected chargeback rates for tags in selected tag category which is
stored in @edit[:new][:cbtag_cat]

to keep selected chargeback rates for any count of tag categories
we need to extend @edit[:cb_assign][:tags] to
@edit[:cb_assign][:tags] [category_id] and this hash can
keep multiple chargeback rates across any count of
tag categories.

we need to update code when chargeback rate is
selected ( in method cb_assign_params_to_edit called by
controller action cb_assign_field_changed) and then
when we want to save this selection of chargeback rates
in cb_assign_update.

Note
This PR will need probably also some work to make it better from UX aspects but leave it on other PR.
This PR accomplish it from functional aspects.

Links

https://bugzilla.redhat.com/show_bug.cgi?id=1581349
https://bugzilla.redhat.com/show_bug.cgi?id=1601228

@miq-bot miq-bot added the wip label Jul 17, 2018
@lpichler lpichler force-pushed the save_multiple_tag_categories_in_chargeback_assignments branch from 205b3ae to e80a6a7 Compare July 17, 2018 21:00
@edit[:cb_assign][:tags] as you can see in
code(@edit[:cb_assign][cb_assign_key]) is bringing
selected chargeback rates for tags in selected tag category which is
stored in @edit[:new][:cbtag_cat]

to keep selected chargeback rates for any count of tag categories
we need to extend @edit[:cb_assign][:tags]  to
@edit[:cb_assign][:tags] [category_id]  and this hash can
keep multiple chargeback rates across any count of
tag categories.

we need to update code when chargeback rate is
selected (  in method cb_assign_params_to_edit called
controller action cb_assign_field_changed) and then
when we want to save this selection of chargeback rates
in cb_assign_update.
@lpichler lpichler force-pushed the save_multiple_tag_categories_in_chargeback_assignments branch from e80a6a7 to 5b77319 Compare July 18, 2018 08:02
@miq-bot
Copy link
Member

miq-bot commented Jul 18, 2018

Checked commit lpichler@5b77319 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Haml - Linter::Haml STDERR:
warning: parser/current is loading parser/ruby23, which recognizes
warning: 2.3.6-compliant syntax, but you are running 2.3.3.
warning: please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.

@lpichler lpichler changed the title [WIP] Allow to save multiple tag categories in chargeback controller Allow to save multiple tag categories in chargeback controller Jul 18, 2018
@miq-bot miq-bot removed the wip label Jul 18, 2018
@lpichler
Copy link
Contributor Author

@miq-bot add_label gaprindashvili/yes, blocker

cc @gtanzillo

please review @hstastna

@miq-bot assign @mzazrivec

@hstastna
Copy link

@miq-bot add_reviewer @hstastna

@miq-bot miq-bot requested a review from hstastna July 19, 2018 12:50
Copy link

@hstastna hstastna left a comment

Choose a reason for hiding this comment

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

For me the change is not obvious from the screenshot, and I am missing any steps how to test this properly. I'm not sure how these Assignments work. How can I check if multiple tag categories were properly set? Tried to test it but I still see only the one last selected tag category there. Maybe I am missing something. Thank you.

@hstastna
Copy link

hstastna commented Jul 19, 2018

Ahh, I was missing the next steps how to test it:

  1. Cloud Intel > Chargeback > Assignments > Compute
  2. Set Assign To as Tagged... from the dropdown
  3. Then set Tag Category, too
  4. Set some Selections from the dropdowns
  5. Save the assignment
  6. Repeat the steps 3 - 5 and save again
  7. Change the Tag Category to the previous one you've set
    => you can see the same selections as you've previously set, not just <Nothing>

@lpichler Please, correct me if I am wrong. Thanks. 🎇

@hstastna
Copy link

hstastna commented Jul 19, 2018

The weird thing is that if I try to set the assignment and to save it more times in a row, right after saving it, the previous assignment appears in the screen, not the last one. I mean, different Category and Selections appear in the screen, not the latest saved ones. For me this behavior does not make sense.

@mzazrivec mzazrivec added the wip label Jul 31, 2018
@mzazrivec mzazrivec changed the title Allow to save multiple tag categories in chargeback controller [WIP] Allow to save multiple tag categories in chargeback controller Jul 31, 2018
lpichler added a commit to lpichler/manageiq that referenced this pull request Aug 3, 2018
It is possible to assign chargeback rates to more tags from
various tag categories (ManageIQ/manageiq-ui-classic#4310).
lpichler added a commit to lpichler/manageiq that referenced this pull request Aug 3, 2018
It is possible to assign chargeback rates to more tags from
various tag categories (ManageIQ/manageiq-ui-classic#4310).

It will cause costs for each rate will be added up.

Example:

VM has tag department/it and enviroment/test

Rate 1 is assigned to tag department/it
Rate 2 is assigned to tag enviroment/test

Then if checkbox from (ManageIQ/manageiq-ui-classic#4310)
will turn on it will add costs from Rate 1 and Rate 2
together.

cost = Rate 1 + Rate 1

But metric value will not be doubled.
@lpichler lpichler changed the title [WIP] Allow to save multiple tag categories in chargeback controller Allow to save multiple tag categories in chargeback controller Aug 3, 2018
@miq-bot miq-bot removed the wip label Aug 3, 2018
@lpichler
Copy link
Contributor Author

lpichler commented Aug 3, 2018

@hstastna thanks for the review and comments, yes UX is weird but from functional point of view (saving multiple rates across various category) it is OK. So I am suggesting to do such UX in follow up PR.

@mzazrivec mzazrivec added this to the Sprint 92 Ending Aug 13, 2018 milestone Aug 3, 2018
@mzazrivec mzazrivec merged commit a296004 into ManageIQ:master Aug 3, 2018
lpichler added a commit to lpichler/manageiq that referenced this pull request Aug 3, 2018
It is possible to assign chargeback rates to more tags from
various tag categories (ManageIQ/manageiq-ui-classic#4310).

It will cause costs for each rate will be added up.

Example:

VM has tag department/it and enviroment/test

Rate 1 is assigned to tag department/it
Rate 2 is assigned to tag enviroment/test

Then if checkbox from (ManageIQ/manageiq-ui-classic#4310)
will turn on it will add costs from Rate 1 and Rate 2
together.

cost = Rate 1 + Rate 1

But metric value will not be doubled.
lpichler added a commit to lpichler/manageiq that referenced this pull request Aug 3, 2018
It is possible to assign chargeback rates to more tags from
various tag categories (ManageIQ/manageiq-ui-classic#4310).

It will cause costs for each rate will be added up.

Example:

VM has tag department/it and enviroment/test

Rate 1 is assigned to tag department/it
Rate 2 is assigned to tag enviroment/test

Then if checkbox from (ManageIQ/manageiq-ui-classic#4310)
will turn on it will add costs from Rate 1 and Rate 2
together.

cost = Rate 1 + Rate 1

But metric value will not be doubled.
lpichler added a commit to lpichler/manageiq that referenced this pull request Aug 3, 2018
It is possible to assign chargeback rates to more tags from
various tag categories (ManageIQ/manageiq-ui-classic#4310).

It will cause costs for each rate will be added up.

Example:

VM has tag department/it and enviroment/test

Rate 1 is assigned to tag department/it
Rate 2 is assigned to tag enviroment/test

Then if checkbox from (ManageIQ/manageiq-ui-classic#4310)
will turn on it will add costs from Rate 1 and Rate 2
together.

cost = Rate 1 + Rate 1

But metric value will not be doubled.
simaishi pushed a commit that referenced this pull request Aug 6, 2018
…n_chargeback_assignments

Allow to save multiple tag categories in chargeback controller
(cherry picked from commit a296004)

https://bugzilla.redhat.com/show_bug.cgi?id=1612889
@simaishi
Copy link
Contributor

simaishi commented Aug 6, 2018

Gaprindashvili backport details:

$ git log -1
commit 4bcb15607377b17d3810f6fff120b92a254061c7
Author: Milan Zázrivec <[email protected]>
Date:   Fri Aug 3 15:14:09 2018 +0200

    Merge pull request #4310 from lpichler/save_multiple_tag_categories_in_chargeback_assignments
    
    Allow to save multiple tag categories in chargeback controller
    (cherry picked from commit a296004924bb35d0403b297f5c64b8f341aa5971)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1612889

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants