Order custom buttons by array of ids #18060

lpichler · 2018-10-04T19:44:41Z

This PR updates #18049 and replacing order.

There user defined order for custom buttons so order is determined by set of ids in custom_button_set.set_data[:button_order] (not by name)

I used PostgreSQL for order.

Links

@miq-bot assign @bdunne
cc @gtanzillo
thanks @h-kataria

@miq-bot add_label hammer/yes, gaprindashvili/yes

lpichler · 2018-10-04T19:45:19Z

@simaishi so BZ https://bugzilla.redhat.com/show_bug.cgi?id=1628737 don't have merged last PR yet at this moment.

bdunne · 2018-10-05T12:35:06Z

spec/models/application_record_spec.rb

+      end
+
+      context 'by string column' do
+        it 'orders by memory_shares column' do


Ordering by name here, right?

bdunne · 2018-10-05T12:38:19Z

app/models/application_record.rb

@@ -3,6 +3,11 @@
 class ApplicationRecord < ActiveRecord::Base
  self.abstract_class = true

+  scope :with_array_order, lambda { |ids, column = :id, column_type = :bigint|


Do we need this is ApplicationRecord?

For this BZ no. Now we need it only for CustomButtonSet model. But it looks to me as useful functionality so I put it here because we have user defined-order on more places.

What do you think? Where I should put it? Should I put it only to CustomButtonSet model and then eventually in the future put it to the mixin?

@bdunne Are you ok with leaving this in ApplicationRecord? Seems like it wouldn't hurt to have it there.

@lpichler I'm interpreting @bdunne response to 👎 for keeping this in ApplicationRecord. Let's move it to CustomButtonSet. We can always elevate it later is another model needs it.

thanks, just small correction for what I wrote - it was about CustomButton model

miq-bot · 2018-10-05T15:02:21Z

Checked commits lpichler/manageiq@54de2a3~...8d1d914 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 0 offenses detected
Everything looks fine. 🍰

h-kataria · 2018-10-05T19:17:29Z

looks good. Thx for fixing @lpichler

…uttonSet

lpichler · 2018-10-10T16:49:37Z

done, thanks @gtanzillo @bdunne @h-kataria

@miq-bot assign @gtanzillo
@miq-bot unassign @bdunne

miq-bot · 2018-10-10T16:50:16Z

@lpichler unrecognized command 'unassign', ignoring...

Accepted commands are: add_label, add_reviewer, assign, close_issue, move_issue, remove_label, rm_label, set_milestone

gtanzillo

👍

himdel · 2018-10-11T08:34:51Z

@lpichler This causes a travis failure for UI classic:

 PG::UndefinedFunction: ERROR:  function array_position(bigint[], bigint) does not exist

According to https://www.postgresql.org/docs/9.4/static/functions-array.html vs https://www.postgresql.org/docs/9.5/static/functions-array.html, that function only appeared in postgres 9.5.

Can you fix please? :)

bdunne · 2018-10-11T13:34:42Z

@carbonin Haven't we been running Postgres 9.5 since euwe branch?

carbonin · 2018-10-11T13:37:11Z

Yeah, I'm not sure why we're testing against 9.4 anywhere at this point.

carbonin · 2018-10-11T13:42:20Z

Aside from that, is there any reason to use this sorting strategy vs the one in #18087? The list of custom buttons should be rather small so there's probably not much of a performance difference, right? @lpichler ?

himdel · 2018-10-11T14:03:58Z

OK, so sounds like we can change the ui-classic travis to use 9.5 and close #18087 ?

@bdunne Please note that config/initializers/postgres_required_versions.rb in master still only complains about <9.4, not 9.4.

So.. are we sure people know that 9.4 is not supported? :)

(Also, we need to know when it was dropped, since this PR is gaprindashvili/yes.)

himdel · 2018-10-11T14:12:13Z

Also, the last mention of PostgreSQL in the manageiq CHANGELOG points at #16171. Which is from last year, against gaprindashvili and explicitly states that 9.4 is supported.

So far, looks like the only justification for #15994 was "because we can" and not "because we dropped 9.4".

