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 Copy option to Catalog Item/Bundle #5667

Merged
merged 11 commits into from
Jul 17, 2019

Conversation

ZitaNemeckova
Copy link
Contributor

@ZitaNemeckova ZitaNemeckova commented Jun 3, 2019

Go to Services -> Catalogs -> Catalog Items -> select one -> Configuration -> Copy this Item

Backend PR(merged) ManageIQ/manageiq#18464 . Only valid and non-ServiceTemplateAnsiblePlaybook items can be copied.

Not all attributes are copied. Tracked in ManageIQ/manageiq#18949
It's not possible to copy a copy with custom buttons. Tracked in ManageIQ/manageiq#18954 => Fixed in ManageIQ/manageiq#18960

Toolbar:
Screenshot 2019-07-02 at 15 43 33

Form:
Screenshot 2019-07-02 at 15 45 47

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1678149

@miq-bot add_label wip

@miq-bot miq-bot changed the title Add Copy option to Catalog Item/Bundle [WIP] Add Copy option to Catalog Item/Bundle Jun 3, 2019
@miq-bot miq-bot added the wip label Jun 3, 2019
@ZitaNemeckova ZitaNemeckova force-pushed the copy_catalog branch 2 times, most recently from ce7138f to 2cf99cd Compare July 2, 2019 13:41
@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label unmergeable
@miq-bot add_label enhancement, changelog/yes, hammer/no, react

Copy link
Contributor

@PanSpagetka PanSpagetka left a comment

Choose a reason for hiding this comment

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

You have prefilled name in the form like "Copy of X", but form in initial state can't be submitted (Add button is disabled).


handleError = (error) => {
const { data: { error: { message } } } = error;
return !!message ? message : __('There was an error in copying. Item is not valid or Ansible.');
Copy link
Contributor

Choose a reason for hiding this comment

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

This error message is little bit confusing. I needed to read it several times to understand it correctly.

Copy link
Contributor

@h-kataria h-kataria Jul 8, 2019

Choose a reason for hiding this comment

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

@ZitaNemeckova maybe message should be "Selected Catalog Bundle or Ansible Playbook Items can not be copied."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went with Selected item can not be copied. Because it's Ansible Playbook or not valid.

@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot changed the title [WIP] Add Copy option to Catalog Item/Bundle Add Copy option to Catalog Item/Bundle Jul 11, 2019
@miq-bot miq-bot removed the wip label Jul 11, 2019
@h-kataria
Copy link
Contributor

@ZitaNemeckova it seems to be working pretty good. Minor issues noticed

  • if i create a Catalog Item with a name "1" and then try to copy that with the same name i get a message on screen that "Name has already been taken" only if i shift my focus out of the text field, is that working as designed?
  • Second issue is that if i try to copy item with a name "1 " it does not complain and lets me add an item with the specified name, should we strip trailing/leading blank spaces

@miq-bot
Copy link
Member

miq-bot commented Jul 16, 2019

Checked commits ZitaNemeckova/manageiq-ui-classic@97d6abf~...97e3000 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 1 offense detected

app/controllers/catalog_controller.rb

@ZitaNemeckova
Copy link
Contributor Author

@h-kataria Fixed first point. About the second I still think that leaving trailing/leading blank spaces is ok. Fixing it would probably create more trouble.

@h-kataria
Copy link
Contributor

@ZitaNemeckova @d-m-u found another issue, when copying a catalog item, it's additional Tenant information does not get copied.

@h-kataria h-kataria self-assigned this Jul 16, 2019
@tinaafitz
Copy link
Member

@d-m-u Can you add the additional tenant information @h-kataria mentioned above?

@h-kataria
Copy link
Contributor

verified in UI. Looks good.

@h-kataria h-kataria added this to the Sprint 116 Ending Jul 22, 2019 milestone Jul 17, 2019
@h-kataria h-kataria merged commit a3d8183 into ManageIQ:master Jul 17, 2019
@ZitaNemeckova ZitaNemeckova deleted the copy_catalog branch March 23, 2020 12:55
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