[#369] Inactive migration plans can be scheduled to run #401

AllenBW · 2018-06-08T17:41:14Z

fixes #369

Where we're at...

this one's dedicated to @bdunne @gtanzillo @abellotti

https://bugzilla.redhat.com/show_bug.cgi?id=1564255

AllenBW · 2018-06-13T18:07:34Z

@vconzola any chance yah have the rest of the mockups for this available? (as mentioned here)

Other things we're waiting on

~~Theres a lil api work @bdunne is workin' on...~~ Allow ordering ServiceTemplates with a schedule_time manageiq-api#400
@serenamarie125 @vconzola any word on what we're doing for a date time picker here? or for now just use something, anything, even if separate to get this in
fix a few 🐛 cancel modal, delayed datepicker launch
profit

🌮 💃

EDIT - yeah, sorry, asking now cuz this has kinda reached the point of highest heights without ☝️

Another Edit - we just gotta get bdunne's pr in then this will be g2g

AparnaKarve · 2018-06-16T00:02:23Z

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

+                    <strong>{plan.name}</strong>
+                    {__(' targted to run on ')}
+                    {moment(plan.schedule_time).format('MMMM Do YYYY, h:mm a')}
+                    {__('?')}


I am aware the PR is still WIP, but here's the thing that we discussed in the i18n context about complete sentences.

The sentence should ideally look like this -
"Are you sure you want to unschedule plan %s targeted to run on %s?"

Check out https://github.com/ManageIQ/miq_v2v_ui_plugin/blob/9aa142a0b212750cb7affb286654e0c98b49c1e9/app/javascript/react/screens/App/Overview/components/DeleteInfrastructureMappingConfirmationModal/DeleteInfrastructureMappingConfirmationModal.js#L58 for styled, sprintf implementation.

yeah! thanks for the 👁 🙇 ❤️

priley86 · 2018-06-19T21:12:53Z

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

+                        'Are you sure you want to unschedule plan %s  targted to run on %s ?'
+                      ),
+                      plan.name,
+                      moment(plan.schedule_time).format('MMMM Do YYYY, h:mm a')


can this one use our formatDateTime helper? 🍬

this one yeah! that sounds like a gooooood call

priley86 · 2018-06-19T21:13:28Z

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

