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

[Mike Pontillo]

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

[Anthony Dillon (ya-bo-ng)]

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?

[Kit Randel (blr)]

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

review: Approve

[Mike Pontillo (mpontillo)]

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)

5557462... by Mike Pontillo on 2018-10-03

Hide domain actions menu for non-admins.

[Lee Trager (ltrager)]

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

review: Approve

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', [
         $scope.predicate = "name";
         $scope.reverse = false;
         $scope.loading = true;
+        $scope.confirmSetDefaultRow = null;
 
         // This will hold the AddDomainController once it's initialized.  The
         // controller will set this variable as it's always a child of this
@@ -43,6 +44,19 @@ angular.module('MAAS').controller('DomainsListController', [
             return UsersManager.isSuperUser();
         };
 
+        $scope.confirmSetDefault = function(domain) {
+            $scope.confirmSetDefaultRow = domain;
+        };
+
+        $scope.cancelSetDefault = function() {
+            $scope.confirmSetDefaultRow = null;
+        };
+
+        $scope.setDefault = function(domain) {
+            DomainsManager.setDefault(domain);
+            $scope.confirmSetDefaultRow = null;
+        };
+
         ManagerHelperService.loadManagers(
             $scope, [DomainsManager, UsersManager]).then(
             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() {
         });
     });
 
+    describe("confirmSetDefault", function() {
+
+        it("sets confirmSetDefaultRow to the specified row", function() {
+            var controller = makeController();
+            var obj = {
+                id: makeInteger(0, 100)
+            };
+            $scope.confirmSetDefault(obj);
+            expect($scope.confirmSetDefaultRow).toBe(obj);
+        });
+    });
+
+    describe("cancelSetDefault", function() {
+
+        it("sets confirmSetDefaultRow to the specified row", function() {
+            var controller = makeController();
+            var obj = {
+                id: makeInteger(0, 100)
+            };
+            $scope.confirmSetDefaultRow = obj;
+            $scope.cancelSetDefault();
+            expect($scope.confirmSetDefaultRow).toBe(null);
+        });
+    });
+
+    describe("setDefault", function() {
+
+        it("calls DomainsManager.setDefault and clears selection", function() {
+            var controller = makeController();
+            spyOn(DomainsManager, "setDefault");
+            var obj = {
+                id: makeInteger(0, 100)
+            };
+            $scope.confirmSetDefaultRow = obj;
+            $scope.setDefault(obj);
+            expect(DomainsManager.setDefault).toHaveBeenCalledWith(obj);
+            expect($scope.confirmSetDefaultRow).toBe(null);
+
+        });
+    });
+
     setupController = function(domains) {
         var defer = $q.defer();
         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(
             }
         };
 
