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

Power operations in Service Detail view #330

Merged

Conversation

@AparnaKarve
Copy link
Contributor

AparnaKarve commented Nov 10, 2016

#271 added the Service Power Operation feature in the Services List view.

This PR adds the same feature in the Service Detail view.

  • A new Service PowerOperations was created that would be injected in both the Service list controller as well as the Service detail controller.
  • Buttons in the top right corner were rearranged to avoid making the page look overly busy (with too many buttons).
  • Also added a Power State field to indicate the current Power status

Screenshots

Before:
screen shot 2016-11-10 at 11 02 30 am

After:
screen shot 2016-11-10 at 11 00 28 am

screen shot 2016-11-10 at 10 57 08 am

screen shot 2016-11-10 at 10 59 20 am

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

@AparnaKarve

This comment has been minimized.

Copy link
Contributor Author

AparnaKarve commented Nov 10, 2016

@AparnaKarve AparnaKarve changed the title Power ops in service detail Power operations in Service Detail view Nov 10, 2016

@chriskacerguis chriskacerguis self-assigned this Nov 11, 2016

@chriskacerguis

This comment has been minimized.

Copy link
Contributor

chriskacerguis commented Nov 11, 2016

@AparnaKarve is this for Euwe?

@AparnaKarve

This comment has been minimized.

Copy link
Contributor Author

AparnaKarve commented Nov 11, 2016

@chriskacerguis I think so... because #271 which is basically the same feature has already been backported to Euwe. So why not this, right?

@chriskacerguis

This comment has been minimized.

Copy link
Contributor

chriskacerguis commented Nov 11, 2016

@AparnaKarve can you add unit tests to this?

@miq-bot

This comment has been minimized.

Copy link
Member

miq-bot commented Nov 14, 2016

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@AparnaKarve AparnaKarve force-pushed the AparnaKarve:power_ops_in_service_detail branch from 26f13c6 to acda702 Nov 14, 2016

@miq-bot

This comment has been minimized.

Copy link
Member

miq-bot commented Nov 14, 2016

Checked commits AparnaKarve/manageiq-ui-self_service@db9d51e~...acda702 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
0 files checked, 0 offenses detected
Everything looks good. 🏆

@AparnaKarve

This comment has been minimized.

Copy link
Contributor Author

AparnaKarve commented Nov 14, 2016

@chriskacerguis I've updated some tests for Service List view in 31ccd70 and added new ones for Service Detail in acda702

@AllenBW
Copy link
Member

AllenBW left a comment

Overall looks good!

@@ -116,6 +130,21 @@
<input class="form-control" disabled value="${{ ::vm.service.chargeback.used_cost_sum | number:3 }}"/>
</div>
</div>
<div class="form-group">

This comment has been minimized.

Copy link
@AllenBW

AllenBW Nov 15, 2016

Member

EEEEEK 🕷 🕸
Oh wait, its inline style, still just as terrifying, this should be refactored to live in the sass, ping me if you need an 👂

This comment has been minimized.

Copy link
@AparnaKarve

AparnaKarve Nov 15, 2016

Author Contributor

@AllenBW You'll notice the <div class="form-group"> usage in other places too (minus this PR) and I think could be addressed in a separate PR.
What do you think?

This comment has been minimized.

Copy link
@AllenBW

AllenBW Nov 15, 2016

Member

@AparnaKarve OH apologies, was referring to everything below, the style stuff included in the html

@chriskacerguis chriskacerguis merged commit 24c9fdb into ManageIQ:master Nov 15, 2016

3 checks passed

codeclimate Code Climate didn't find any new or fixed issues.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound No violations found. Woof!
@chriskacerguis

This comment has been minimized.

Copy link
Contributor

chriskacerguis commented Dec 1, 2016

Adding Blocker label. Needed per this bug: https://bugzilla.redhat.com/show_bug.cgi?id=1391685

@chriskacerguis

This comment has been minimized.

Copy link
Contributor

chriskacerguis commented Dec 1, 2016

@chessbyte

This comment has been minimized.

Copy link
Member

chessbyte commented Dec 2, 2016

Euwe Backport conflict:

$ git cherry-pick -e -x -m 1  24c9fdb  
error: could not apply 24c9fdb... Merge pull request #330 from AparnaKarve/power_ops_in_service_detail
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

$ git status
On branch euwe
Your branch is up-to-date with 'upstream/euwe'.
You are currently cherry-picking commit 24c9fdb.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:

	new file:   client/app/services/poweroperations.service.js
	modified:   client/app/states/services/details/details.state.js
	new file:   tests/services-details.state.spec.js