(That said, I definitely don't object to dropping 9.4, assuming postgres_required_versions is changed to match.)

EDIT for completenes:

developer_setup also mentions 9.4,
and so do the latest docs (though I have no idea what ManageIQ 4.x is)

lpichler · 2018-10-11T14:19:36Z

@carbonin yes, we would not face performance issues regard to count of custom buttons. So both solutions are usable.

carbonin · 2018-10-11T14:51:07Z

@himdel The appliance (which is the only way manageiq ships to users) is using 9.5 and has been for at least a year.

If this is not what you are testing against then that is a problem and should be changed regardless of what the docs say. If the docs say that 9.4 is supported then maybe that is true for developers, but it is not the version that we are shipping in production so that should be made clear.

himdel · 2018-10-11T15:02:38Z

IMO 9.4 is supported only as long as we're testing it.
So, dropping 9.4 from ui-travis means dropping support for 9.4.
I'm willing to fix the docs, but... I'd like an answer stronger than "maybe" :).

(posted on gitter hoping for stronger anwsers :))

@carbonin And does that mean that even gaprindashvili appliances are always 9.5?

carbonin · 2018-10-11T15:04:02Z

Yes, I think we should drop 9.4 @Fryguy @jrafanie Want to ack that?

jrafanie · 2018-10-11T15:04:07Z

@himdel I'd say we drop support for a postgresql version the branch "After" we update appliances to use the newer version. Since we're at 9.5 in euwe, fine, gaprindashvili, and hammer, I'm 👍 to dropping 9.4 completely

gtanzillo · 2018-10-11T15:16:28Z

I am also 👍 for dropping 9.4. Especially if continuing to test with 9.4 means that we are not testing on 9.5

himdel · 2018-10-12T13:03:05Z

OK, looks like these are all that's needed to get rid of the remaining 9.4- references (except for vcr_cassettes and changelogs and blogposts for older versions):

…_of_ids Order custom buttons by array of ids (cherry picked from commit 4995683) https://bugzilla.redhat.com/show_bug.cgi?id=1628737

simaishi · 2018-10-12T15:47:19Z

Hammer backport details:

$ git log -1
commit dae2099d765d92dceb996ab7ff1133f160092acb
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Wed Oct 10 14:09:50 2018 -0400

    Merge pull request #18060 from lpichler/order_custom_buttons_by_array_of_ids
    
    Order custom buttons by array of ids
    
    (cherry picked from commit 49956837c8143e928004beb8fce4bf4babedc081)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1628737

…_of_ids Order custom buttons by array of ids (cherry picked from commit 4995683) https://bugzilla.redhat.com/show_bug.cgi?id=1635759

simaishi · 2018-11-05T14:57:43Z

Gaprindashvili backport details:

$ git log -1
commit b5765185e779217d33bc8a457c28e507ced06d6e
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Wed Oct 10 14:09:50 2018 -0400

    Merge pull request #18060 from lpichler/order_custom_buttons_by_array_of_ids
    
    Order custom buttons by array of ids
    
    (cherry picked from commit 49956837c8143e928004beb8fce4bf4babedc081)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1635759

…by_array_of_ids Order custom buttons by array of ids (cherry picked from commit 4995683) https://bugzilla.redhat.com/show_bug.cgi?id=1635759

miq-bot assigned bdunne Oct 4, 2018

miq-bot added gaprindashvili/yes labels Oct 4, 2018

bdunne reviewed Oct 5, 2018

View reviewed changes

lpichler force-pushed the order_custom_buttons_by_array_of_ids branch from 30ba222 to 8d1d914 Compare October 5, 2018 14:59

Order buttons by custom_button_set.set_data[:button_order] in CustomB…

cfb3c21

…uttonSet

lpichler force-pushed the order_custom_buttons_by_array_of_ids branch from 8d1d914 to cfb3c21 Compare October 10, 2018 16:49

miq-bot assigned gtanzillo and unassigned bdunne Oct 10, 2018

gtanzillo approved these changes Oct 10, 2018

View reviewed changes

gtanzillo added this to the Sprint 97 Ending Oct 22, 2018 milestone Oct 10, 2018

gtanzillo merged commit 4995683 into ManageIQ:master Oct 10, 2018

lpichler deleted the order_custom_buttons_by_array_of_ids branch October 11, 2018 10:29

lpichler mentioned this pull request Oct 11, 2018

Order CustomButtons in PostgreSQL 9.4 #18087

Closed

himdel mentioned this pull request Oct 11, 2018

Travis - change postgresql to 9.5 ManageIQ/manageiq-ui-classic#4758

