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

UI Add support to copy class, instance, method automate objects #105

Closed
dclarizio opened this issue Jun 26, 2014 · 8 comments
Closed

UI Add support to copy class, instance, method automate objects #105

dclarizio opened this issue Jun 26, 2014 · 8 comments
Assignees
@h-kataria
Labels
enhancement

Comments

@dclarizio
Copy link

Back end support in #73.

When positioned on a specific class/instance/method, or when one is selected from a list, show a "Copy <Class/Instance/Method>" button in the toolbar. May need to disable it if there are no unlocked domains that can be copied to.

Once pressed, the right cell changes to a "Copy <Class/Instance/Method> <c/i/m name/description>" form.

  • First field is a drop down to select the Domain to copy to. If only 1 available, it would just be text, not a drop down. For same domain as the source, put "(same domain)" suffix in the text for that entry. Present the domains in priority order with the top choice as the default initially selected value.
  • Second field is a check box that is initially checked called "Copy to the same Namespace in the target domain" "Copy will override Source". If the same domain was chosen, this should be hidden and the next field shown.
  • Third field is a tree chooser with the Domain's Namespace nodes for which Namespace to copy to. If the above check box is checked, this field should be hidden. Otherwise, a single Namespace in the chosen domain should be selectable.
  • When a Namespace is chosen in the tree for an Instance or Method copy, a check box to "Overwrite the existing <Instance/Method>?" should appear so the user can force the copy to overwrite the existing element.

When the Copy button is pressed, call the back end methods and present any errors. If no errors, tree needs to be redrawn to reflect the changes and user should be put back to where they started positioned on the newly copied element with the success message.
@dclarizio
Copy link
Author

@mkanoor @gmcculloug @tinaafitz Please review the above design and provide any comments or suggestions. Thx, Dan

@mkanoor
Copy link
Contributor

mkanoor commented Jun 26, 2014

@dclarizio @gmcculloug @tinaafitz

  1. Should we have an option that says "Override this instance/method" which basically hides the namespace and doesn't allow the user to change it. For the override to work the Namespace can't be changed. If an instance exists in say MIQ/X/Y/Z/INST1 when copied to the new domain with the sole purpose of an override it should be under CUST/X/Y/Z/INST1. If we let the user change the Namespace he might change it to CUST/X/X/X/INST1 and the override would fail. We might want to differentiate between a generic copy and a override.
  2. Will the UI check if the instance/method exists in a new domain and prompt the user for an overwrite before calling the backend.

@dclarizio
Copy link
Author

@mkanoor
1: How about we change field 2 check box label to "Copy will override Source <class/instance/method>"? If unchecked, user chooses a target Namespace.
2: We could make that check and show an overwrite check box if it already exists. For an instance or method copy it makes sense. What if a class is being copied and it exists, would forcing it replace the entire class and all child instances/methods of that class?

@mkanoor
Copy link
Contributor

mkanoor commented Jun 26, 2014

@dclarizio @gmcculloug @tinaafitz

  1. Sounds good. Also at the end of a "Copy will override Source" should we switch to the newly created instance/method from the current domain.class.instance/method.
  2. The overwrite for the instance/methods looks good. For a class it would be a challenge. We already have the ability to delete class in the UI so a user could delete the class and all of its instances and try the copy again. I think once a class exists in the target domain we should not let them overwrite it. Eventhough the backend has support for it, doing it from this UI could be dangerous.

@dclarizio
Copy link
Author

@mkanoor Agreed on 1, we should position on the new element on any copy, good idea. I will change the design above to include 1 and 2. Thx, Dan

@mkanoor
Copy link
Contributor

mkanoor commented Jun 27, 2014

@dclarizio @gmcculloug @tinaafitz
I think the use case we should tackle first is
"User wants to modify a readonly instance or method", we are calling this overriding.
So should the override option be available only for instances and methods? When you override a instance or a method we will create the namespace(s) and classes under the covers if they don't exist.

I am not sure if a user would ever want to override a class, they can create a new class using the class editor or maybe copy an existing one to get a good starting point.

Copying instances/methods into the same class or a different class should be a different feature, that is unrelated to overriding, which requires a strict adherence to the namespace and class structure. Under the covers the logic might be the same but presentation wise we should make that distinction.

@dclarizio
Copy link
Author

@mkanoor @gmcculloug @tinaafitz
Hmmm, if we had an "Override this Instance/Method" button, what happens if the element is already overridden? Would we jump them to the existing one? What if there are multiple domains that override that element, which do we jump them to? What if some of those domains are locked or disabled?

