Created Lifecycle menu #912

chalettu · 2017-09-06T17:12:57Z

@miq-bot add_label enhancement
@miq-bot add_label fine/no
@miq-bot add_label ux/review
@miq-bot add_label my services / details

Issue: #936

chalettu · 2017-09-06T17:19:54Z

Screenshot of implementation

chriskacerguis · 2017-09-06T18:07:07Z

@Loicavenel Per your request, here is the change. Please let us know if that is what you had in mind.

/cc @serenamarie125 for UX review.

chriskacerguis · 2017-09-06T18:08:46Z

client/app/services/vms.service.spec.js


      beforeEach(module('app.services','app.shared'));
      beforeEach( () => {
-          bard.inject('VmsService', 'CollectionsApi');
+          bard.inject('VmsService', 'CollectionsApi', 'RBAC');


Hmmm...are we checking the test files for StandardJS formatting? @AllenBW did we put that in to test in Travis? The ; is not allowed by StandardJS

We aren't checking test files @chriskacerguis

he aint lying, we weren't before, wasn't going to add it in the conversion (https://github.com/ManageIQ/manageiq-ui-service/blob/master/package.json#L199-L200)

AllenBW · 2017-09-06T18:31:26Z

For posterity, the pt https://www.pivotaltracker.com/story/show/149777601

AllenBW

@chriskacerguis @Loicavenel Wondering why we are prioritizing this effort while the whole page is being rewritten here why not just update the mock to include the change (which btw it doesn't presently) ?

AllenBW · 2017-09-06T18:36:33Z

client/app/services/vms.service.js

@@ -130,7 +134,8 @@ export function VmsService (CollectionsApi, RBAC) {
      create: RBAC.hasAny(['vm_snapshot_add']),
      delete: RBAC.hasAny(['vm_snapshot_delete']),
      deleteAll: RBAC.hasAny(['vm_snapshot_delete_all']),
-      revert: RBAC.hasAny(['vm_snapshot_revert'])
+      revert: RBAC.hasAny(['vm_snapshot_revert']),
+      instanceRetire: RBAC.hasAny(['instance_control', 'instance_retire'])


I was (am) guilty of this, making up names for rbac permissions...

It would be best to keep the name as close to the actualproduct feature as possible, so for this line instance_control: RBAC.hasAny(['instance_control')]is preferred

serenamarie125 · 2017-09-06T18:40:25Z

Unsure where this requirement is from. Shouldn't all retirement options be available here @Loicavenel

serenamarie125 · 2017-09-06T18:47:00Z

The addition of retirement here is great ! But here are my questions/ concerns:
1 - this option should only be available for VMs
2 - the menu itself shouldn't reference "resources"
3 - can we also add options to change the retirement date? I'm currently at the airport and don't have time to get onto the Ops UI to see the options, but i'm guessing that we should be able to sy either Retire now or Change retirement date
@Loicavenel curious on your thoughts
@chalettu i can provide better wording tonight/tomorrow if needed

Loicavenel · 2017-09-06T19:24:36Z

@serenamarie125 I think we should not offer "change the retirement date" as this is a service.

chriskacerguis · 2017-09-11T13:19:22Z

@chalettu can you please rebase, so NSP goes green?

chriskacerguis · 2017-09-12T17:58:21Z

@chalettu can you please rebase, so NSP goes green?

chriskacerguis · 2017-09-18T14:13:51Z

@chalettu ^

chriskacerguis · 2017-09-18T14:22:48Z

@serenamarie125 thoughts on @Loicavenel 's thoughts?

chriskacerguis · 2017-09-19T14:01:08Z

@serenamarie125 thoughts on @Loicavenel 's thoughts?

chalettu · 2017-09-19T15:51:59Z

In discussing this feature with @Loicavenel we had concluded that only offering the option to retire the VM now should be the only available option at the VM level. @serenamarie125 I will change the wording in the popup to say VM instead of Resource. Will that work for updating the verbiage appropriately?

serenamarie125 · 2017-09-19T17:02:12Z

@chriskacerguis, loic's comments were on the ability to change the retirement value. I'm fine with that.

serenamarie125 · 2017-09-19T17:20:41Z

@chalettu As for the dialog, can you change:

Dialog title to "Retire" (EVEN better if you can say "Retire - VM name")
Action button to "Retire"
Descriptive text to say "Are you sure you want to retire this service now?" (EVEN better if you can say "Are you sure you want to retire VM name now?")

chalettu · 2017-09-21T21:09:04Z

@serenamarie125 this dialog pictured is not for a Service, its for a VM that is part of a service. I can go head and change the verbiage to add the term Retire but for all those places you mentioned using servicename, they don't fit this dialog as it is on the VM level. I will attach a screenshot of the updated look.

serenamarie125 · 2017-09-21T21:18:19Z

@chalettu my bad, i edited my comments with VM name rather than Resource. Thank you!

chalettu · 2017-09-22T16:32:38Z

@serenamarie125 , here is a copy of the updated modal. Please let me know if we need to make any adjustments or if we can approve this PR .

AllenBW

Gonna apppprooooovvveeeeee but @chalettu @chriskacerguis where do we stand on standard in .spec.js ? imma thinking it should be turned on for consistency... (in another pr of course)

chriskacerguis · 2017-09-25T22:11:14Z

@AllenBW 👍 yeah, we should turn it on for the .spec.js

serenamarie125 · 2017-09-26T02:22:43Z

Hey @chalettu, are you able to include the name of the VM rather than saying "this VM?"

miq-bot · 2017-09-27T20:27:54Z

Checked commits https://github.com/chalettu/manageiq-ui-service/compare/ba7e0f3d5bc2f99f11e30c191946579e3ed999cc~...ba369d818bb8d2b8dc2c27d561520e6420cfacdf with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. 🏆

chalettu · 2017-09-27T20:38:53Z

@serenamarie125 I was able to add the VM name to modal. Please let me know if you think it's good to go.

serenamarie125 · 2017-09-28T01:18:34Z

@miq-bot add_label ux/approved

serenamarie125 · 2017-09-28T01:18:47Z

@miq-bot remove_label ux/review

serenamarie125

LGTM @chalettu . Thanks!

chriskacerguis · 2017-09-28T01:30:35Z

@Loicavenel could you please add your approval if you are good with this?

AllenBW

🤙 🇲🇰

chalettu requested review from AllenBW and chriskacerguis as code owners September 6, 2017 17:12

miq-bot added enhancement fine/no ux/review labels Sep 6, 2017

chriskacerguis self-assigned this Sep 6, 2017

chriskacerguis added this to the Sprint 69 Ending Sep 18, 2017 milestone Sep 6, 2017

chriskacerguis reviewed Sep 6, 2017

View reviewed changes

AllenBW previously approved these changes Sep 6, 2017

View reviewed changes

chriskacerguis removed the my services / details label Sep 11, 2017

chalettu dismissed AllenBW’s stale review via 37ddcfe September 14, 2017 15:37

chriskacerguis removed this from the Sprint 69 Ending Sep 18, 2017 milestone Sep 25, 2017

chriskacerguis previously approved these changes Sep 25, 2017

View reviewed changes

AllenBW previously approved these changes Sep 25, 2017

View reviewed changes

chriskacerguis mentioned this pull request Sep 25, 2017

Sprint 70 Plan #1004

Closed

6 tasks

chriskacerguis changed the title ~~[Finishes #149777601] Created Lifecycle menu~~ Created Lifecycle menu Sep 26, 2017

chalettu added 3 commits September 27, 2017 14:24

[Finishes #149777601] Created Lifecycle menu

ba7e0f3

Updated retire modal text

fee123e

Added VM Name to Retire modal

ba369d8

chalettu dismissed stale reviews from AllenBW and chriskacerguis via ba369d8 September 27, 2017 20:27

chriskacerguis requested review from serenamarie125 and Loicavenel September 27, 2017 22:47

serenamarie125 approved these changes Sep 28, 2017

View reviewed changes

miq-bot added ux/approved and removed ux/review labels Sep 28, 2017

chriskacerguis approved these changes Sep 28, 2017

View reviewed changes

AllenBW approved these changes Sep 28, 2017

View reviewed changes

chriskacerguis removed the request for review from Loicavenel September 28, 2017 12:15

chriskacerguis added this to the Sprint 70 Ending Oct 2, 2017 milestone Sep 28, 2017

chriskacerguis merged commit 80724bb into ManageIQ:master Sep 28, 2017

chalettu deleted the retire-vm-details branch January 10, 2018 20:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Created Lifecycle menu #912

Created Lifecycle menu #912

chalettu commented Sep 6, 2017 •

edited by chriskacerguis

Loading

chalettu commented Sep 6, 2017

chriskacerguis commented Sep 6, 2017

chriskacerguis Sep 6, 2017

chalettu Sep 6, 2017

AllenBW Sep 6, 2017

AllenBW commented Sep 6, 2017

AllenBW left a comment

AllenBW Sep 6, 2017

serenamarie125 commented Sep 6, 2017

serenamarie125 commented Sep 6, 2017

Loicavenel commented Sep 6, 2017

chriskacerguis commented Sep 11, 2017

chriskacerguis commented Sep 12, 2017

chriskacerguis commented Sep 18, 2017

chriskacerguis commented Sep 18, 2017

chriskacerguis commented Sep 19, 2017

chalettu commented Sep 19, 2017

serenamarie125 commented Sep 19, 2017

serenamarie125 commented Sep 19, 2017 •

edited

Loading

chalettu commented Sep 21, 2017

serenamarie125 commented Sep 21, 2017

chalettu commented Sep 22, 2017

AllenBW left a comment

chriskacerguis commented Sep 25, 2017

serenamarie125 commented Sep 26, 2017

miq-bot commented Sep 27, 2017

chalettu commented Sep 27, 2017

serenamarie125 commented Sep 28, 2017

serenamarie125 commented Sep 28, 2017

serenamarie125 left a comment

chriskacerguis commented Sep 28, 2017

AllenBW left a comment

Created Lifecycle menu #912

Created Lifecycle menu #912

Conversation

chalettu commented Sep 6, 2017 • edited by chriskacerguis Loading

chalettu commented Sep 6, 2017

chriskacerguis commented Sep 6, 2017

chriskacerguis Sep 6, 2017

Choose a reason for hiding this comment

chalettu Sep 6, 2017

Choose a reason for hiding this comment

AllenBW Sep 6, 2017

Choose a reason for hiding this comment

AllenBW commented Sep 6, 2017

AllenBW left a comment

Choose a reason for hiding this comment

AllenBW Sep 6, 2017

Choose a reason for hiding this comment

serenamarie125 commented Sep 6, 2017

serenamarie125 commented Sep 6, 2017

Loicavenel commented Sep 6, 2017

chriskacerguis commented Sep 11, 2017

chriskacerguis commented Sep 12, 2017

chriskacerguis commented Sep 18, 2017

chriskacerguis commented Sep 18, 2017

chriskacerguis commented Sep 19, 2017

chalettu commented Sep 19, 2017

serenamarie125 commented Sep 19, 2017

serenamarie125 commented Sep 19, 2017 • edited Loading

chalettu commented Sep 21, 2017

serenamarie125 commented Sep 21, 2017

chalettu commented Sep 22, 2017

AllenBW left a comment

Choose a reason for hiding this comment

chriskacerguis commented Sep 25, 2017

serenamarie125 commented Sep 26, 2017

miq-bot commented Sep 27, 2017

chalettu commented Sep 27, 2017

serenamarie125 commented Sep 28, 2017

serenamarie125 commented Sep 28, 2017

serenamarie125 left a comment

Choose a reason for hiding this comment

chriskacerguis commented Sep 28, 2017

AllenBW left a comment

Choose a reason for hiding this comment

chalettu commented Sep 6, 2017 •

edited by chriskacerguis

Loading

serenamarie125 commented Sep 19, 2017 •

edited

Loading