[#641] Add EditPlanNameModal for editing name and description of completed plans #663

mturley · 2018-09-24T20:41:58Z

Closes #641.
Depends on ManageIQ/manageiq#17989.

codeclimate · 2018-09-24T20:42:30Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameModal.js

+    });
+  };
+
+  render() {


Function render has 71 lines of code (exceeds 25 allowed). Consider refactoring.

codeclimate · 2018-09-24T20:42:30Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameModal.js

+
+    const formBody = (
+      <Form horizontal>
+        <Field


Identical blocks of code found in 3 locations. Consider refactoring.

codeclimate · 2018-09-24T20:42:30Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameModal.js

+            })
+          ]}
+        />
+        <Field


Identical blocks of code found in 3 locations. Consider refactoring.

codeclimate · 2018-09-24T20:42:30Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameModal.js

+  };
+
+  render() {
+    const {


Similar blocks of code found in 11 locations. Consider refactoring.

codeclimate · 2018-09-24T20:42:30Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameReducer.js

+        .set('savingPlan', false)
+        .set('savingPlanRejected', false)
+        .set('savingPlanError', null);
+    case `${V2V_POST_EDIT_PLAN_TITLE}_REJECTED`:


Similar blocks of code found in 6 locations. Consider refactoring.

codeclimate · 2018-09-24T21:37:19Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameModal.js

+
+class EditPlanNameModal extends React.Component {
+  onSubmit = () => {
+    const {


Similar blocks of code found in 4 locations. Consider refactoring.

codeclimate · 2018-09-24T21:37:19Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameModal.js

+
+    return (
+      <Modal show={editPlanNameModalVisible} onHide={hideEditPlanNameModalAction}>
+        <Modal.Header>


Similar blocks of code found in 3 locations. Consider refactoring.

codeclimate · 2018-09-24T21:37:19Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameActions.js

+
+export { fetchTransformationPlansAction } from '../../OverviewActions';
+
+export const showAlertAction = (alertText, alertType = 'error') => dispatch => {


Similar blocks of code found in 3 locations. Consider refactoring.

codeclimate · 2018-09-24T21:37:19Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameModal.js

+        </Modal.Header>
+        <Modal.Body>
+          {!savingPlan ? formBody : spinner}
+          <div className="modal-alert">


Similar blocks of code found in 2 locations. Consider refactoring.

miq-bot · 2018-09-26T21:15:00Z

This pull request is not mergeable. Please rebase and repush.

…lans

codeclimate · 2018-09-27T14:01:09Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameModal.js

+
+class EditPlanNameModal extends React.Component {
+  onSubmit = () => {
+    const {


Similar blocks of code found in 6 locations. Consider refactoring.

miq-bot · 2018-09-27T14:04:21Z

Checked commits mturley/manageiq-v2v@7d3ec5a~...573db0d 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. 🍰

AparnaKarve · 2018-10-03T20:53:38Z

@mturley would it be possible to display the "Plan Name already exists" alert at top of the modal?

mturley · 2018-10-03T21:13:15Z

@AparnaKarve I could, but I didn't want to have to add a bunch of whitespace above the name field or have the contents move around vertically when the alert is shown. In the Plan Wizard we have the wizard steps as a natural margin to lay this alert over, but displaying it at the top as-is would cover up the name field.

@vconzola, I'm curious about your thoughts on this. Should we leave the alert at the bottom of the modal, or add sufficient margin above the name field so it can appear at the top?

vconzola · 2018-10-04T13:56:09Z

@mturley @AparnaKarve I'm not sure we need the inline message at all. I think the text below the input text box is probably sufficient if you added "plan name already exists" after "Enter a unique name". But let me check with some other designers. Also, drop the "Please" at the beginning. We're not supposed to use "please" in our UIs.

vconzola · 2018-10-04T16:02:07Z

@mturley OK, here's the deal. Inline notification is only needed for server side validation of form information. In our case, we're validating as information is entered into the field so a field error message only is appropriate. So you can do what I said above: "Plan name already exists. Enter a unique name."

codeclimate · 2018-10-04T18:28:30Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameModal.js

+    });
+  };
+
+  render() {


Function render has 60 lines of code (exceeds 25 allowed). Consider refactoring.

codeclimate · 2018-10-04T18:28:30Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameReducer.js

+        .set('savingPlan', false)
+        .set('savingPlanRejected', false)
+        .set('savingPlanError', null);
+    case `${V2V_POST_EDIT_PLAN_NAME}_REJECTED`:


Similar blocks of code found in 6 locations. Consider refactoring.

mturley · 2018-10-04T18:31:13Z

This one should now be all set. The "Please" was already in place for the two wizards, and I was reusing the validation helper from the plan wizard where it was defined, so I went ahead and updated the text for the mapping wizard too. ("Plan name already exists. Enter a unique name." and "Mapping name already exists. Enter a unique name.")

codeclimate · 2018-10-04T18:34:35Z

app/javascript/react/screens/App/Overview/components/EditPlanNameModal/EditPlanNameModal.js

+    });
+  };
+
+  render() {


Function render has 53 lines of code (exceeds 25 allowed). Consider refactoring.

AparnaKarve · 2018-10-04T21:01:03Z

...pt/react/screens/App/Overview/screens/PlanWizard/components/PlanWizardGeneralStep/helpers.js

-      props.showAlertAction(sprintf(__('Name %s already exists'), newPlanName));
-      const error = { name: 'Please enter a unique name' };
+      if (props.showAlertAction) props.showAlertAction(sprintf(__('Name %s already exists'), newPlanName));
+      const error = { name: 'Plan name already exists. Enter a unique name.' };


__('Plan name already exists. Enter a unique name.')

AparnaKarve · 2018-10-04T21:03:47Z

@mturley Can you make that one little gettext change above?
Everything else looks good!

mturley · 2018-10-04T21:08:22Z

Whoops, thanks @AparnaKarve! Done.

AparnaKarve

Thanks @mturley - this looks great!!

[#641] Add EditPlanNameModal for editing name and description of completed plans (cherry picked from commit 424c024)

simaishi · 2018-10-05T16:06:27Z

Hammer backport details:

$ git log -1
commit d7ade0ec3ed0f7c5ca0a8af5e74bdf9897a2ac97
Author: Aparna Karve <[email protected]>
Date:   Thu Oct 4 14:13:23 2018 -0700

    Merge pull request #663 from mturley/641-edit-plan-name
    
    [#641] Add EditPlanNameModal for editing name and description of completed plans
    
    (cherry picked from commit 424c024e395dd76e0d891b7a6694e184bde3367c)

mturley requested review from AparnaKarve and michaelkro September 24, 2018 20:41

codeclimate bot reviewed Sep 24, 2018

View reviewed changes

mturley force-pushed the 641-edit-plan-name branch from 7021d00 to de03401 Compare September 24, 2018 21:36

codeclimate bot reviewed Sep 24, 2018

View reviewed changes

mturley added hammer/yes Sprint 14 labels Sep 25, 2018

miq-bot added the unmergeable label Sep 26, 2018

mturley changed the title ~~Add EditPlanNameModal for editing name and description of completed plans~~ [#641] Add EditPlanNameModal for editing name and description of completed plans Sep 26, 2018

mturley added 2 commits September 27, 2018 09:59

Add EditPlanNameModal for editing name and description of completed p…

7d3ec5a

…lans

Fix broken test

573db0d

mturley force-pushed the 641-edit-plan-name branch from e4b8b8c to 573db0d Compare September 27, 2018 14:00

codeclimate bot reviewed Sep 27, 2018

View reviewed changes

miq-bot removed the unmergeable label Sep 27, 2018

codeclimate bot reviewed Oct 4, 2018

View reviewed changes

mturley force-pushed the 641-edit-plan-name branch from 12d229d to 1a24622 Compare October 4, 2018 18:33

codeclimate bot reviewed Oct 4, 2018

View reviewed changes

AparnaKarve reviewed Oct 4, 2018

View reviewed changes

Remove alert from EditPlanNameModal, replace validation text

1b79249

mturley force-pushed the 641-edit-plan-name branch from 1a24622 to 1b79249 Compare October 4, 2018 21:07

AparnaKarve approved these changes Oct 4, 2018

View reviewed changes

AparnaKarve merged commit 424c024 into ManageIQ:master Oct 4, 2018

mturley deleted the 641-edit-plan-name branch October 4, 2018 21:19

simaishi pushed a commit that referenced this pull request Oct 5, 2018

Merge pull request #663 from mturley/641-edit-plan-name

d7ade0e

[#641] Add EditPlanNameModal for editing name and description of completed plans (cherry picked from commit 424c024)

simaishi added hammer/backported and removed hammer/yes labels Oct 5, 2018

himdel mentioned this pull request Sep 18, 2019

Remove dead V2V_EDIT_PLAN_TITLE_SHOW_ALERT V2V_EDIT_PLAN_TITLE_HIDE_ALERT #1036

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#641] Add EditPlanNameModal for editing name and description of completed plans #663

[#641] Add EditPlanNameModal for editing name and description of completed plans #663

mturley commented Sep 24, 2018

codeclimate bot Sep 24, 2018

codeclimate bot Sep 24, 2018

codeclimate bot Sep 24, 2018

codeclimate bot Sep 24, 2018

codeclimate bot Sep 24, 2018

codeclimate bot Sep 24, 2018

codeclimate bot Sep 24, 2018

codeclimate bot Sep 24, 2018

codeclimate bot Sep 24, 2018

miq-bot commented Sep 26, 2018

codeclimate bot Sep 27, 2018

miq-bot commented Sep 27, 2018

AparnaKarve commented Oct 3, 2018

mturley commented Oct 3, 2018

vconzola commented Oct 4, 2018 •

edited

Loading

vconzola commented Oct 4, 2018

codeclimate bot Oct 4, 2018

codeclimate bot Oct 4, 2018

mturley commented Oct 4, 2018

codeclimate bot Oct 4, 2018

AparnaKarve Oct 4, 2018

AparnaKarve commented Oct 4, 2018

mturley commented Oct 4, 2018

AparnaKarve left a comment

simaishi commented Oct 5, 2018


		export { fetchTransformationPlansAction } from '../../OverviewActions';

		export const showAlertAction = (alertText, alertType = 'error') => dispatch => {

[#641] Add EditPlanNameModal for editing name and description of completed plans #663

[#641] Add EditPlanNameModal for editing name and description of completed plans #663

Conversation

mturley commented Sep 24, 2018

codeclimate bot Sep 24, 2018

Choose a reason for hiding this comment

codeclimate bot Sep 24, 2018

Choose a reason for hiding this comment

codeclimate bot Sep 24, 2018

Choose a reason for hiding this comment

codeclimate bot Sep 24, 2018

Choose a reason for hiding this comment

codeclimate bot Sep 24, 2018

Choose a reason for hiding this comment

codeclimate bot Sep 24, 2018

Choose a reason for hiding this comment

codeclimate bot Sep 24, 2018

Choose a reason for hiding this comment

codeclimate bot Sep 24, 2018

Choose a reason for hiding this comment

codeclimate bot Sep 24, 2018

Choose a reason for hiding this comment

miq-bot commented Sep 26, 2018

codeclimate bot Sep 27, 2018

Choose a reason for hiding this comment

miq-bot commented Sep 27, 2018

AparnaKarve commented Oct 3, 2018

mturley commented Oct 3, 2018

vconzola commented Oct 4, 2018 • edited Loading

vconzola commented Oct 4, 2018

codeclimate bot Oct 4, 2018

Choose a reason for hiding this comment

codeclimate bot Oct 4, 2018

Choose a reason for hiding this comment

mturley commented Oct 4, 2018

codeclimate bot Oct 4, 2018

Choose a reason for hiding this comment

AparnaKarve Oct 4, 2018

Choose a reason for hiding this comment

AparnaKarve commented Oct 4, 2018

mturley commented Oct 4, 2018

AparnaKarve left a comment

Choose a reason for hiding this comment

simaishi commented Oct 5, 2018

vconzola commented Oct 4, 2018 •

edited

Loading