+                        {__(`Migration scheduled`)}
+                        <br />
+                        {moment(plan.schedule_time).format(
+                          'MMMM Do YYYY, h:mm a'


same deal w/ the helper

AllenBW · 2018-06-20T21:06:19Z

Just in case an inquiring mind was wondering... ux is done.... our path to completion is presently
👇 this pr has to be merged:
ManageIQ/manageiq#17594
so 👉 this pr can be merged: ManageIQ/manageiq-api#400

then whatever cleanup befalls after the fact...

profit 🌮 🥇

priley86 · 2018-07-11T21:29:43Z

If it's not much additional work, fine w/ me ;)

otherwise, lgtm

AllenBW · 2018-07-11T22:00:41Z

Yah, it'll be some work, can do it later if desired by QE 👍

AparnaKarve · 2018-07-11T22:49:41Z

@AllenBW The "Validation failed: Name has already been taken" issue described here #401 (comment) still persists. Is that expected?

EDIT:
Sorry, (ignore the above) -- not getting the above error now. My local manageiq-api repo was not rebased.

AparnaKarve · 2018-07-12T00:00:34Z

@AllenBW This looks really great... the delay however...

The delay is noticeable and big enough to get us in this situation -
An enabled Schedule button when the Plan is already scheduled.

Clicking on the button causes the 500 error Validation failed: Name has already been taken"

Would it be possible to gray out the Schedule and the Unschedule button in the listview until the Plan is actually scheduled/unscheduled?
Also delay the notification until the Plan is actually Scheduled/Unscheduled

AllenBW · 2018-07-12T12:22:01Z

@AparnaKarve the notification is thrown as soon as the callback is resolved, as in when the plan is actually Scheduled/unscheduled. The delay is because we poll the page... If the behavior blocks the merging of this pr, I can work on this nowish 🤷‍♀️

In the defense of leaving it as is for the moment, seems that scheduling while a schedule is already in place, doesn't disrupt the existing schedule, just throws the, "hey there's already something scheduled error" at which point the schedule can be removed and readded 🤔

AllenBW · 2018-07-12T12:58:42Z

So a few changes come to mind.... we can... (in order of effort, least to most to absolute least)

increase the polling frequency here (this is only polling transformation plans, might make the whole migration process feel a little better)
swap the "schedule migration" button visibility to an internally managed variable rather than the presence or absence of the a schedule run_time (but this will still appear laggy, meaning the schedule time won't show up till polling happens)
pass fetchTransformationPlansAction all the way down into the modal, and throw that on click (but again, this might resolve before the schedule is completed, so not a guaranteed fix)
somehow call fetchTransformationPlansAction from the scheduleMigration(action) callback? feels really wrong to be calling an action from an action though... unsure if this is kosher 🤔
give up and eat 🥑 🍿 by the 🌆 while wearing 🕴

michaelkro · 2018-07-12T13:21:19Z

somehow call fetchTransformationPlansAction from the scheduleMigration(action) callback? feels really wrong to be calling an action from an action though... unsure if this is kosher 🤔

I believe this is indeed kosher :) One of the great things about redux-thunk is that it gives us total control over the dispatch process, and one of the things it allows us to do is dispatch multiple actions from a single action creator (documentation)

edit 1: In the documentation, scroll down to function makeASandwichWithSecretSauce
edit 2: Here is an example of where we dispatch multiple actions in V2V

So, if you decided to go down this road, I will not complain :)

priley86 · 2018-07-12T13:42:34Z

yes - of the options you list, it seems like 3 or 4 (calling fetchTransformationPlansAction again explicitly) should work. You can import actions (and reuse them) from within a nested view like this.

I should add that I don't think option 1 is really an option - b/c I have heard previously that the backend automate code is only refreshing every 15 seconds, currently...so polling more frequently on the frontend seems like a bad route.

AllenBW · 2018-07-12T13:45:06Z

@michaelkro Thanks for the input! I've been chewing on what ya posted, still very unclear on how this approach might be successfully wired up. If you have some spare cycles would appreciate a pair on this :-/

bdunne · 2018-07-12T13:53:56Z

@AllenBW @AparnaKarve This should fix the "Name has already been taken" validation error. ManageIQ/manageiq#17696

AllenBW · 2018-07-12T14:23:37Z

@AparnaKarve, @michaelkro put up with me for 30 minutes and we have da solution, MORE DUPLICATE CODE EVERYWHERE aka, when you schedule or unschedule we refetch plans 😋 👍

i picked option 5, he picked option 3

AllenBW · 2018-07-12T14:27:52Z

@priley86 but @AparnaKarve is out today/tomorrow, can you confirm the desired behavior/merge? 🙇‍♀️

priley86 · 2018-07-12T14:30:27Z

@AllenBW sure thing... will test it out now.

priley86 · 2018-07-12T14:44:09Z

yes - calling fetchTransformationPlansAction after scheduleMigration seems to have the desired effect... approving/merging to get this out for QE.

I am going to follow up w/ some schema validation checks later on... (always just a bonus).

AparnaKarve · 2018-07-31T03:02:44Z

@simaishi We will need this backported first before #491.
Thanks.

simaishi · 2018-07-31T21:57:47Z

and it looks like ManageIQ/manageiq-api#400 needs to be backported before this PR... btw, it will really help if all dependencies are listed in the top description, as I could easily miss if it's in comment somewhere.

[#369] Inactive migration plans can be scheduled to run (cherry picked from commit 18a306f) https://bugzilla.redhat.com/show_bug.cgi?id=1608351

simaishi · 2018-07-31T22:38:34Z

Gaprindashvili backport details:

$ git log -1
commit 4032ca240376bd32d0673ce248459fc52e906f75
Author: Patrick Riley <priley86@gmail.com>
Date:   Thu Jul 12 10:44:34 2018 -0400

    Merge pull request #401 from AllenBW/enhancement/#369-schedule-migration
    
    [#369] Inactive migration plans can be scheduled to run
    (cherry picked from commit 18a306ff7a4767ca80d8a72b649e545a8afec683)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1608351

AllenBW added enhancement p1 sprint9 in progress labels Jun 8, 2018

AllenBW force-pushed the enhancement/#369-schedule-migration branch 6 times, most recently from 55dc8a2 to d39f843 Compare June 13, 2018 18:01

AllenBW force-pushed the enhancement/#369-schedule-migration branch 7 times, most recently from 2a6a83b to 54f6bce Compare June 15, 2018 20:06

AparnaKarve reviewed Jun 16, 2018

View reviewed changes

AllenBW force-pushed the enhancement/#369-schedule-migration branch 5 times, most recently from 1f439db to 0afbd49 Compare June 19, 2018 17:41

AllenBW mentioned this pull request Jun 19, 2018

Scheduling catalog items ManageIQ/manageiq#17594

Merged

priley86 reviewed Jun 19, 2018

View reviewed changes

AllenBW force-pushed the enhancement/#369-schedule-migration branch from 0afbd49 to 8a47d69 Compare June 20, 2018 12:25

AllenBW changed the title ~~[WIP] Inactive migration plans can be scheduled to run~~ [WIP] #369 -Inactive migration plans can be scheduled to run Jun 21, 2018

Inactive migration plans can be scheduled to run

624c379

AllenBW force-pushed the enhancement/#369-schedule-migration branch from eb1d285 to 624c379 Compare July 12, 2018 14:22

priley86 approved these changes Jul 12, 2018

View reviewed changes

priley86 merged commit 18a306f into ManageIQ:master Jul 12, 2018

AllenBW deleted the enhancement/#369-schedule-migration branch July 12, 2018 14:48

priley86 mentioned this pull request Jul 13, 2018

add schedules schema validation to overview #488

Merged

AparnaKarve added the blocker label Jul 31, 2018

AparnaKarve assigned simaishi Jul 31, 2018

AparnaKarve mentioned this pull request Jul 31, 2018

[#399] Pre/Post Playbook Overview UI + API #491

Merged

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Jul 31, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#369] Inactive migration plans can be scheduled to run #401

[#369] Inactive migration plans can be scheduled to run #401

AllenBW commented Jun 8, 2018 •

edited by AparnaKarve

Loading

AllenBW commented Jun 13, 2018 •

edited

Loading

AparnaKarve Jun 16, 2018

AparnaKarve Jun 16, 2018

AllenBW Jun 18, 2018

priley86 Jun 19, 2018

AllenBW Jun 20, 2018

priley86 Jun 19, 2018

AllenBW Jun 20, 2018

AllenBW commented Jun 20, 2018

priley86 commented Jul 11, 2018

AllenBW commented Jul 11, 2018

AparnaKarve commented Jul 11, 2018 •

edited

Loading

AparnaKarve commented Jul 12, 2018 •

edited

Loading

AllenBW commented Jul 12, 2018 •

edited

Loading

AllenBW commented Jul 12, 2018

michaelkro commented Jul 12, 2018 •

edited

Loading

priley86 commented Jul 12, 2018 •

edited

Loading

AllenBW commented Jul 12, 2018

bdunne commented Jul 12, 2018

AllenBW commented Jul 12, 2018 •

edited

Loading

AllenBW commented Jul 12, 2018

priley86 commented Jul 12, 2018

priley86 commented Jul 12, 2018

AparnaKarve commented Jul 31, 2018

simaishi commented Jul 31, 2018

simaishi commented Jul 31, 2018

[#369] Inactive migration plans can be scheduled to run #401

[#369] Inactive migration plans can be scheduled to run #401

Conversation

AllenBW commented Jun 8, 2018 • edited by AparnaKarve Loading

Where we're at...

AllenBW commented Jun 13, 2018 • edited Loading

Other things we're waiting on

AparnaKarve Jun 16, 2018

Choose a reason for hiding this comment

AparnaKarve Jun 16, 2018

Choose a reason for hiding this comment

AllenBW Jun 18, 2018

Choose a reason for hiding this comment

priley86 Jun 19, 2018

Choose a reason for hiding this comment

AllenBW Jun 20, 2018

Choose a reason for hiding this comment

priley86 Jun 19, 2018

Choose a reason for hiding this comment

AllenBW Jun 20, 2018

Choose a reason for hiding this comment

AllenBW commented Jun 20, 2018

priley86 commented Jul 11, 2018

AllenBW commented Jul 11, 2018

AparnaKarve commented Jul 11, 2018 • edited Loading

AparnaKarve commented Jul 12, 2018 • edited Loading

AllenBW commented Jul 12, 2018 • edited Loading

AllenBW commented Jul 12, 2018

michaelkro commented Jul 12, 2018 • edited Loading

priley86 commented Jul 12, 2018 • edited Loading

AllenBW commented Jul 12, 2018

bdunne commented Jul 12, 2018

AllenBW commented Jul 12, 2018 • edited Loading

AllenBW commented Jul 12, 2018

priley86 commented Jul 12, 2018

priley86 commented Jul 12, 2018

AparnaKarve commented Jul 31, 2018

simaishi commented Jul 31, 2018

simaishi commented Jul 31, 2018

AllenBW commented Jun 8, 2018 •

edited by AparnaKarve

Loading

AllenBW commented Jun 13, 2018 •

edited

Loading

AparnaKarve commented Jul 11, 2018 •

edited

Loading

AparnaKarve commented Jul 12, 2018 •

edited

Loading

AllenBW commented Jul 12, 2018 •

edited

Loading

michaelkro commented Jul 12, 2018 •

edited

Loading

priley86 commented Jul 12, 2018 •

edited

Loading

AllenBW commented Jul 12, 2018 •

edited

Loading