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.

Sign up for GitHub

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

Jump to bottom

Update the button order on copy #19227

Merged
merged 1 commit into from
Sep 3, 2019
Merged

Update the button order on copy #19227

merged 1 commit into from
Sep 3, 2019

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Aug 29, 2019 •
edited
Loading

The service template copy should update the button order that stored in the options hash.

@h-kataria please review

Fixes the last remaining issue with https://bugzilla.redhat.com/show_bug.cgi?id=1740556

@d-m-u
Copy link
Contributor Author

d-m-u commented Aug 29, 2019

@miq-bot add_label bug, ivanchuk/yes

@d-m-u d-m-u force-pushed the fixing_service_template_button_order branch from 8ea6074 to eea42b8 Compare August 29, 2019 20:53
mkanoor
mkanoor reviewed Aug 30, 2019
View reviewed changes
app/models/service_template/copy.rb
@@ -5,7 +5,7 @@ def template_copy(new_name = "Copy of " + name + Time.zone.now.to_s)
if template_valid? && type != 'ServiceTemplateAnsiblePlaybook'
ActiveRecord::Base.transaction do
dup.tap do |template|
template.update_attributes(:name => new_name, :display => false)
template.update!(:name => new_name, :display => false, :options => {:button_order => []})
Copy link
Contributor

Choose a reason for hiding this comment

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

@d-m-u How does the button_order get updated to cb-#{id}? Is there a rails callback that's updating it?

Copy link
Contributor Author

@d-m-u d-m-u Aug 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

that's what the rest of the changes, 30 and 35, in this file are doing

@d-m-u d-m-u force-pushed the fixing_service_template_button_order branch 2 times, most recently from 1b53d33 to 0899c32 Compare August 30, 2019 19:34
@d-m-u
Copy link
Contributor Author

d-m-u commented Aug 30, 2019

@dmetzger57 this should be the last remaining thing needed for the ticket

@d-m-u
Copy link
Contributor Author

d-m-u commented Aug 30, 2019

@tinaafitz @bdunne anyone want to review please?

@d-m-u d-m-u force-pushed the fixing_service_template_button_order branch from 0899c32 to fdcd9c2 Compare August 30, 2019 19:53
@h-kataria
Copy link
Contributor

@d-m-u let me know when you have an update to test this in UI, currently when copying a Catalog Item it only copies custom buttons directly under an item and ignore custom button groups.

@d-m-u d-m-u force-pushed the fixing_service_template_button_order branch 2 times, most recently from faa7a7a to 9ce4609 Compare August 30, 2019 20:56
@h-kataria
Copy link
Contributor

verified in UI. Issue is fixed.

@d-m-u d-m-u force-pushed the fixing_service_template_button_order branch from 9ce4609 to 9a2756b Compare August 30, 2019 21:11
@d-m-u d-m-u force-pushed the fixing_service_template_button_order branch from 9a2756b to 8ad6599 Compare August 30, 2019 21:57
@miq-bot
Copy link
Member

miq-bot commented Aug 30, 2019

Checked commit d-m-u@8ad6599 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 👍

@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 3, 2019

could I please get eyes on this? it's needed for today's build...

@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 3, 2019

@miq-bot assign @gmcculloug

@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 3, 2019

@lfu @ZitaNemeckova
anyone at all want to review this? please?

@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 3, 2019

@tinaafitz could you please help me get this moving?

@d-m-u
Copy link
Contributor Author

d-m-u commented Sep 3, 2019

@gmcculloug could I please get you to merge?

@gmcculloug gmcculloug merged commit 0780eec into ManageIQ:master Sep 3, 2019
@gmcculloug gmcculloug added this to the Sprint 120 Ending Sep 16, 2019 milestone Sep 3, 2019
simaishi pushed a commit that referenced this pull request Sep 3, 2019
@gmcculloug @simaishi
Merge pull request #19227 from d-m-u/fixing_service_template_button_o…
02de6c9 
…rder

Update the button order on copy

(cherry picked from commit 0780eec)

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

simaishi commented Sep 3, 2019

Ivanchuk backport details:

$ git log -1
commit 02de6c93213ef0dee92e23f46565a523c925cde2
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Tue Sep 3 11:48:54 2019 -0400

    Merge pull request #19227 from d-m-u/fixing_service_template_button_order
    
    Update the button order on copy
    
    (cherry picked from commit 0780eece232a47f139bebda46772b6cf68618384)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1740556

@d-m-u d-m-u deleted the fixing_service_template_button_order branch September 26, 2019 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkanoor mkanoor mkanoor approved these changes

@h-kataria h-kataria h-kataria approved these changes

@tinaafitz tinaafitz tinaafitz approved these changes

Assignees

@gmcculloug gmcculloug

Labels
blocker bug changelog/yes ivanchuk/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@d-m-u @h-kataria @miq-bot @simaishi @tinaafitz @mkanoor @gmcculloug