+        // Set the specified domain as the default.
+        DomainsManager.prototype.setDefault = function(domain) {
+            var promise = RegionConnection.callMethod(
+                "domain.set_default", {'domain': domain.id});
+            promise.then(function() {
+                // Reload the domains, so that the new default will be
+                // reflected everywhere, and record counts will be properly
+                // updated.
+                self.reloadItems();
+            });
+        };
+
         DomainsManager.prototype.getDefaultDomain = function() {
             if(this._items.length === 0) {
                 return null;
             } else {
                 var i;
                 for(i=0;i<this._items.length;i++) {
-                    if(this._items[i].id === 0) {
+                    if(this._items[i].is_default === true) {
                         return this._items[i];
                     }
                 }
@@ -104,5 +116,6 @@ angular.module('MAAS').factory(
             return null;
         };
 
-        return new DomainsManager();
+        var self = new DomainsManager();
+        return self;
     }]);
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() {
 
     // Load the DomainsManager.
     var DomainsManager;
+    var $q, $rootScope;
     beforeEach(inject(function($injector) {
         DomainsManager = $injector.get("DomainsManager");
         RegionConnection = $injector.get("RegionConnection");
+        $q = $injector.get("$q");
+        $rootScope = $injector.get("$rootScope");
     }));
 
     // Make a random domain.
@@ -31,6 +34,11 @@ describe("DomainsManager", function() {
         if(angular.isDefined(selected)) {
             domain.$selected = selected;
         }
+        if(domain.id === 0) {
+            domain.is_default = true;
+        } else {
+            domain.is_default = false;
+        }
         return domain;
     }
 
@@ -44,7 +52,7 @@ describe("DomainsManager", function() {
             expect(DomainsManager.getDefaultDomain()).toBe(null);
         });
 
-        it("getDefaultDomain returns domain with id = 0", function() {
+        it("getDefaultDomain returns domain with is_default", function() {
             var zero = makeDomain(0);
             DomainsManager._items.push(makeDomain());
             DomainsManager._items.push(zero);
@@ -61,6 +69,26 @@ describe("DomainsManager", function() {
         });
     });
 
+    describe("setDefault", function() {
+        it("calls set_default for domain", function () {
+            var scope = $rootScope.$new();
+            var defer = $q.defer();
+            var promise = defer.promise;
+            spyOn(RegionConnection, "callMethod").and.returnValue(promise);
+            spyOn(DomainsManager, "reloadItems");
+            var domain_id = makeInteger(0, 100);
+            var record = {
+                id: domain_id
+            };
+            DomainsManager.setDefault(record);
+            expect(RegionConnection.callMethod).toHaveBeenCalledWith(
+                "domain.set_default", {'domain': domain_id});
+            defer.resolve(record);
+            scope.$digest();
+            expect(DomainsManager.reloadItems).toHaveBeenCalled();
+        });
+    });
+
     describe("createDNSRecord", function() {
         it("calls create_address_record for A record", function() {
             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 @@
 </div>
 <div class="p-strip" data-ng-show="!loading">
     <section class="row">
-        <table class="p-table--mobile-card">
+        <table class="p-table-expanding" role="grid">
             <thead>
-                <tr>
-                    <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>
-                    <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>
-                    <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>
-                    <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>
+                <tr role="row">
+                    <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>
+                    <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>
+                    <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>
+                    <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>
+                    <th class="col-3 u-align--right" title="Actions" data-ng-if="isSuperUser()"><span class="u-float--right">Actions</span></th>
                 </tr>
             </thead>
             <tbody>
-                <tr data-ng-repeat="row in domains | orderBy:predicate:reverse track by row.id">
-                    <td class="table-col--25" aria-label="Domain"><a href="#/domain/{$ row.id $}" title="{$ row.displayname $}">{$ row.displayname $}</a></td>
-                    <td class="table-col--25" aria-label="Authorative" title="{$ row.authoritative ? 'Yes' : 'No' $}">{$ row.authoritative ? "Yes" : "No" $}</td>
-                    <td class="table-col--25 u-align--right" aria-label="Hosts" title="{$ row.hosts $}">{$ row.hosts $}</td>
-                    <td class="table-col--25 u-align--right" aria-label="Total records" title="{$ row.resource_count $}">{$ row.resource_count $}</td>
+                <tr role="row" data-ng-repeat="row in domains | orderBy:predicate:reverse track by row.id"
+                    data-ng-class="{ 'is-active': confirmSetDefaultRow === row }">
+                    <td class="col-3" aria-label="Domain"><a href="#/domain/{$ row.id $}" title="{$ row.displayname $}">{$ row.displayname $}</a></td>
+                    <td class="col-2 u-align--right" aria-label="Authoritative" title="{$ row.authoritative ? 'Yes' : 'No' $}">{$ row.authoritative ? "Yes" : "No" $}</td>
+                    <td class="col-2 u-align--right" aria-label="Hosts" title="{$ row.hosts $}">{$ row.hosts $}</td>
+                    <td class="col-2 u-align--right" aria-label="Total records" title="{$ row.resource_count $}">{$ row.resource_count $}</td>
+                    <td class="col-3 u-align--right p-table--action-cell" style="min-height: 43px;" aria-label="Actions" data-ng-if="isSuperUser()">
+                        <div class="p-contextual-menu" data-ng-class="{ 'u-hide': row.is_default === true || confirmSetDefaultRow === row }" toggle-ctrl>
+                            <button class="p-button--base is-small p-contextual-menu__toggle" data-ng-click="toggleMenu()">
+                                <i class="p-icon--contextual-menu u-no-margin--right">Actions</i></button>
+                            <div class="p-contextual-menu__dropdown" role="menu" data-ng-show="isToggled">
+                                <button class="p-contextual-menu__link"
+                                        aria-label="Set default"
+                                        data-ng-click="toggleMenu(); confirmSetDefault(row)">Set default...</button>
+                            </div>
+                        </div>
+                    </td>
+                    <td class="is-active p-table-expanding__panel p-table--action-cell col-12 u-align--right" data-ng-if="confirmSetDefaultRow === row">
+                        <div class="row u-no-margin--top">
+                            <hr />
+                        </div>
+                        <div class="row u-no-margin--top">
+                            <div class="col-6">
+                                Setting this domain as the default will update all existing machines
+                                (in Ready state) with the new default domain. Are you sure?
+                            </div>
+                            <div class="col-6">
+                                <span class="u-float--right">
+                                    <button class="p-button--base" type="button" data-ng-click="cancelSetDefault()">Cancel</button>
+                                    <button class="p-button--positive u-no-margin--top" data-ng-click="setDefault(row)">Save</button>
+                                </span>
+                            </div>
+                        </div>
+                    </td>
                 </tr>
             </tbody>
         </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
 from maasserver.models import (
     DNSData,
     DNSResource,
+    GlobalDefault,
 )
 from maasserver.models.domain import Domain
 from maasserver.websockets.base import (
@@ -41,6 +42,7 @@ class DomainHandler(TimestampedModelHandler, AdminOnlyMixin):
             'update',
             'delete',
             'set_active',
+            'set_default',
             'create_dnsresource',
             'update_dnsresource',
             'delete_dnsresource',
@@ -65,8 +67,10 @@ class DomainHandler(TimestampedModelHandler, AdminOnlyMixin):
         data["resource_count"] = len(rrsets)
         if domain.is_default():
             data["displayname"] = "%s (default)" % data["name"]
+            data["is_default"] = True
         else:
             data["displayname"] = data["name"]
+            data["is_default"] = False
         return data
 
     def _get_domain_or_permission_error(self, params):
@@ -199,3 +203,10 @@ class DomainHandler(TimestampedModelHandler, AdminOnlyMixin):
         self._get_domain_or_permission_error(params)
         dnsdata = DNSData.objects.get(id=params['dnsdata_id'])
         dnsdata.delete()
+
+    def set_default(self, params):
+        domain = self._get_domain_or_permission_error(params)
+        global_defaults = GlobalDefault.objects.instance()
+        global_defaults.domain = domain
+        global_defaults.save()
+        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):
             "ttl": domain.ttl,
             "updated": dehydrate_datetime(domain.updated),
             "created": dehydrate_datetime(domain.created),
+            "is_default": domain.is_default(),
             }
         ip_map = StaticIPAddress.objects.get_hostname_ip_mapping(
             domain, raw_ttl=True)
@@ -617,3 +618,24 @@ class TestDomainHandlerAddressRecords(MAASServerTestCase):
                 'domain': domain.id,
                 'dnsresource': resource.id,
             })
+
+    def test__set_default_sets_default(self):
+        user = factory.make_admin()
+        handler = DomainHandler(user, {}, None)
+        factory.make_Domain()
+        domain2 = factory.make_Domain()
+        self.assertThat(domain2.is_default(), Equals(False))
+        handler.set_default({
+            'domain': domain2.id,
+        })
+        domain2 = reload_object(domain2)
+        self.assertThat(domain2.is_default(), Equals(True))
+
+    def test__set_default_as_non_admin_fails(self):
+        user = factory.make_User()
+        handler = DomainHandler(user, {}, None)
+        domain = factory.make_Domain()
+        with ExpectedException(HandlerPermissionError):
+            handler.set_default({
+                'domain': domain.id,
+            })