MAAS

Merge ~mpontillo/maas:default-domain-ui--bug-1756745 into maas:master

  1. Git
  2. lp:~mpontillo/maas
  3. default-domain-ui--bug-1756745
  4. Merge into master
Proposed by Mike Pontillo
Status: Merged
Approved by: Mike Pontillo
Approved revision: 55574626b4b4c830577f61335ac79e0d16183730
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~mpontillo/maas:default-domain-ui--bug-1756745
Merge into: maas:master
Diff against target: 334 lines (+173/-14)
7 files modified
src/maasserver/static/js/angular/controllers/domains_list.js (+14/-0)
src/maasserver/static/js/angular/controllers/tests/test_domains_list.js (+41/-0)
src/maasserver/static/js/angular/factories/domains.js (+15/-2)
src/maasserver/static/js/angular/factories/tests/test_domains.js (+29/-1)
src/maasserver/static/partials/domains-list.html (+41/-11)
src/maasserver/websockets/handlers/domain.py (+11/-0)
src/maasserver/websockets/handlers/tests/test_domain.py (+22/-0)
Reviewer Review Type Date Requested Status
Lee Trager (community) Approve
Kit Randel (community) Approve
Review via email: mp+355810@code.launchpad.net

Commit message

LP: #1756745 - Allow setting the default domain from the UI.

To post a comment you must log in.
Revision history for this message
Anthony Dillon (ya-bo-ng) wrote :

For medium and small screens simply replace `p-table--mobile-card` class on the table with `p-table`. The mobile card is not required for this table.

Also, seems like you should not be able to set the current default domain as default. Could you conditionally display the action menu?

Revision history for this message
Kit Randel (blr) wrote :

lgtm. As discussed we can add an item to our backlog to modernise the table css at some point.

review: Approve
Revision history for this message
Mike Pontillo (mpontillo) wrote :

Thanks for the help and feedback.

I've moved forward using the col-N classes, since I see regressions with regard to column alignment (and sometimes placement of the confirm message) in this table every time I try to use the new CSS-based method.

I've also hard-coded a min-height style for now, to make all the rows the same height (regardless of whether or not they have an actions menu)

~mpontillo/maas:default-domain-ui--bug-1756745 updated
5557462... by Mike Pontillo

Hide domain actions menu for non-admins.

Revision history for this message
Lee Trager (ltrager) wrote :

I focused on the websocket side of things but this looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/src/maasserver/static/js/angular/controllers/domains_list.js b/src/maasserver/static/js/angular/controllers/domains_list.js
index ee71c21..da2b3ee 100644
--- a/src/maasserver/static/js/angular/controllers/domains_list.js
+++ b/src/maasserver/static/js/angular/controllers/domains_list.js
@@ -22,6 +22,7 @@ angular.module('MAAS').controller('DomainsListController', [
22 $scope.predicate = "name";22 $scope.predicate = "name";
23 $scope.reverse = false;23 $scope.reverse = false;
24 $scope.loading = true;24 $scope.loading = true;
25 $scope.confirmSetDefaultRow = null;
2526
26 // This will hold the AddDomainController once it's initialized. The27 // This will hold the AddDomainController once it's initialized. The
27 // controller will set this variable as it's always a child of this28 // controller will set this variable as it's always a child of this
@@ -43,6 +44,19 @@ angular.module('MAAS').controller('DomainsListController', [
43 return UsersManager.isSuperUser();44 return UsersManager.isSuperUser();
44 };45 };
4546
47 $scope.confirmSetDefault = function(domain) {
48 $scope.confirmSetDefaultRow = domain;
49 };
50
51 $scope.cancelSetDefault = function() {
52 $scope.confirmSetDefaultRow = null;
53 };
54
55 $scope.setDefault = function(domain) {
56 DomainsManager.setDefault(domain);
57 $scope.confirmSetDefaultRow = null;
58 };
59
46 ManagerHelperService.loadManagers(60 ManagerHelperService.loadManagers(
47 $scope, [DomainsManager, UsersManager]).then(61 $scope, [DomainsManager, UsersManager]).then(
48 function() {62 function() {
diff --git a/src/maasserver/static/js/angular/controllers/tests/test_domains_list.js b/src/maasserver/static/js/angular/controllers/tests/test_domains_list.js
index 02576bd..86eb06f 100644
--- a/src/maasserver/static/js/angular/controllers/tests/test_domains_list.js
+++ b/src/maasserver/static/js/angular/controllers/tests/test_domains_list.js
@@ -101,6 +101,47 @@ describe("DomainsListController", function() {
101 });101 });
102 });102 });
103103
104 describe("confirmSetDefault", function() {
105
106 it("sets confirmSetDefaultRow to the specified row", function() {
107 var controller = makeController();
108 var obj = {
109 id: makeInteger(0, 100)
110 };
111 $scope.confirmSetDefault(obj);
112 expect($scope.confirmSetDefaultRow).toBe(obj);
113 });
114 });
115
116 describe("cancelSetDefault", function() {
117
118 it("sets confirmSetDefaultRow to the specified row", function() {
119 var controller = makeController();
120 var obj = {
121 id: makeInteger(0, 100)
122 };
123 $scope.confirmSetDefaultRow = obj;
124 $scope.cancelSetDefault();
125 expect($scope.confirmSetDefaultRow).toBe(null);
126 });
127 });
128
129 describe("setDefault", function() {
130
131 it("calls DomainsManager.setDefault and clears selection", function() {
132 var controller = makeController();
133 spyOn(DomainsManager, "setDefault");
134 var obj = {
135 id: makeInteger(0, 100)
136 };
137 $scope.confirmSetDefaultRow = obj;
138 $scope.setDefault(obj);
139 expect(DomainsManager.setDefault).toHaveBeenCalledWith(obj);
140 expect($scope.confirmSetDefaultRow).toBe(null);
141
142 });
143 });
144
104 setupController = function(domains) {145 setupController = function(domains) {
105 var defer = $q.defer();146 var defer = $q.defer();
106 var controller = makeController(defer);147 var controller = makeController(defer);
diff --git a/src/maasserver/static/js/angular/factories/domains.js b/src/maasserver/static/js/angular/factories/domains.js
index ebc1561..5a9537f 100644
--- a/src/maasserver/static/js/angular/factories/domains.js
+++ b/src/maasserver/static/js/angular/factories/domains.js
@@ -78,13 +78,25 @@ angular.module('MAAS').factory(
78 }78 }
79 };79 };
8080
81 // Set the specified domain as the default.
82 DomainsManager.prototype.setDefault = function(domain) {
83 var promise = RegionConnection.callMethod(
84 "domain.set_default", {'domain': domain.id});
85 promise.then(function() {
86 // Reload the domains, so that the new default will be
87 // reflected everywhere, and record counts will be properly
88 // updated.
89 self.reloadItems();
90 });
91 };
92
81 DomainsManager.prototype.getDefaultDomain = function() {93 DomainsManager.prototype.getDefaultDomain = function() {
82 if(this._items.length === 0) {94 if(this._items.length === 0) {
83 return null;95 return null;
84 } else {96 } else {
85 var i;97 var i;
86 for(i=0;i<this._items.length;i++) {98 for(i=0;i<this._items.length;i++) {
87 if(this._items[i].id === 0) {99 if(this._items[i].is_default === true) {
88 return this._items[i];100 return this._items[i];
89 }101 }
90 }102 }
@@ -104,5 +116,6 @@ angular.module('MAAS').factory(
104 return null;116 return null;
105 };117 };
106118
107 return new DomainsManager();119 var self = new DomainsManager();
120 return self;
108 }]);121 }]);
diff --git a/src/maasserver/static/js/angular/factories/tests/test_domains.js b/src/maasserver/static/js/angular/factories/tests/test_domains.js
index 0beb002..920c12c 100644
--- a/src/maasserver/static/js/angular/factories/tests/test_domains.js
+++ b/src/maasserver/static/js/angular/factories/tests/test_domains.js
@@ -12,9 +12,12 @@ describe("DomainsManager", function() {
1212
13 // Load the DomainsManager.13 // Load the DomainsManager.
14 var DomainsManager;14 var DomainsManager;
15 var $q, $rootScope;
15 beforeEach(inject(function($injector) {16 beforeEach(inject(function($injector) {
16 DomainsManager = $injector.get("DomainsManager");17 DomainsManager = $injector.get("DomainsManager");
17 RegionConnection = $injector.get("RegionConnection");18 RegionConnection = $injector.get("RegionConnection");
19 $q = $injector.get("$q");
20 $rootScope = $injector.get("$rootScope");
18 }));21 }));
1922
20 // Make a random domain.23 // Make a random domain.
@@ -31,6 +34,11 @@ describe("DomainsManager", function() {
31 if(angular.isDefined(selected)) {34 if(angular.isDefined(selected)) {
32 domain.$selected = selected;35 domain.$selected = selected;
33 }36 }
37 if(domain.id === 0) {
38 domain.is_default = true;
39 } else {
40 domain.is_default = false;
41 }
34 return domain;42 return domain;
35 }43 }
3644
@@ -44,7 +52,7 @@ describe("DomainsManager", function() {
44 expect(DomainsManager.getDefaultDomain()).toBe(null);52 expect(DomainsManager.getDefaultDomain()).toBe(null);
45 });53 });
4654
47 it("getDefaultDomain returns domain with id = 0", function() {55 it("getDefaultDomain returns domain with is_default", function() {
48 var zero = makeDomain(0);56 var zero = makeDomain(0);
49 DomainsManager._items.push(makeDomain());57 DomainsManager._items.push(makeDomain());
50 DomainsManager._items.push(zero);58 DomainsManager._items.push(zero);
@@ -61,6 +69,26 @@ describe("DomainsManager", function() {
61 });69 });
62 });70 });
6371
72 describe("setDefault", function() {
73 it("calls set_default for domain", function () {
74 var scope = $rootScope.$new();
75 var defer = $q.defer();
76 var promise = defer.promise;
77 spyOn(RegionConnection, "callMethod").and.returnValue(promise);
78 spyOn(DomainsManager, "reloadItems");
79 var domain_id = makeInteger(0, 100);
80 var record = {
81 id: domain_id
82 };
83 DomainsManager.setDefault(record);
84 expect(RegionConnection.callMethod).toHaveBeenCalledWith(
85 "domain.set_default", {'domain': domain_id});
86 defer.resolve(record);
87 scope.$digest();
88 expect(DomainsManager.reloadItems).toHaveBeenCalled();
89 });
90 });
91
64 describe("createDNSRecord", function() {92 describe("createDNSRecord", function() {
65 it("calls create_address_record for A record", function() {93 it("calls create_address_record for A record", function() {
66 spyOn(RegionConnection, "callMethod");94 spyOn(RegionConnection, "callMethod");
diff --git a/src/maasserver/static/partials/domains-list.html b/src/maasserver/static/partials/domains-list.html
index db6fa03..db7cced 100644
--- a/src/maasserver/static/partials/domains-list.html
+++ b/src/maasserver/static/partials/domains-list.html
@@ -63,21 +63,51 @@
63</div>63</div>
64<div class="p-strip" data-ng-show="!loading">64<div class="p-strip" data-ng-show="!loading">
65 <section class="row">65 <section class="row">
66 <table class="p-table--mobile-card">66 <table class="p-table-expanding" role="grid">
67 <thead>67 <thead>
68 <tr>68 <tr role="row">
69 <th class="table-col--25" data-ng-click="predicate='name'; reverse = !reverse" data-ng-class="{'is-sorted': predicate === 'name', 'sort-asc': reverse === false, 'sort-desc': reverse === true}" title="Domain">Domain</th>69 <th class="col-3" data-ng-click="predicate='name'; reverse = !reverse" data-ng-class="{'is-sorted': predicate === 'name', 'sort-asc': reverse === false, 'sort-desc': reverse === true}" title="Domain">Domain</th>
70 <th class="table-col--25" data-ng-click="predicate='authoritative'; reverse = !reverse" data-ng-class="{'is-sorted': predicate === 'authoritative', 'sort-asc': reverse === false, 'sort-desc': reverse === true}" title="Authoritative">Authoritative</th>70 <th class="col-2 u-align--right" data-ng-click="predicate='authoritative'; reverse = !reverse" data-ng-class="{'is-sorted': predicate === 'authoritative', 'sort-asc': reverse === false, 'sort-desc': reverse === true}" title="Authoritative">Authoritative</th>
71 <th class="table-col--25 u-align--right" data-ng-click="predicate='hosts'; reverse = !reverse" data-ng-class="{'is-sorted': predicate === 'hosts', 'sort-asc': reverse === false, 'sort-desc': reverse === true}" title="Hosts">Hosts</th>71 <th class="col-2 u-align--right" data-ng-click="predicate='hosts'; reverse = !reverse" data-ng-class="{'is-sorted': predicate === 'hosts', 'sort-asc': reverse === false, 'sort-desc': reverse === true}" title="Hosts">Hosts</th>
72 <th class="table-col--25 u-align--right" data-ng-click="predicate='resource_count'; reverse = !reverse" data-ng-class="{'is-sorted': predicate === 'resource_count', 'sort-asc': reverse === false, 'sort-desc': reverse === true}" title="Total Records">Total Records</th>72 <th class="col-2 u-align--right" data-ng-click="predicate='resource_count'; reverse = !reverse" data-ng-class="{'is-sorted': predicate === 'resource_count', 'sort-asc': reverse === false, 'sort-desc': reverse === true}" title="Total Records">Total Records</th>
73 <th class="col-3 u-align--right" title="Actions" data-ng-if="isSuperUser()"><span class="u-float--right">Actions</span></th>
73 </tr>74 </tr>
74 </thead>75 </thead>
75 <tbody>76 <tbody>
76 <tr data-ng-repeat="row in domains | orderBy:predicate:reverse track by row.id">77 <tr role="row" data-ng-repeat="row in domains | orderBy:predicate:reverse track by row.id"
77 <td class="table-col--25" aria-label="Domain"><a href="#/domain/{$ row.id $}" title="{$ row.displayname $}">{$ row.displayname $}</a></td>78 data-ng-class="{ 'is-active': confirmSetDefaultRow === row }">
78 <td class="table-col--25" aria-label="Authorative" title="{$ row.authoritative ? 'Yes' : 'No' $}">{$ row.authoritative ? "Yes" : "No" $}</td>79 <td class="col-3" aria-label="Domain"><a href="#/domain/{$ row.id $}" title="{$ row.displayname $}">{$ row.displayname $}</a></td>
79 <td class="table-col--25 u-align--right" aria-label="Hosts" title="{$ row.hosts $}">{$ row.hosts $}</td>80 <td class="col-2 u-align--right" aria-label="Authoritative" title="{$ row.authoritative ? 'Yes' : 'No' $}">{$ row.authoritative ? "Yes" : "No" $}</td>
80 <td class="table-col--25 u-align--right" aria-label="Total records" title="{$ row.resource_count $}">{$ row.resource_count $}</td>81 <td class="col-2 u-align--right" aria-label="Hosts" title="{$ row.hosts $}">{$ row.hosts $}</td>
82 <td class="col-2 u-align--right" aria-label="Total records" title="{$ row.resource_count $}">{$ row.resource_count $}</td>
83 <td class="col-3 u-align--right p-table--action-cell" style="min-height: 43px;" aria-label="Actions" data-ng-if="isSuperUser()">
84 <div class="p-contextual-menu" data-ng-class="{ 'u-hide': row.is_default === true || confirmSetDefaultRow === row }" toggle-ctrl>
85 <button class="p-button--base is-small p-contextual-menu__toggle" data-ng-click="toggleMenu()">
86 <i class="p-icon--contextual-menu u-no-margin--right">Actions</i></button>
87 <div class="p-contextual-menu__dropdown" role="menu" data-ng-show="isToggled">
88 <button class="p-contextual-menu__link"
89 aria-label="Set default"
90 data-ng-click="toggleMenu(); confirmSetDefault(row)">Set default...</button>
91 </div>
92 </div>
93 </td>
94 <td class="is-active p-table-expanding__panel p-table--action-cell col-12 u-align--right" data-ng-if="confirmSetDefaultRow === row">
95 <div class="row u-no-margin--top">
96 <hr />
97 </div>
98 <div class="row u-no-margin--top">
99 <div class="col-6">
100 Setting this domain as the default will update all existing machines
101 (in Ready state) with the new default domain. Are you sure?
102 </div>
103 <div class="col-6">
104 <span class="u-float--right">
105 <button class="p-button--base" type="button" data-ng-click="cancelSetDefault()">Cancel</button>
106 <button class="p-button--positive u-no-margin--top" data-ng-click="setDefault(row)">Save</button>
107 </span>
108 </div>
109 </div>
110 </td>
81 </tr>111 </tr>
82 </tbody>112 </tbody>
83 </div>113 </div>
diff --git a/src/maasserver/websockets/handlers/domain.py b/src/maasserver/websockets/handlers/domain.py
index a9a7c30..6a06ba6 100644
--- a/src/maasserver/websockets/handlers/domain.py
+++ b/src/maasserver/websockets/handlers/domain.py
@@ -15,6 +15,7 @@ from maasserver.forms.domain import DomainForm
15from maasserver.models import (15from maasserver.models import (
16 DNSData,16 DNSData,
17 DNSResource,17 DNSResource,
18 GlobalDefault,
18)19)
19from maasserver.models.domain import Domain20from maasserver.models.domain import Domain
20from maasserver.websockets.base import (21from maasserver.websockets.base import (
@@ -41,6 +42,7 @@ class DomainHandler(TimestampedModelHandler, AdminOnlyMixin):
41 'update',42 'update',
42 'delete',43 'delete',
43 'set_active',44 'set_active',
45 'set_default',
44 'create_dnsresource',46 'create_dnsresource',
45 'update_dnsresource',47 'update_dnsresource',
46 'delete_dnsresource',48 'delete_dnsresource',
@@ -65,8 +67,10 @@ class DomainHandler(TimestampedModelHandler, AdminOnlyMixin):
65 data["resource_count"] = len(rrsets)67 data["resource_count"] = len(rrsets)
66 if domain.is_default():68 if domain.is_default():
67 data["displayname"] = "%s (default)" % data["name"]69 data["displayname"] = "%s (default)" % data["name"]
70 data["is_default"] = True
68 else:71 else:
69 data["displayname"] = data["name"]72 data["displayname"] = data["name"]
73 data["is_default"] = False
70 return data74 return data
7175
72 def _get_domain_or_permission_error(self, params):76 def _get_domain_or_permission_error(self, params):
@@ -199,3 +203,10 @@ class DomainHandler(TimestampedModelHandler, AdminOnlyMixin):
199 self._get_domain_or_permission_error(params)203 self._get_domain_or_permission_error(params)
200 dnsdata = DNSData.objects.get(id=params['dnsdata_id'])204 dnsdata = DNSData.objects.get(id=params['dnsdata_id'])
201 dnsdata.delete()205 dnsdata.delete()
206
207 def set_default(self, params):
208 domain = self._get_domain_or_permission_error(params)
209 global_defaults = GlobalDefault.objects.instance()
210 global_defaults.domain = domain
211 global_defaults.save()
212 return self.full_dehydrate(domain)
diff --git a/src/maasserver/websockets/handlers/tests/test_domain.py b/src/maasserver/websockets/handlers/tests/test_domain.py
index b8d11b6..e917ecb 100644
--- a/src/maasserver/websockets/handlers/tests/test_domain.py
+++ b/src/maasserver/websockets/handlers/tests/test_domain.py
@@ -51,6 +51,7 @@ class TestDomainHandler(MAASServerTestCase):
51 "ttl": domain.ttl,51 "ttl": domain.ttl,
52 "updated": dehydrate_datetime(domain.updated),52 "updated": dehydrate_datetime(domain.updated),
53 "created": dehydrate_datetime(domain.created),53 "created": dehydrate_datetime(domain.created),
54 "is_default": domain.is_default(),
54 }55 }
55 ip_map = StaticIPAddress.objects.get_hostname_ip_mapping(56 ip_map = StaticIPAddress.objects.get_hostname_ip_mapping(
56 domain, raw_ttl=True)57 domain, raw_ttl=True)
@@ -617,3 +618,24 @@ class TestDomainHandlerAddressRecords(MAASServerTestCase):
617 'domain': domain.id,618 'domain': domain.id,
618 'dnsresource': resource.id,619 'dnsresource': resource.id,
619 })620 })
621
622 def test__set_default_sets_default(self):
623 user = factory.make_admin()
624 handler = DomainHandler(user, {}, None)
625 factory.make_Domain()
626 domain2 = factory.make_Domain()
627 self.assertThat(domain2.is_default(), Equals(False))
628 handler.set_default({
629 'domain': domain2.id,
630 })
631 domain2 = reload_object(domain2)
632 self.assertThat(domain2.is_default(), Equals(True))
633
634 def test__set_default_as_non_admin_fails(self):
635 user = factory.make_User()
636 handler = DomainHandler(user, {}, None)
637 domain = factory.make_Domain()
638 with ExpectedException(HandlerPermissionError):
639 handler.set_default({
640 'domain': domain.id,
641 })

Subscribers

People subscribed via source and target branches