Unmerged paths:
  (use "git add <file>..." to mark resolution)

	both modified:   client/app/states/services/details/details.html
	both modified:   client/app/states/services/list/list.state.js
	both modified:   tests/services-list.state.spec.js

$ git diff
diff --cc client/app/states/services/details/details.html
index bfa0792,2b64004..0000000
--- a/client/app/states/services/details/details.html
+++ b/client/app/states/services/details/details.html
@@@ -44,7 -45,11 +45,15 @@@
                      confirmation-show-cancel="true" translate>Remove Service
                </button>
                <div class="btn-group" dropdown>
++<<<<<<< HEAD
 +                <button id="single-button" type="button" class="btn btn-default" dropdown-toggle tooltip="{{'Inactivate the service'|translate}}" tooltip-placement="bottom" ng-show="vm.showRetireService">
++=======
+                 <button id="single-button" type="button" class="btn btn-default"
+                   ng-disabled="vm.powerOperationInProgressState(vm.service)"
+                   dropdown-toggle tooltip="{{'Inactivate the service'|translate}}"
+                   tooltip-placement="bottom"
+                   ng-show="vm.showRetireService || vm.showScheduleRetirementService">
++>>>>>>> 24c9fdb... Merge pull request #330 from AparnaKarve/power_ops_in_service_detail
                    {{'Retire'|translate}} <span class="caret"></span>
                  </button>
                  <ul class="dropdown-menu" role="menu">
diff --cc client/app/states/services/list/list.state.js
index 5156140,b2bcb39..0000000
--- a/client/app/states/services/list/list.state.js
+++ b/client/app/states/services/list/list.state.js
@@@ -218,18 -154,21 +231,30 @@@
        filterChange(ServicesState.getFilters());
        ServicesState.filterApplied = false;
      } else {
 -      vm.servicesList = ListView.applyFilters(ServicesState.getFilters(), vm.servicesList, vm.services, ServicesState, matchesFilter);
 -
 -      /* Make sure sorting direction is maintained */
 -      sortChange(ServicesState.getSort().currentField, ServicesState.getSort().isAscending);
 +      applyFilters();
      }
  
++<<<<<<< HEAD
 +    vm.enableButtonForItemFn = function(action, item) {
 +      return powerOperationUnknownState(item)
 +        || powerOperationOffState(item)
 +        || powerOperationSuspendState(item)
 +        || powerOperationTimeoutState(item);
 +    };
 +
 +    vm.hideMenuForItemFn = function(item) {
 +      return powerOperationUnknownState(item) || powerOperationInProgressState(item);
++=======
+     vm.enableButtonForItemFn = function (action, item) {
+       return vm.powerOperationUnknownState(item)
+         || vm.powerOperationOffState(item)
+         || vm.powerOperationSuspendState(item)
+         || vm.powerOperationTimeoutState(item);
+     };
+ 
+     vm.hideMenuForItemFn = function (item) {
+       return vm.powerOperationUnknownState(item) || vm.powerOperationInProgressState(item);
++>>>>>>> 24c9fdb... Merge pull request #330 from AparnaKarve/power_ops_in_service_detail
      };
  
      vm.updateMenuActionForItemFn = function(action, item) {
diff --cc tests/services-list.state.spec.js
index 3e4453e,378ee9d..0000000
--- a/tests/services-list.state.spec.js
+++ b/tests/services-list.state.spec.js
@@@ -123,8 -142,10 +146,15 @@@ describe('Dashboard', function() 
      beforeEach(function() {
        bard.inject('$controller', '$log', '$state', '$rootScope');
  
++<<<<<<< HEAD
 +      controller = $controller($state.get('services.list').controller, {services: services, Chargeback: Chargeback});
 +      $rootScope.$apply();
++=======
+       controller = $controller($state.get('services.list').controller,
+         {services: services,
+          Chargeback: Chargeback,
+          PowerOperations: PowerOperations});
++>>>>>>> 24c9fdb... Merge pull request #330 from AparnaKarve/power_ops_in_service_detail
      });
  
      it('sets the powerState value on the Service', function() {
@chessbyte

This comment has been minimized.

Copy link
Member

chessbyte commented Dec 2, 2016

@chriskacerguis @AparnaKarve Please resolve conflict and create Euwe-specific PR (referencing this one) or propose other PRs to backport to Euwe.

@AparnaKarve

This comment has been minimized.

Copy link
Contributor Author

AparnaKarve commented Dec 2, 2016

@chessbyte @chriskacerguis Euwe-specific PR for this is #371

@AparnaKarve AparnaKarve deleted the AparnaKarve:power_ops_in_service_detail branch Dec 5, 2016

@chessbyte

This comment has been minimized.

Copy link
Member

chessbyte commented Dec 6, 2016

Backported to Euwe via #371

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.