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

Add rake task to export custom buttons #17699

Merged
merged 1 commit into from
Jul 25, 2018

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Jul 12, 2018

We need to be able to import/export custom buttons. This is basically a generic active record object export that works on custom buttons.

The related import PR:
#17726

Related:

#17726

Exporting

Create a directory for the exports:
mkdir /tmp/custom_buttons
Export cbs:
rake evm:export:custom_buttons -- --directory /tmp/custom_buttons

The export has a list of custom_button_sets with nested custom_buttons and associated resource actions as well as custom_buttons without sets and their associated resources.

export looks like this:
screen shot 2018-07-19 at 8 58 09 am

@miq-bot miq-bot added the wip label Jul 12, 2018
@d-m-u
Copy link
Contributor Author

d-m-u commented Jul 12, 2018

@miq-bot add_label enhancement

@d-m-u d-m-u force-pushed the custom_button_export branch 5 times, most recently from 07909fb to aabc4dd Compare July 18, 2018 17:15
@d-m-u d-m-u force-pushed the custom_button_export branch 3 times, most recently from 4813ce3 to 6a84cf4 Compare July 18, 2018 20:42
@gmcculloug gmcculloug self-assigned this Jul 19, 2018
@d-m-u d-m-u changed the title [WIP] Start of adding rake task for cb exports [WIP] Add rake task for custom button export Jul 19, 2018
@d-m-u
Copy link
Contributor Author

d-m-u commented Jul 19, 2018

@tinaafitz @syncrou @eclarizio
@whoever_wants_to_review_me

@d-m-u
Copy link
Contributor Author

d-m-u commented Jul 19, 2018

@miq-bot remove_label wip

@miq-bot miq-bot changed the title [WIP] Add rake task for custom button export Add rake task for custom button export Jul 19, 2018
@miq-bot miq-bot removed the wip label Jul 19, 2018
@ManageIQ ManageIQ deleted a comment from d-m-u Jul 19, 2018
Copy link
Contributor

@syncrou syncrou left a comment

Choose a reason for hiding this comment

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

@d-m-u - some small nits, but would like to see specs around this.

end
end

export_dir = options[:directory]
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 Should we have some validation that this directory exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

def self.create_association_list(obj, item)
associations = obj.class.try(:reflections)
if associations
associations = associations.collect { |r| { r[0] => r[1].class.to_s.demodulize } }.select { |d| d.values.first != "BelongsToReflection" && d.keys.first != "all_relationships" }
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 - Building the associations are in the weeds, but I wouldn't mind seeing variable names that reflect what the variable is e.g. r, d

@d-m-u d-m-u force-pushed the custom_button_export branch 2 times, most recently from aa2c3fd to 8bee5d7 Compare July 19, 2018 19:21
@miq-bot
Copy link
Member

miq-bot commented Jul 19, 2018

Checked commit d-m-u@e6f816f with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
3 files checked, 0 offenses detected
Everything looks fine. 🍰

Copy link
Contributor

@syncrou syncrou left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gmcculloug gmcculloug changed the title Add rake task for custom button export Add rake task to export custom buttons Jul 25, 2018
@gmcculloug
Copy link
Member

@d-m-u Looks good. My only comment is around exporting the userid for custom_button. It makes sense to include it here for when import/export is used as a backup solution. My concern is what to do during import into a new system when that userid does not exist.

At the moment this ID is only used for tracking who created the button. I'll comment on the import PR so we can address this.

@gmcculloug gmcculloug merged commit 3648368 into ManageIQ:master Jul 25, 2018
@gmcculloug gmcculloug added this to the Sprint 91 Ending Jul 30, 2018 milestone Jul 25, 2018
@d-m-u d-m-u deleted the custom_button_export branch October 25, 2018 14:15
@borisko123
Copy link
Contributor

borisko123 commented Mar 15, 2020

Hi, it seems I've found the problem in https://github.com/ManageIQ/manageiq/blob/master/spec/lib/task_helpers/imports/custom_buttons_spec.rb
Namely, the certain example is passed, but not for good reason.
The example at line 28 is about raising error, when we have 'bad file' in directory
It is passed, but this is not due to right flow, but due to error in
https://github.com/ManageIQ/manageiq/blob/master/lib/task_helpers/imports/custom_buttons.rb
The glob on line 6 is about *.yaml, BUT rspec generates temp 'bad' file with extension yamlXXXXX
where XXXX is some random thing.
So the glob does not include this file and the actually repeats " context "with existing identical buttons""

It seems the glob should be like *.yaml* in order to test this case
Also, line 33 should be deleted.

Also, we should explicitly remove tmp file in the example code, otherwize the whole test may fail
randomly.
Something like:
file = Tempfile.new('foo') begin ...do something with file... ensure file.close file.unlink # deletes the temp file end
Am I right?

@gmcculloug
Copy link
Member

Hi @borisko123, thanks for reporting this issue. Can you please open a git issue and someone can take a look. Thanks.

@borisko123
Copy link
Contributor

Hi @borisko123, thanks for reporting this issue. Can you please open a git issue and someone can take a look. Thanks.

Hi @gmcculloug,
Among others things at PR #20137(add dialog support, ability to update existing buttons), I also fixed this issue.The solution was to use another Tempfile initializer, which enables to retain file extension.
The glob is untouched.
Please take a look

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