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

Replaces angular ownership form with data-driven-form #5046

Merged
merged 7 commits into from Dec 7, 2018

Conversation

Hyperkid123
Copy link
Contributor

Continuing migrating to React forms.

This time it is ownership form.
It is located at Services -> My Services -> select some service -> Set Ownership.

@Hyperkid123
Copy link
Contributor Author

@miq-bot add_reviewer @himdel
@miq-bot add_reviewer @martinpovolny
@miq-bot add_label react
@miq-bot add_label hammer/no

%strong
= _('Note: Some items might be hidden due to the possibility of an ownership change')

- unless @explorer
Copy link
Contributor

Choose a reason for hiding this comment

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

x edit buttons angular should no longer be needed :)

= _('Note: Some items might be hidden due to the possibility of an ownership change')

- unless @explorer
= render :partial => "layouts/angular/x_edit_buttons_angular"

- if @ownershipitems
- @embedded = true
- @quadicon_no_url = true
- @gtl_type = settings(:views, :tagging)
= render :partial => "layouts/gtl"
Copy link
Contributor

Choose a reason for hiding this comment

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

render :partial => "layouts/gtl", :locals => {:no_flash_div => true}

}

loadInitialData = objectIds =>
http.post('/service/ownership_form_fields', { object_ids: objectIds })
Copy link
Contributor

@himdel himdel Dec 6, 2018

Choose a reason for hiding this comment

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

Sorry, all those /service/.. are wrong :(. (also goes for cancelUrl and submitUrl)

We need to be using the current controller, otherwise this fails for all the screens changing ownership for non-services.

Looks like the code is expecting the controller to be one of:

          when "orchestration_stack"
          when "service"
          when "vm_or_template", "vm_infra", "vm_cloud", "vm"
          when "miq_template"

(get_class_from_controller_param)

Copy link
Contributor

Choose a reason for hiding this comment

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

You can use /${ManageIQ.controller}/ownership_form_fields if it helps.

@himdel himdel self-assigned this Dec 6, 2018
@miq-bot
Copy link
Member

miq-bot commented Dec 6, 2018

Checked commits Hyperkid123/manageiq-ui-classic@f412d11~...376e4e5 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

@himdel
Copy link
Contributor

himdel commented Dec 7, 2018

/orchestration_stack/show/227?display=instances 👍
/service/explorer 👍 (works on Services)
/vm_infra/explorer 👍
/vm_cloud/explorer 👍
(vm, vm_or_template and miq_template controllers just redirect to infra or cloud 👍 )

Changing both user & group for a single item 👍
Changing just user or group for a single item 👍
Changing both for multiple items 👍

Changing just user for mulitple items 👎 - resets group
Changing just group for multiple items 👎 - resets user

... but turns out those 2 are a separate bug, introduced by #1578, fixing separately. (#5064)

@himdel himdel added this to the Sprint 101 Ending Dec 17, 2018 milestone Dec 7, 2018
@himdel himdel merged commit 81efe0e into ManageIQ:master Dec 7, 2018
@himdel
Copy link
Contributor

himdel commented Dec 7, 2018

Causes travis failures:

 FAIL  app/javascript/spec/set-service-ownership-form/set-service-ownership-form.spec.js
  Set service ownership form component
    ✕ encountered a declaration exception (3ms)
  ● Set service ownership form component › encountered a declaration exception
    Cannot spy the miqAjaxButton property because it is not a function; undefined given instead

Fixed in #5065

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
React rewrite
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants