Archive Plans #367

michaelkro · 2018-06-01T00:22:01Z

Enable archiving of completed transformation plans (successful & errored)

Add archive button to Completed Migrations ListView
Set up fetching so that archived and active (unarchived) plans are
returned separately.
Set up POST to archive a plan
Modify MigrationsCompletedList to also be used for display of
archived plans

ADDITIONAL CHANGES

Refactor to remove usage of Object.values() due to node support
Extract migrations filters strings to constants

Closes #314, #315, #316

Notes

To quickly unarchive all your plans, use the following command in the rails console
```
ServiceTemplateTransformationPlan.update_all(deleted_on: nil)
```
Alternatively, use the unarchive! method on individual records
Big thank you to @AparnaKarve for showing me how to set up filters when fetching plans!

Screens

Flow

Archive Button

Archive Modal (Plan with unmigrated VMs)

Archive List

Archive Details

Archive Empty

priley86 · 2018-06-01T12:43:10Z

app/javascript/components/index.js

@@ -94,6 +94,15 @@ export const coreComponents = [
      fetchTransformationPlansUrl:
        '/api/service_templates/?' +
        "filter[]=type='ServiceTemplateTransformationPlan'" +
+        '&filter[]=deleted_on=nil' +


If I read @agrare's email correctly... "You shouldn't need to use the deleted on attribute directly, there is an active and an archived scope that you should be able to use."... this doesn't make sense?

@agrare can you confirm these two query filters for archived/unarchived are correctly constructed?

@priley86 I can confirm that the following works:

>> ServiceTemplate.active => #<ActiveRecord::Relation [#<ServiceTemplate id: 1, name: nil, description: nil, guid: "ffd1a950-a007-49d8-891f-9c4741c5dbb9", type: nil, service_template_id: nil, options: {}, created_at: "2018-06-01 13:07:25", updated_at: "2018-06-01 13:07:25", display: nil, evm_owner_id: nil, miq_group_id: 1, service_type: "unknown", prov_type: nil, provision_cost: nil, service_template_catalog_id: nil, long_description: nil, tenant_id: 1, generic_subtype: nil, deleted_on: nil>]> >> ServiceTemplate.archived => #<ActiveRecord::Relation []>

I can't confirm how that should be accessed through the API

@agrare To be able to use ServiceTemplate.archived in an API, we would need archived to be a virtual column I think.
Accessing deleted_on directly is working well in the API, and should be OK to use it directly since there are no other complexities involved with the archived/ unarchived logic.

deleted_on == nil is unarchived
deleted_on != nil is archived

Oh really having it as a scope isn't enough? Well if you can get it working through the API and put a PR to core in I'll merge it.

@agrare I just created this core PR - ManageIQ/manageiq#17509

@agrare Looks like we will need a PR to amend the virtual column to archived,
since archived? works for this case -

/api/service_templates/<id>?attributes=archived?

but not for this case -

/api/service_templates?filter[]=archived?=false, what @michaelkro is using above

(There seems to be a bug with filter in the API... not sure how easy it is to resolve. From what I can tell it's related to MiqExpression)

priley86 · 2018-06-01T12:50:48Z

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

@@ -176,8 +186,11 @@ class Overview extends React.Component {

    createTransformationPlanRequestAction(url).then(() => {
      retryMigrationAction(planId);
-      setMigrationsFilterAction('Migration Plans in Progress');
-      fetchTransformationPlansAction(fetchTransformationPlansUrl);
+      setMigrationsFilterAction('In Progress Plans');


Looking at these again... it would be nice to break the "In Progress Plans", "Migration Plans in Progress", "Migration Plans Not Started" strings out into constants across the board. It seems like "In Progress Plans" is the same as "Migration Plans in Progress" ?

priley86 · 2018-06-01T12:52:47Z

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

-    'Migration Plans Not Started',
-    'Migration Plans in Progress',
-    'Migration Plans Completed'
+    'Not Started Plans',


ahh you are renaming them in an array. Can we also reference the strings as constants?

priley86 · 2018-06-01T12:53:10Z

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

@@ -81,15 +96,15 @@ const Migrations = ({
      </Grid.Col>
      {transformationPlans.length > 0 && (
        <React.Fragment>
-          {activeFilter === 'Migration Plans Not Started' && (
+          {activeFilter === 'Not Started Plans' && (


same here...constants preferred.

priley86 · 2018-06-01T12:55:47Z

app/javascript/react/screens/App/Overview/components/Migrations/MigrationsCompletedList.js

+                {failed && (
+                  <p>
+                    {__(
+                      'This plan includes VMs that failed to migrate. If you archive the plan, you will not be able to retry the failed migrations'


Missing period at the end

"If you archive the plan, you will not be able to retry the failed migrations."

michaelkro · 2018-06-01T20:37:12Z

@priley86, the CI failures are due to the use of Object.values() here

Seems like this method isn't supported in node 6.x

I can remove my unit tests to keep CI from failing, but wasn't sure how we want to handle this

priley86 · 2018-06-01T20:49:03Z

yes i'm fine w/ this... Object.keys it is i guess? Not sure why Travis was passing before.

michaelkro · 2018-06-04T13:03:20Z

Our CI script runs builds using node 6 and 6.10. Object.values is an ES2017 feature that is not supported by node < 6.14

Please see node.green

priley86 · 2018-06-04T14:34:45Z

I'm not sure how important Node6 support is for v2v at this point. It's nice having the support, but doesn't seem required anymore. My guess is we should just stay in sync w/ ui-classic which appears to be >=6.9.1 right now.
https://github.com/ManageIQ/manageiq-ui-classic/blob/master/package.json#L108

@AparnaKarve @AllenBW can you confirm?

AparnaKarve · 2018-06-04T14:46:15Z

@priley86 Agree, let's stay in sync with ui-classic

priley86 · 2018-06-05T13:06:38Z

can you rebase #391 when you get a chance? there was a request to merge that one first just now...

michaelkro · 2018-06-05T13:08:08Z

Yup, no worries!

serenamarie125

LGTM! @vconzola did you have a chance to review?

priley86 · 2018-06-06T12:00:53Z

this looks good to me! will go ahead and rebase my branch locally...

@AparnaKarve are we OK to merge this now or do we need to resolve the filter issue above on the backend first?

vconzola · 2018-06-06T12:32:26Z

@serenamarie125 I wasn't listed as a reviewer, but it looks good to me too.

AparnaKarve · 2018-06-06T19:32:29Z

app/javascript/react/screens/App/Overview/components/Migrations/MigrationsCompletedList.js

-                'There are no existing migration plans in a Completed state.'
+              {sprintf(
+                __('There are no existing migration plans in %s state.'),
+                archived ? 'an Archived' : 'a Completed'
              )}


The archived conditional needs to decide what message to display...

So if archived is true, then

sprintf(__('There are no existing migration plans in an Archived state.'))

and likewise the other message for archived = false

The only reason why I'm suggesting this change is because not all languages work like English.
For e.g. in a different language, the sentence construction rules or the grammar may not be the same as English and the %s placeholder may not work grammatically.

Also there is no gettext for an Archived and a Completed, and we need that.

Updated :)

I squashed my commit to help those working on the G backport

Cool! :)
That looks good!

* Add archive button to Completed Migrations ListView * Set up fetching so that archived and active (unarchived) plans are returned separately. * Set up POST to archive a plan * Modify MigrationsCompletedList to also be used for display of archived plans ADDITIONAL CHANGES * Refactor to remove usage of Object.values() due to node support * Extract migrations filters strings to constants

AparnaKarve · 2018-06-06T19:37:52Z

Other than the above comment, everything else looks just great!!

Thanks @michaelkro !!

himdel · 2018-11-16T15:46:46Z

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

+  fetchArchivedTransformationPlansUrl: PropTypes.string,
+  archivedTransformationPlans: PropTypes.array,
+  allArchivedPlanRequestsWithTasks: PropTypes.array,
+  isFetchingArchivedTransformationPlans: PropTypes.string,


Was the string intentional here?

I'm seeing...

miq_debug.self-1351f771c2cd2fab0d83069fd3f62aadbb7effc11a01ed4942f52308793453be.js?body=1:30 Warning: Failed prop type: Invalid prop `loading` of type `string` supplied to `Spinner`, expected `boolean`. in Spinner (created by Overview) in Overview (created by Connect(Overview))

and given the condition...

<Spinner loading={ !requestsWithTasksPreviouslyFetched && !this.hasMadeInitialPlansFetch && (isFetchingAllRequestsWithTasks || isFetchingProviders || isFetchingTransformationPlans || isFetchingArchivedTransformationPlans || isFetchingTransformationMappings) }

When isFetchingArchivedTransformationPlans is the first truthy value, it gets passed to loading.

Not sure if you want an explicit !! isFetchingArchivedTransformationPlans to convert to bool, or if this should never have been a string.

Can you take a look please @michaelkro ?

Hi @himdel, I've been noticing that error as well but just haven't had the time to fully investigate why that particular value is a string.

In some cases, we have used IDs as flags to isolate loading state, e.g., disable the migrate button for a specific migration plan ListItem after it is clicked.

However, that being said, I will take a look and at the very least put up a PR coercing it to a bool, since that is really all we need in this case.

Thanks for taking the time to report the issue!

michaelkro force-pushed the archive-plan-button branch 3 times, most recently from 06dec14 to 0173cb5 Compare June 1, 2018 11:58

priley86 reviewed Jun 1, 2018

View reviewed changes

priley86 mentioned this pull request Jun 1, 2018

[WIP] i18n fixes and cleanups #371

Closed

michaelkro force-pushed the archive-plan-button branch 2 times, most recently from c544975 to a935941 Compare June 1, 2018 19:54

priley86 mentioned this pull request Jun 1, 2018

Fix routes #381

Merged

michaelkro force-pushed the archive-plan-button branch from bd24d29 to a935941 Compare June 1, 2018 20:20

priley86 mentioned this pull request Jun 1, 2018

remove route text attribute #382

Closed

priley86 mentioned this pull request Jun 1, 2018

remove route text attribute #383

Closed

michaelkro force-pushed the archive-plan-button branch 6 times, most recently from 6172381 to 93060c9 Compare June 5, 2018 01:01

michaelkro changed the title ~~[WIP] Archive Plans~~ Archive Plans Jun 5, 2018

michaelkro mentioned this pull request Jun 5, 2018

User should be able to archive a Migration Plan #314

Closed

priley86 mentioned this pull request Jun 5, 2018

Add missing gettext #391

Merged

michaelkro force-pushed the archive-plan-button branch 3 times, most recently from d8d602a to cb5962b Compare June 5, 2018 19:23

michaelkro added p1 sprint9 labels Jun 5, 2018

michaelkro force-pushed the archive-plan-button branch 2 times, most recently from e5b1b17 to 2864998 Compare June 5, 2018 22:48

serenamarie125 approved these changes Jun 6, 2018

View reviewed changes

priley86 approved these changes Jun 6, 2018

View reviewed changes

priley86 requested a review from vconzola June 6, 2018 15:45

vconzola approved these changes Jun 6, 2018

View reviewed changes

AparnaKarve reviewed Jun 6, 2018

View reviewed changes

michaelkro force-pushed the archive-plan-button branch from 2864998 to 37d3d34 Compare June 6, 2018 19:37

AparnaKarve approved these changes Jun 6, 2018

View reviewed changes

AparnaKarve merged commit 26dd8ee into ManageIQ:master Jun 6, 2018

This was referenced Jun 6, 2018

Drill into an Archived Migration Plan #316

Closed

View Archived Migration Plans from the main Migration page #315

Closed

michaelkro deleted the archive-plan-button branch August 6, 2018 11:26

himdel reviewed Nov 16, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Archive Plans #367

Archive Plans #367

michaelkro commented Jun 1, 2018 •

edited

Loading

priley86 Jun 1, 2018 •

edited

Loading

agrare Jun 1, 2018

AparnaKarve Jun 1, 2018

agrare Jun 1, 2018

AparnaKarve Jun 1, 2018

AparnaKarve Jun 4, 2018

priley86 Jun 1, 2018

priley86 Jun 1, 2018

priley86 Jun 1, 2018

priley86 Jun 1, 2018

michaelkro commented Jun 1, 2018

priley86 commented Jun 1, 2018 •

edited

Loading

michaelkro commented Jun 4, 2018

priley86 commented Jun 4, 2018

AparnaKarve commented Jun 4, 2018

priley86 commented Jun 5, 2018

michaelkro commented Jun 5, 2018

serenamarie125 left a comment

priley86 commented Jun 6, 2018

vconzola commented Jun 6, 2018

AparnaKarve Jun 6, 2018 •

edited

Loading

michaelkro Jun 6, 2018

AparnaKarve Jun 6, 2018

AparnaKarve commented Jun 6, 2018

himdel Nov 16, 2018

michaelkro Nov 16, 2018

Archive Plans #367

Archive Plans #367

Conversation

michaelkro commented Jun 1, 2018 • edited Loading

Notes

Screens

Flow

Archive Button

Archive Modal (Plan with unmigrated VMs)

Archive List

Archive Details

Archive Empty

priley86 Jun 1, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelkro commented Jun 1, 2018

priley86 commented Jun 1, 2018 • edited Loading

michaelkro commented Jun 4, 2018

priley86 commented Jun 4, 2018

AparnaKarve commented Jun 4, 2018

priley86 commented Jun 5, 2018

michaelkro commented Jun 5, 2018

serenamarie125 left a comment

Choose a reason for hiding this comment

priley86 commented Jun 6, 2018

vconzola commented Jun 6, 2018

AparnaKarve Jun 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AparnaKarve commented Jun 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelkro commented Jun 1, 2018 •

edited

Loading

priley86 Jun 1, 2018 •

edited

Loading

priley86 commented Jun 1, 2018 •

edited

Loading

AparnaKarve Jun 6, 2018 •

edited

Loading