For this reason, I'm thinking we just have copy and have the "override" check box that simply means the copy will be placed in the exact same namespace in the chosen domain. Without significantly changing the UI to present the overrides along with the original elements, I'm afraid we would confuse the user more by calling the copy function "override".

If we need to discuss further, let's have a call. Thx, Dan

@h-kataria h-kataria mentioned this issue Jul 7, 2014
Merged
h-kataria added a commit to h-kataria/manageiq that referenced this issue Jul 8, 2014
@h-kataria
Added support to copy class, instance, method automate objects.
58485ad 
- Added hidden product features to support copy of class, instance, method automate objects.
- Added copy buttons in toolbars and changed application_helper to display new buttons.
- Added support in controller for copy buttons.
- Added spec tests to test new model methods unlocked_domains? and all_unlocked_domains.
- Added spec tests to test copy of class in controller spec tests.
- Added any specific routes needed for these tasks.

Issue ManageIQ#105
h-kataria added a commit to h-kataria/manageiq that referenced this issue Jul 8, 2014
@h-kataria
Rubocop comments addressed.
cd23587 
- Fixed any issues found during testing and cleanup, found some issues because of the PR that was merged right before this PR to remove @record from session object.
- Added new copy_objects_form view, missed adding in previous commit.
- Added new miq_ae_domains_spec model, missed adding in previous commit.

Issue ManageIQ#105
h-kataria added a commit to h-kataria/manageiq that referenced this issue Jul 8, 2014
@h-kataria
PR comments addressed.
b3f33f0 
- Converted copy_objects_form view to haml.
- Refactored code to address PR comments.
- Renamed methods in miq_ae_domain model and fixed them to exclude domain records with system value as NULL or false.
- Added new model method "copy" to miq_ae_instance, miq_ae_method, miq_ae_class models to clean up controller code and be called from controller to save copied objects.
- Fixed spec tests to call newly renamed methods, and fixed Product Features count in miq_product_feature_spec.rb

Issue ManageIQ#105
h-kataria added a commit to h-kataria/manageiq that referenced this issue Jul 9, 2014
@h-kataria
Fixed issues found dutring testing.
9cad678 
- Fixed labels on the copy form.
- Fixed text on toolbar buttons and right cell header in copy form.
- Fixed open_parent_nodes method to retunr parent node of the existing node so children can be added to that node. Also added code to remove any existing children of the node in automate tree before adding child nodes, it was causing duplication of nodes in the tree.
- Changed code to pass in entry_point variable to common ae_tree_select view to display in the title of tree pop-up.

Issue ManageIQ#105
h-kataria added a commit to h-kataria/manageiq that referenced this issue Jul 9, 2014
@h-kataria
Fixed issues found dutring testing.
34e9d96 
- Fixed labels on the copy form.
- Fixed text on toolbar buttons and right cell header in copy form.
- Fixed open_parent_nodes method to retunr parent node of the existing node so children can be added to that node. Also added code to remove any existing children of the node in automate tree before adding child nodes, it was causing duplication of nodes in the tree.
- Changed code to pass in entry_point variable to common ae_tree_select view to display in the title of tree pop-up.
- Added new fqname_sans_domain method that returns fqname of selected namespace without domain, to prevent an error during copy.

Issue ManageIQ#105
@h-kataria
Copy link
Contributor

@dclarizio This is completed, can be closed

@Fryguy Fryguy added this to the Sprint 09 - Jul 9, 2014 milestone Aug 29, 2014
simaishi pushed a commit that referenced this issue Nov 10, 2017
@simon3z @simaishi
Merge pull request #105 from zakiva/add_image_name
0f30b0a 
Image scanning: Add image name to task name
(cherry picked from commit 7c48e385e8d10f20adaf164e59fe3876fbd7b318)

https://bugzilla.redhat.com/show_bug.cgi?id=1496943
d-m-u pushed a commit to d-m-u/manageiq that referenced this issue Jun 6, 2018
@simon3z @d-m-u
Merge pull request ManageIQ#105 from zakiva/add_image_name
19389cc 
Image scanning: Add image name to task name
(cherry picked from commit 7c48e385e8d10f20adaf164e59fe3876fbd7b318)

https://bugzilla.redhat.com/show_bug.cgi?id=1496943
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@h-kataria h-kataria

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Fryguy @dclarizio @h-kataria @mkanoor