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

Convert the subnet form to DDF #7734

Merged
merged 1 commit into from
May 21, 2021

Conversation

DavidResende0
Copy link
Member

@DavidResende0 DavidResende0 commented May 14, 2021

Continuation of: #7632

Resolves: #6861
Depends On: ManageIQ/manageiq-providers-openstack#712

Before:
image

After:
image

package.json Outdated Show resolved Hide resolved
if (!!recordId && appendState.initialValues.type === 'ManageIQ::Providers::Openstack::NetworkManager::CloudSubnet') {
Object.assign(fields[0], {isDisabled: true});
Object.assign(fields[1], {isDisabled: true});
Object.assign(fields[4], {isDisabled: true});
Copy link
Member Author

Choose a reason for hiding this comment

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

This is needed since these fields are coming from the provider. There's also a chance that other providers may need similar treatment but I can't test those at the moment.

Comment on lines +49 to +52
values.enable_dhcp = values.dhcp_enabled;
delete values.dhcp_enabled;
delete Object.assign(values, values.extra_attributes).extra_attributes;
Copy link
Member Author

Choose a reason for hiding this comment

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

Both of these changes are to alter the values being submitted by the form so that the cloud subnet is created correctly (renaming dchp_enabled to enabled_dhcp and removing the extra_attributes object while moving its contents back directly into values).

The alternative to this is to change the form schema in the provider and the API to match what the back-end is expecting.

@DavidResende0
Copy link
Member Author

@miq-bot assign @kavyanekkalapu
@miq-bot add_reviewer @kavyanekkalapu , @Fryguy
@agrare

As far as I can tell the form seems to be working now, but I haven't been able to test the form with any non-openstack providers so there could be issues there.

I also left a couple of comments in different areas where I'm not thrilled with the solutions and I was hoping someone could take a look and maybe suggest alternatives, in particular ways that some of these changes could happen in the provider repos

@miq-bot miq-bot requested review from Fryguy and kavyanekkalapu May 14, 2021 15:29
@DavidResende0 DavidResende0 changed the title Convert the subnet form to DDF [WIP] Convert the subnet form to DDF May 14, 2021
@miq-bot miq-bot added the wip label May 14, 2021
app/controllers/ops_controller/settings/zones.rb Outdated Show resolved Hide resolved
app/controllers/ops_controller/settings/zones.rb Outdated Show resolved Hide resolved
app/javascript/components/subnet-form/index.jsx Outdated Show resolved Hide resolved
app/javascript/oldjs/services/alerts_center_service.js Outdated Show resolved Hide resolved
app/views/ops/_zone_form.html.haml Outdated Show resolved Hide resolved
spec/controllers/ops_controller/settings/zones_spec.rb Outdated Show resolved Hide resolved
@kavyanekkalapu
Copy link
Member

@DavidResende0 i checked functionality, it is working as expected, found one minor issue, edit form is loading slow, has delay of approximately 5 sec, it is just showing blank page until that time, it would be nice to show loading symbol until data is loaded.., please check if that is possible.

@DavidResende0 DavidResende0 force-pushed the react-cloud-subnet branch 2 times, most recently from 77fb874 to 4e145f1 Compare May 17, 2021 15:09
@DavidResende0 DavidResende0 changed the title [WIP] Convert the subnet form to DDF Convert the subnet form to DDF May 17, 2021
@miq-bot miq-bot removed the wip label May 17, 2021
@DavidResende0
Copy link
Member Author

@kavyanekkalapu, I looked into the loading issue for the edit form and I haven't been able to reproduce it but if I wanted to add a loading symbol, what would I do.

@miq-bot miq-bot requested a review from kavyanekkalapu May 19, 2021 18:56
@DavidResende0 DavidResende0 force-pushed the react-cloud-subnet branch 2 times, most recently from 4f420f0 to 3b005ba Compare May 19, 2021 21:03
app/controllers/cloud_subnet_controller.rb Show resolved Hide resolved
app/controllers/cloud_subnet_controller.rb Outdated Show resolved Hide resolved
app/controllers/cloud_subnet_controller.rb Show resolved Hide resolved
app/controllers/cloud_subnet_controller.rb Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@miq-bot
Copy link
Member

miq-bot commented May 20, 2021

Checked commit DavidResende0@9f53567 with ruby 2.6.3, rubocop 1.13.0, haml-lint 0.35.0, and yamllint
5 files checked, 2 offenses detected

app/views/cloud_subnet/edit.html.haml

  • ⚠️ - Line 2 - Layout/TrailingEmptyLines: Final newline missing.

app/views/cloud_subnet/new.html.haml

  • ⚠️ - Line 2 - Layout/TrailingEmptyLines: Final newline missing.

@Fryguy
Copy link
Member

Fryguy commented Sep 16, 2021

@DavidResende0 A conflict occurred during the backport of this pull request to morphy.

If this pull request is based on another pull request that has not been marked for backport, add the appropriate labels to the other pull request. Otherwise, please create a new pull request direct to the morphy branch in order to resolve this.

Conflict details:

diff --cc app/javascript/packs/component-definitions-common.js
index e5a2d8456d,018ff163d7..0000000000
--- a/app/javascript/packs/component-definitions-common.js
+++ b/app/javascript/packs/component-definitions-common.js
@@@ -41,8 -46,10 +41,9 @@@ import ReportChartWidget from '../compo
  import ReportDataTable from '../components/report-data-table';
  import RetirementForm from '../components/retirement-form';
  import ServiceDialogFromForm from '../components/service-dialog-from-form/service-dialog-from';
+ import SubnetForm from '../components/subnet-form';
  import EditServiceForm from '../components/edit-service-form';
  import SetOwnershipForm from '../components/set-ownership-form';
 -import ServersDataChart from '../components/provider-dashboard-charts/servers-pie-chart';
  import TableListViewWrapper from '../react/table_list_view_wrapper';
  import TaggingWrapperConnected from '../components/taggingWrapper';
  import { TagView } from '../tagging';
@@@ -104,6 -119,8 +105,11 @@@ ManageIQ.component.addReact('Retirement
  ManageIQ.component.addReact('ServiceDialogFromForm', ServiceDialogFromForm);
  ManageIQ.component.addReact('EditServiceForm', EditServiceForm);
  ManageIQ.component.addReact('SetOwnershipForm', SetOwnershipForm);
++<<<<<<< HEAD
++=======
+ ManageIQ.component.addReact('ServersDataChart', ServersDataChart);
+ ManageIQ.component.addReact('SubnetForm', SubnetForm);
++>>>>>>> 745801e296... Merge pull request #7734 from DavidResende0/react-cloud-subnet
  ManageIQ.component.addReact('TableListView', TableListView);
  ManageIQ.component.addReact('TableListViewWrapper', TableListViewWrapper);
  ManageIQ.component.addReact('TagGroup', props => <TagGroup {...props} />);

Fryguy pushed a commit that referenced this pull request Sep 16, 2021
Convert the subnet form to DDF

(cherry picked from commit 745801e)
@Fryguy
Copy link
Member

Fryguy commented Sep 16, 2021

Backported to morphy

commit afcb1947b3465960c355093d8bb1d6b3fcc36ac8 (HEAD -> morphy, upstream/morphy)
Author: Kavya Nekkalapu <37085529+kavyanekkalapu@users.noreply.github.com>
Date:   Fri May 21 11:15:35 2021 -0400

    Merge pull request #7734 from DavidResende0/react-cloud-subnet

    Convert the subnet form to DDF

    (cherry picked from commit 745801e2963ea5a3a2ac83da7279ad3d945c69c3)

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.

Form conversion: Networks/Subnets Add
6 participants