Merged

simaishi added the blocker label Oct 12, 2018

simaishi pushed a commit that referenced this pull request Oct 12, 2018

Merge pull request #18060 from lpichler/order_custom_buttons_by_array…

dae2099

…_of_ids Order custom buttons by array of ids (cherry picked from commit 4995683) https://bugzilla.redhat.com/show_bug.cgi?id=1628737

simaishi added hammer/backported and removed hammer/yes labels Oct 12, 2018

himdel mentioned this pull request Oct 22, 2018

PostgreSQL 9.4 is no longer supported, replace with 9.5 ManageIQ/manageiq-documentation#986

Merged

simaishi pushed a commit that referenced this pull request Nov 5, 2018

Merge pull request #18060 from lpichler/order_custom_buttons_by_array…

b576518

…_of_ids Order custom buttons by array of ids (cherry picked from commit 4995683) https://bugzilla.redhat.com/show_bug.cgi?id=1635759

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Nov 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Order custom buttons by array of ids #18060

Order custom buttons by array of ids #18060

lpichler commented Oct 4, 2018

lpichler commented Oct 4, 2018 •

edited

Loading

bdunne Oct 5, 2018

bdunne Oct 5, 2018

lpichler Oct 5, 2018

gtanzillo Oct 9, 2018

bdunne Oct 9, 2018

gtanzillo Oct 9, 2018

lpichler Oct 10, 2018

miq-bot commented Oct 5, 2018

h-kataria commented Oct 5, 2018

lpichler commented Oct 10, 2018 •

edited

Loading

miq-bot commented Oct 10, 2018

gtanzillo left a comment

himdel commented Oct 11, 2018

bdunne commented Oct 11, 2018

carbonin commented Oct 11, 2018

carbonin commented Oct 11, 2018

himdel commented Oct 11, 2018 •

edited

Loading

himdel commented Oct 11, 2018 •

edited

Loading

lpichler commented Oct 11, 2018

carbonin commented Oct 11, 2018

himdel commented Oct 11, 2018 •

edited

Loading

carbonin commented Oct 11, 2018

jrafanie commented Oct 11, 2018

gtanzillo commented Oct 11, 2018

himdel commented Oct 12, 2018 •

edited

Loading

simaishi commented Oct 12, 2018

simaishi commented Nov 5, 2018

Order custom buttons by array of ids #18060

Order custom buttons by array of ids #18060

Conversation

lpichler commented Oct 4, 2018

Links

lpichler commented Oct 4, 2018 • edited Loading

bdunne Oct 5, 2018

Choose a reason for hiding this comment

bdunne Oct 5, 2018

Choose a reason for hiding this comment

lpichler Oct 5, 2018

Choose a reason for hiding this comment

gtanzillo Oct 9, 2018

Choose a reason for hiding this comment

bdunne Oct 9, 2018

Choose a reason for hiding this comment

gtanzillo Oct 9, 2018

Choose a reason for hiding this comment

lpichler Oct 10, 2018

Choose a reason for hiding this comment

miq-bot commented Oct 5, 2018

h-kataria commented Oct 5, 2018

lpichler commented Oct 10, 2018 • edited Loading

miq-bot commented Oct 10, 2018

gtanzillo left a comment

Choose a reason for hiding this comment

himdel commented Oct 11, 2018

bdunne commented Oct 11, 2018

carbonin commented Oct 11, 2018

carbonin commented Oct 11, 2018

himdel commented Oct 11, 2018 • edited Loading

himdel commented Oct 11, 2018 • edited Loading

lpichler commented Oct 11, 2018

carbonin commented Oct 11, 2018

himdel commented Oct 11, 2018 • edited Loading

carbonin commented Oct 11, 2018

jrafanie commented Oct 11, 2018

gtanzillo commented Oct 11, 2018

himdel commented Oct 12, 2018 • edited Loading

simaishi commented Oct 12, 2018

simaishi commented Nov 5, 2018

lpichler commented Oct 4, 2018 •

edited

Loading

lpichler commented Oct 10, 2018 •

edited

Loading

himdel commented Oct 11, 2018 •

edited

Loading

himdel commented Oct 11, 2018 •

edited

Loading

himdel commented Oct 11, 2018 •

edited

Loading

himdel commented Oct 12, 2018 •

edited

Loading