ocket8888 commented on a change in pull request #5179:
URL: https://github.com/apache/trafficcontrol/pull/5179#discussion_r582439148



##########
File path: docs/source/api/v4/cdn_notifications.rst
##########
@@ -0,0 +1,201 @@
+..
+..
+.. Licensed under the Apache License, Version 2.0 (the "License");
+.. you may not use this file except in compliance with the License.
+.. You may obtain a copy of the License at
+..
+..     http://www.apache.org/licenses/LICENSE-2.0
+..
+.. Unless required by applicable law or agreed to in writing, software
+.. distributed under the License is distributed on an "AS IS" BASIS,
+.. WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+.. See the License for the specific language governing permissions and
+.. limitations under the License.
+..
+
+
+.. _to-api-cdn-notifications:
+
+*********************
+``cdn_notifications``
+*********************
+
+``GET``
+=======
+List all CDN notifications.
+
+:Auth. Required: Yes
+:Roles Required: Read-Only
+:Response Type:  Array
+
+Request Structure
+-----------------
+.. table:: Request Query Parameters
+
+       
+------------+----------+-----------------------------------------------------------------------------------------------------+
+       | Parameter  | Required | Description                                   
                                                      |
+       
+============+==========+=====================================================================================================+
+       | cdn        | no       | The CDN name of the notification you wish to 
retrieve.                                              |
+       
+------------+----------+-----------------------------------------------------------------------------------------------------+
+       | user       | no       | The username of the user responsible for 
creating the CDN notification.                             |
+       
+------------+----------+-----------------------------------------------------------------------------------------------------+
+
+.. code-block:: http
+       :caption: Request Example
+
+       GET /api/4.0/cdn_notifications HTTP/1.1
+       User-Agent: python-requests/2.22.0
+       Accept-Encoding: gzip, deflate
+       Accept: */*
+       Connection: keep-alive
+       Cookie: mojolicious=...
+
+Response Structure
+------------------
+:cdn:                  The name of the CDN to which the notification belongs to
+:lastUpdated:  The time and date this server entry was last updated in an 
ISO-like format
+:notification: The content of the notification
+:user:                 The user responsible for creating the notification
+
+.. code-block:: http
+       :caption: Response Example
+
+       HTTP/1.1 200 OK
+       Access-Control-Allow-Credentials: true
+       Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, 
Accept, Set-Cookie, Cookie
+       Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+       Access-Control-Allow-Origin: *
+       Content-Encoding: gzip
+       Content-Type: application/json
+       Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 02 Dec 2019 22:51:14 
GMT; Max-Age=3600; HttpOnly
+       Whole-Content-Sha512: 
F2NmDbTpXqrIQDX7IBKH9+1drtTL4XedSfJv6klMgLEZwbLCkddIXuSLpmgVCID6kTVqy3fTKjZS3U+HJ3YUEQ==
+       X-Server-Name: traffic_ops_golang/
+       Date: Mon, 02 Dec 2019 21:51:14 GMT
+       Content-Length: 128
+
+       { "response": [
+               {
+                       "cdn": "cdn1",
+                       "lastUpdated": "2019-12-02 21:49:08+00",
+                       "notification": "the content of the notification",
+                       "user": "username123",
+               }
+       ]}
+
+``POST``
+========
+Creates a notification for a specific CDN.
+
+.. note:: Currently only one notification per CDN is supported.
+
+:Auth. Required:       Yes
+:Roles Required:       "admin" or "operations"
+:Response Type:                Object
+
+Request Structure
+-----------------
+:cdn:                  The name of the CDN to which the notification shall 
belong
+:notification: The content of the notification
+
+.. code-block:: http
+       :caption: Request Example
+
+       POST /api/4.0/cdn_notifications HTTP/1.1
+       User-Agent: python-requests/2.22.0
+       Accept-Encoding: gzip, deflate
+       Accept: */*
+       Connection: keep-alive
+       Cookie: mojolicious=...
+       Content-Length: 29
+
+       {"cdn": "cdn1", "notification": "the content of the notification"}
+
+
+Response Structure
+------------------
+:cdn:                  The name of the CDN to which the notification belongs to
+:lastUpdated:  The time and date this server entry was last updated in an 
ISO-like format
+:notification: The content of the notification
+:user:                 The user responsible for creating the notification
+
+
+.. code-block:: http
+       :caption: Response Example
+
+       HTTP/1.1 200 OK
+       Access-Control-Allow-Credentials: true
+       Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, 
Accept, Set-Cookie, Cookie
+       Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+       Access-Control-Allow-Origin: *
+       Content-Encoding: gzip
+       Content-Type: application/json
+       Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 02 Dec 2019 22:49:08 
GMT; Max-Age=3600; HttpOnly
+       Whole-Content-Sha512: 
mx8b2GTYojz4QtMxXCMoQyZogCB504vs0yv6WGly4dwM81W3XiejWNuUwchRBYYi8QHaWsMZ3DaiGGfQi/8Giw==
+       X-Server-Name: traffic_ops_golang/
+       Date: Mon, 02 Dec 2019 21:49:08 GMT
+       Content-Length: 150
+
+       { "alerts": [
+               {
+                       "text": "notification was created.",
+                       "level": "success"
+               }
+       ],
+       "response": {
+               "cdn": "cdn1",
+               "lastUpdated": "2019-12-02 21:49:08+00",
+               "notification": "the content of the notification",
+               "user": "username123",
+       }
+}

Review comment:
       This closing brace needs to be indented. Or probably just at the end of 
the previous line.

##########
File path: lib/go-tc/cdn_notification.go
##########
@@ -0,0 +1,58 @@
+package tc
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "database/sql"
+
+       "github.com/apache/trafficcontrol/lib/go-tc/tovalidate"
+       "github.com/apache/trafficcontrol/lib/go-util"
+
+       "github.com/go-ozzo/ozzo-validation"
+)
+
+// CDNNotificationsResponse is a list of CDN notifications as a response.
+type CDNNotificationsResponse struct {
+       Response []CDNNotification `json:"response"`
+       Alerts
+}
+
+// CDNNotificationRequest encodes the request data for the POST
+// cdn_notifications endpoint.
+type CDNNotificationRequest struct {
+       CDN          string `json:"cdn"`
+       Notification string `json:"notification"`
+}
+
+// CDNNotification is a notification created for a specific CDN
+type CDNNotification struct {
+       CDN          *string    `json:"cdn" db:"cdn"`

Review comment:
       Is this allowed to be `null`?

##########
File path: docs/source/api/v4/cdn_notifications.rst
##########
@@ -0,0 +1,201 @@
+..
+..
+.. Licensed under the Apache License, Version 2.0 (the "License");
+.. you may not use this file except in compliance with the License.
+.. You may obtain a copy of the License at
+..
+..     http://www.apache.org/licenses/LICENSE-2.0
+..
+.. Unless required by applicable law or agreed to in writing, software
+.. distributed under the License is distributed on an "AS IS" BASIS,
+.. WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+.. See the License for the specific language governing permissions and
+.. limitations under the License.
+..
+
+
+.. _to-api-cdn-notifications:
+
+*********************
+``cdn_notifications``
+*********************
+
+``GET``
+=======
+List all CDN notifications.
+
+:Auth. Required: Yes
+:Roles Required: Read-Only
+:Response Type:  Array
+
+Request Structure
+-----------------
+.. table:: Request Query Parameters
+
+       
+------------+----------+-----------------------------------------------------------------------------------------------------+
+       | Parameter  | Required | Description                                   
                                                      |
+       
+============+==========+=====================================================================================================+
+       | cdn        | no       | The CDN name of the notification you wish to 
retrieve.                                              |
+       
+------------+----------+-----------------------------------------------------------------------------------------------------+
+       | user       | no       | The username of the user responsible for 
creating the CDN notification.                             |
+       
+------------+----------+-----------------------------------------------------------------------------------------------------+
+
+.. code-block:: http
+       :caption: Request Example
+
+       GET /api/4.0/cdn_notifications HTTP/1.1
+       User-Agent: python-requests/2.22.0
+       Accept-Encoding: gzip, deflate
+       Accept: */*
+       Connection: keep-alive
+       Cookie: mojolicious=...
+
+Response Structure
+------------------
+:cdn:                  The name of the CDN to which the notification belongs to
+:lastUpdated:  The time and date this server entry was last updated in an 
ISO-like format
+:notification: The content of the notification
+:user:                 The user responsible for creating the notification
+
+.. code-block:: http
+       :caption: Response Example
+
+       HTTP/1.1 200 OK
+       Access-Control-Allow-Credentials: true
+       Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, 
Accept, Set-Cookie, Cookie
+       Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+       Access-Control-Allow-Origin: *
+       Content-Encoding: gzip
+       Content-Type: application/json
+       Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 02 Dec 2019 22:51:14 
GMT; Max-Age=3600; HttpOnly
+       Whole-Content-Sha512: 
F2NmDbTpXqrIQDX7IBKH9+1drtTL4XedSfJv6klMgLEZwbLCkddIXuSLpmgVCID6kTVqy3fTKjZS3U+HJ3YUEQ==
+       X-Server-Name: traffic_ops_golang/
+       Date: Mon, 02 Dec 2019 21:51:14 GMT
+       Content-Length: 128
+
+       { "response": [
+               {
+                       "cdn": "cdn1",
+                       "lastUpdated": "2019-12-02 21:49:08+00",
+                       "notification": "the content of the notification",
+                       "user": "username123",
+               }
+       ]}
+
+``POST``
+========
+Creates a notification for a specific CDN.
+
+.. note:: Currently only one notification per CDN is supported.
+
+:Auth. Required:       Yes
+:Roles Required:       "admin" or "operations"
+:Response Type:                Object
+
+Request Structure
+-----------------
+:cdn:                  The name of the CDN to which the notification shall 
belong

Review comment:
       I don't think it's actually a problem, but there's a lot of weird 
whitespace between these field properties and their values. Looks like a tab 
character or three.

##########
File path: docs/source/api/v4/cdn_notifications.rst
##########
@@ -0,0 +1,201 @@
+..
+..
+.. Licensed under the Apache License, Version 2.0 (the "License");
+.. you may not use this file except in compliance with the License.
+.. You may obtain a copy of the License at
+..
+..     http://www.apache.org/licenses/LICENSE-2.0
+..
+.. Unless required by applicable law or agreed to in writing, software
+.. distributed under the License is distributed on an "AS IS" BASIS,
+.. WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+.. See the License for the specific language governing permissions and
+.. limitations under the License.
+..
+
+
+.. _to-api-cdn-notifications:
+
+*********************
+``cdn_notifications``
+*********************
+
+``GET``
+=======
+List all CDN notifications.
+
+:Auth. Required: Yes
+:Roles Required: Read-Only
+:Response Type:  Array
+
+Request Structure
+-----------------
+.. table:: Request Query Parameters
+
+       
+------------+----------+-----------------------------------------------------------------------------------------------------+
+       | Parameter  | Required | Description                                   
                                                      |
+       
+============+==========+=====================================================================================================+
+       | cdn        | no       | The CDN name of the notification you wish to 
retrieve.                                              |
+       
+------------+----------+-----------------------------------------------------------------------------------------------------+
+       | user       | no       | The username of the user responsible for 
creating the CDN notification.                             |
+       
+------------+----------+-----------------------------------------------------------------------------------------------------+
+
+.. code-block:: http
+       :caption: Request Example
+
+       GET /api/4.0/cdn_notifications HTTP/1.1
+       User-Agent: python-requests/2.22.0
+       Accept-Encoding: gzip, deflate
+       Accept: */*
+       Connection: keep-alive
+       Cookie: mojolicious=...
+
+Response Structure
+------------------
+:cdn:                  The name of the CDN to which the notification belongs to
+:lastUpdated:  The time and date this server entry was last updated in an 
ISO-like format
+:notification: The content of the notification
+:user:                 The user responsible for creating the notification
+
+.. code-block:: http
+       :caption: Response Example
+
+       HTTP/1.1 200 OK
+       Access-Control-Allow-Credentials: true
+       Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, 
Accept, Set-Cookie, Cookie
+       Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+       Access-Control-Allow-Origin: *
+       Content-Encoding: gzip
+       Content-Type: application/json
+       Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 02 Dec 2019 22:51:14 
GMT; Max-Age=3600; HttpOnly
+       Whole-Content-Sha512: 
F2NmDbTpXqrIQDX7IBKH9+1drtTL4XedSfJv6klMgLEZwbLCkddIXuSLpmgVCID6kTVqy3fTKjZS3U+HJ3YUEQ==
+       X-Server-Name: traffic_ops_golang/
+       Date: Mon, 02 Dec 2019 21:51:14 GMT
+       Content-Length: 128
+
+       { "response": [
+               {
+                       "cdn": "cdn1",
+                       "lastUpdated": "2019-12-02 21:49:08+00",
+                       "notification": "the content of the notification",
+                       "user": "username123",
+               }
+       ]}
+
+``POST``
+========
+Creates a notification for a specific CDN.
+
+.. note:: Currently only one notification per CDN is supported.
+
+:Auth. Required:       Yes
+:Roles Required:       "admin" or "operations"
+:Response Type:                Object
+
+Request Structure
+-----------------
+:cdn:                  The name of the CDN to which the notification shall 
belong
+:notification: The content of the notification
+
+.. code-block:: http
+       :caption: Request Example
+
+       POST /api/4.0/cdn_notifications HTTP/1.1
+       User-Agent: python-requests/2.22.0
+       Accept-Encoding: gzip, deflate
+       Accept: */*
+       Connection: keep-alive
+       Cookie: mojolicious=...
+       Content-Length: 29
+
+       {"cdn": "cdn1", "notification": "the content of the notification"}
+
+
+Response Structure
+------------------
+:cdn:                  The name of the CDN to which the notification belongs to
+:lastUpdated:  The time and date this server entry was last updated in an 
ISO-like format
+:notification: The content of the notification
+:user:                 The user responsible for creating the notification
+
+
+.. code-block:: http
+       :caption: Response Example
+
+       HTTP/1.1 200 OK
+       Access-Control-Allow-Credentials: true
+       Access-Control-Allow-Headers: Origin, X-Requested-With, Content-Type, 
Accept, Set-Cookie, Cookie
+       Access-Control-Allow-Methods: POST,GET,OPTIONS,PUT,DELETE
+       Access-Control-Allow-Origin: *
+       Content-Encoding: gzip
+       Content-Type: application/json
+       Set-Cookie: mojolicious=...; Path=/; Expires=Mon, 02 Dec 2019 22:49:08 
GMT; Max-Age=3600; HttpOnly
+       Whole-Content-Sha512: 
mx8b2GTYojz4QtMxXCMoQyZogCB504vs0yv6WGly4dwM81W3XiejWNuUwchRBYYi8QHaWsMZ3DaiGGfQi/8Giw==
+       X-Server-Name: traffic_ops_golang/
+       Date: Mon, 02 Dec 2019 21:49:08 GMT
+       Content-Length: 150
+
+       { "alerts": [
+               {
+                       "text": "notification was created.",
+                       "level": "success"
+               }
+       ],
+       "response": {
+               "cdn": "cdn1",
+               "lastUpdated": "2019-12-02 21:49:08+00",
+               "notification": "the content of the notification",
+               "user": "username123",
+       }
+}
+
+
+``DELETE``
+----------
+Deletes an existing CDN notification.
+
+:Auth. Required: Yes
+:Roles Required: "admin" or "operations"
+:Response Type: ``undefined``
+
+Request Structure
+-----------------
+
+.. code-block:: http
+       :caption: Request Example
+
+       DELETE /api/4.0/cdn_notifications?cdn=cdn1 HTTP/1.1

Review comment:
       Looks like an undocumented, required query parameter here.

##########
File path: traffic_ops/traffic_ops_golang/cdnnotification/cdnnotifications.go
##########
@@ -0,0 +1,190 @@
+package cdnnotification
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "database/sql"
+       "errors"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-util"
+       
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+       "net/http"
+
+       "github.com/apache/trafficcontrol/lib/go-tc"
+       "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"

Review comment:
       project-internal imports should be grouped beneath standard library 
imports

##########
File path: lib/go-tc/cdn_notification.go
##########
@@ -0,0 +1,58 @@
+package tc
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "database/sql"
+
+       "github.com/apache/trafficcontrol/lib/go-tc/tovalidate"
+       "github.com/apache/trafficcontrol/lib/go-util"
+
+       "github.com/go-ozzo/ozzo-validation"
+)
+
+// CDNNotificationsResponse is a list of CDN notifications as a response.
+type CDNNotificationsResponse struct {
+       Response []CDNNotification `json:"response"`
+       Alerts
+}
+
+// CDNNotificationRequest encodes the request data for the POST
+// cdn_notifications endpoint.
+type CDNNotificationRequest struct {
+       CDN          string `json:"cdn"`
+       Notification string `json:"notification"`
+}
+
+// CDNNotification is a notification created for a specific CDN
+type CDNNotification struct {
+       CDN          *string    `json:"cdn" db:"cdn"`
+       LastUpdated  *TimeNoMod `json:"lastUpdated" db:"last_updated"`

Review comment:
       Can we stop using `TimeNoMod`? If you just use `time.Time` it'll 
automatically use RFC3339, which is a lot easier to deal with.

##########
File path: traffic_portal/app/src/common/modules/header/HeaderController.js
##########
@@ -64,6 +78,68 @@ var HeaderController = function($rootScope, $scope, $state, 
$uibModal, $location
         trafficPortalService.dbDump();
     };
 
+    $scope.toggleNotification = function(cdn) {
+        if (cdn.notificationCreatedBy) {
+            confirmDeleteNotification(cdn);
+        } else {
+            confirmCreateNotification(cdn);
+        }
+    };
+
+    let confirmCreateNotification = function(cdn) {
+        const params = {
+            title: 'Create Global ' + cdn.name + ' Notification',
+            message: 'What is the content of your global notification for the 
' + cdn.name + ' CDN?'
+        };
+        const modalInstance = $uibModal.open({
+            templateUrl: 'common/modules/dialog/input/dialog.input.tpl.html',
+            controller: 'DialogInputController',
+            size: 'md',
+            resolve: {
+                params: function () {
+                    return params;
+                }
+            }
+        });
+        modalInstance.result.then(function(notification) {
+            cdnService.createNotification(cdn, notification).
+                then(
+                    function() {
+                        
$rootScope.$broadcast('headerController::notificationCreated');
+                    }
+                );
+        }, function () {
+            // do nothing

Review comment:
       Do you wanna just `console.error` the error?

##########
File path: traffic_ops/v4-client/cdn_notifications.go
##########
@@ -0,0 +1,49 @@
+/*
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package client
+
+import (
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-tc"
+       "net/http"
+)
+
+const (
+       API_CDN_NOTIFICATIONS = apiBase + "/cdn_notifications"

Review comment:
       Can you add a GoDoc to this exported symbol? Or just un-export it, I 
don't think anything needs it. Although I know (almost) all other client files 
use this casing for their request paths.

##########
File path: traffic_ops/v4-client/cdn_notifications.go
##########
@@ -0,0 +1,49 @@
+/*
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package client
+
+import (
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-tc"
+       "net/http"
+)
+
+const (
+       API_CDN_NOTIFICATIONS = apiBase + "/cdn_notifications"
+)
+
+// Returns a list of CDN Notifications.

Review comment:
       GoDoc should begin with the name of the documented symbol.

##########
File path: traffic_portal/app/src/common/modules/header/HeaderController.js
##########
@@ -64,6 +78,68 @@ var HeaderController = function($rootScope, $scope, $state, 
$uibModal, $location
         trafficPortalService.dbDump();
     };
 
+    $scope.toggleNotification = function(cdn) {
+        if (cdn.notificationCreatedBy) {
+            confirmDeleteNotification(cdn);
+        } else {
+            confirmCreateNotification(cdn);
+        }
+    };
+
+    let confirmCreateNotification = function(cdn) {
+        const params = {
+            title: 'Create Global ' + cdn.name + ' Notification',
+            message: 'What is the content of your global notification for the 
' + cdn.name + ' CDN?'
+        };
+        const modalInstance = $uibModal.open({
+            templateUrl: 'common/modules/dialog/input/dialog.input.tpl.html',
+            controller: 'DialogInputController',
+            size: 'md',
+            resolve: {
+                params: function () {
+                    return params;
+                }
+            }
+        });
+        modalInstance.result.then(function(notification) {
+            cdnService.createNotification(cdn, notification).
+                then(
+                    function() {
+                        
$rootScope.$broadcast('headerController::notificationCreated');
+                    }
+                );
+        }, function () {
+            // do nothing
+        });
+    };
+
+    let confirmDeleteNotification = function(cdn) {
+        const params = {
+            title: 'Delete Global ' + cdn.name + ' Notification',
+            message: 'Are you sure you want to delete the global notification 
for the ' + cdn.name + ' CDN? This will remove the notification from the view 
of all users.'
+        };
+        const modalInstance = $uibModal.open({
+            templateUrl: 
'common/modules/dialog/confirm/dialog.confirm.tpl.html',
+            controller: 'DialogConfirmController',
+            size: 'md',
+            resolve: {
+                params: function () {
+                    return params;
+                }
+            }
+        });
+        modalInstance.result.then(function() {
+            cdnService.deleteNotification(cdn).
+                then(
+                    function() {
+                        
$rootScope.$broadcast('headerController::notificationDeleted');
+                    }
+                );
+        }, function () {
+            // do nothing

Review comment:
       Same as above RE: `console.error`

##########
File path: traffic_portal/app/src/common/modules/header/header.tpl.html
##########
@@ -40,7 +40,7 @@
             <li role="presentation" class="dropdown" 
ng-if="hasCapability('change-logs-read')">
                 <div class="btn-group" title="Change Logs" uib-dropdown 
is-open="alerts.isopen">
                     <button id="alertsButton" type="button" class="btn 
btn-link" ng-click="getChangeLogs()" uib-dropdown-toggle>
-                        <i class="fa fa-comment-o"></i>
+                        <i class="fa" ng-class="{ 'fa-comment': newLogCount() 
> 0, 'fa-comment-o': newLogCount() == 0 }"></i>

Review comment:
       `ng-class` supports expresions that evaluate to strings and arrays of 
strings as well as object property mappings. What I'm getting at is I think 
it's simpler - and calls `newLogCount` less times - to use `ng-class="{ 
newLogCount() > 0 ? 'fa-comment' : 'fa-comment-o'}"`

##########
File path: traffic_ops/traffic_ops_golang/routing/routes.go
##########
@@ -23,6 +23,7 @@ import (
        "encoding/json"
        "errors"
        "fmt"
+       
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/cdnnotification"

Review comment:
       project-local imports should be grouped beneath standard library imports

##########
File path: lib/go-tc/cdn_notification.go
##########
@@ -0,0 +1,58 @@
+package tc
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "database/sql"
+
+       "github.com/apache/trafficcontrol/lib/go-tc/tovalidate"
+       "github.com/apache/trafficcontrol/lib/go-util"
+
+       "github.com/go-ozzo/ozzo-validation"
+)
+
+// CDNNotificationsResponse is a list of CDN notifications as a response.
+type CDNNotificationsResponse struct {
+       Response []CDNNotification `json:"response"`
+       Alerts
+}
+
+// CDNNotificationRequest encodes the request data for the POST
+// cdn_notifications endpoint.
+type CDNNotificationRequest struct {
+       CDN          string `json:"cdn"`
+       Notification string `json:"notification"`
+}
+
+// CDNNotification is a notification created for a specific CDN
+type CDNNotification struct {
+       CDN          *string    `json:"cdn" db:"cdn"`
+       LastUpdated  *TimeNoMod `json:"lastUpdated" db:"last_updated"`
+       Notification *string    `json:"notification" db:"notification"`
+       User         *string    `json:"user" db:"user"`
+}
+
+// Validate validates the CDNNotification request is valid for creation.
+func (n *CDNNotification) Validate(tx *sql.Tx) error {
+       errs := validation.Errors{
+               "cdn": validation.Validate(n.CDN, validation.Required),
+       }
+       return util.JoinErrs(tovalidate.ToErrors(errs))
+}

Review comment:
       With the recent DS CacheURL removal, we've sort of started putting these 
into Traffic Ops itself, since it's weird to expose this thing that requires a 
transaction into the TO database to work.

##########
File path: traffic_ops/v4-client/cdn_notifications.go
##########
@@ -0,0 +1,49 @@
+/*
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+   http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package client
+
+import (
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-tc"
+       "net/http"

Review comment:
       project-local imports should be grouped beneath standard library imports

##########
File path: traffic_portal/app/src/common/modules/form/cdn/FormCDNController.js
##########
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-var FormCDNController = function(cdn, $scope, $location, $uibModal, formUtils, 
stringUtils, locationUtils, cdnService) {
+var FormCDNController = function(cdn, $scope, $location, $state, $uibModal, 
formUtils, stringUtils, locationUtils, cdnService, messageModel) {

Review comment:
       The only changes I see to this file are adding these  dependency 
injections and removing a blank line. Does this controller use those injections 
already, and it was a bug that they weren't being injected?

##########
File path: traffic_portal/app/src/common/api/CDNService.js
##########
@@ -222,7 +222,42 @@ var CDNService = function($http, locationUtils, 
messageModel, ENV) {
                 throw err;
                        }
                );
-       };
+       }
+
+    this.getNotifications = function(queryParams) {
+        return $http.get(ENV.api['root'] + 'cdn_notifications', { params: 
queryParams }).then(
+            function(result) {
+                return result.data.response;
+            },
+            function(err) {
+                throw err;
+            }
+        );
+    };
+
+    this.createNotification = function(cdn, notification) {
+        return $http.post(ENV.api['root'] + 'cdn_notifications', { cdn: 
cdn.name, notification: notification}).then(
+            function(result) {
+                return result;
+            },
+            function(err) {
+                messageModel.setMessages(err.data.alerts, false);
+                throw err;
+            }
+        );
+    };
+
+    this.deleteNotification = function(cdn) {
+        return $http.delete(ENV.api['root'] + 'cdn_notifications?cdn=' + 
cdn.name).then(

Review comment:
       You should either pass the `cdn` parameter in the `$http.delete`'s query 
string parameters optional argument, or use `encodeURIComponent` to ensure 
proper URI component encoding.

##########
File path: traffic_ops/traffic_ops_golang/cdnnotification/cdnnotifications.go
##########
@@ -0,0 +1,190 @@
+package cdnnotification
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import (
+       "database/sql"
+       "errors"
+       "fmt"
+       "github.com/apache/trafficcontrol/lib/go-util"
+       
"github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/dbhelpers"
+       "net/http"
+
+       "github.com/apache/trafficcontrol/lib/go-tc"
+       "github.com/apache/trafficcontrol/traffic_ops/traffic_ops_golang/api"
+)
+
+const readQuery = `
+SELECT cn.cdn, 
+       cn.last_updated,
+       cn.user, 
+       cn.notification 
+FROM cdn_notification as cn
+INNER JOIN cdn ON cdn.name = cn.cdn
+INNER JOIN tm_user ON tm_user.username = cn.user
+`
+
+const insertQuery = `
+INSERT INTO cdn_notification (cdn, "user", notification)
+VALUES ($1, $2, $3)
+RETURNING cdn_notification.cdn,
+          cdn_notification.last_updated,
+          cdn_notification.user,
+          cdn_notification.notification
+`
+
+const deleteQuery = `
+DELETE FROM cdn_notification
+WHERE cdn_notification.cdn = $1
+RETURNING cdn_notification.cdn,
+          cdn_notification.last_updated,
+          cdn_notification.user,
+          cdn_notification.notification
+`
+
+// Read is the handler for GET requests to /cdn_notifications.
+func Read(w http.ResponseWriter, r *http.Request) {
+       inf, userErr, sysErr, errCode := api.NewInfo(r, nil, nil)
+       tx := inf.Tx.Tx
+       if userErr != nil || sysErr != nil {
+               api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+               return
+       }
+       defer inf.Close()
+
+       cdnNotifications := []tc.CDNNotification{}
+
+       queryParamsToQueryCols := map[string]dbhelpers.WhereColumnInfo{
+               "cdn":  dbhelpers.WhereColumnInfo{"cdn.name", nil},
+               "user": dbhelpers.WhereColumnInfo{"tm_user.username", nil},
+       }
+
+       where, orderBy, pagination, queryValues, errs := 
dbhelpers.BuildWhereAndOrderByAndPagination(inf.Params, queryParamsToQueryCols)
+       if len(errs) > 0 {
+               sysErr = util.JoinErrs(errs)
+               errCode = http.StatusBadRequest
+               api.HandleErr(w, r, tx, errCode, nil, sysErr)
+               return
+       }
+
+       query := readQuery + where + orderBy + pagination
+       rows, err := inf.Tx.NamedQuery(query, queryValues)
+       if err != nil {
+               userErr, sysErr, errCode = api.ParseDBError(err)
+               if sysErr != nil {
+                       sysErr = fmt.Errorf("notification read query: %v", 
sysErr)
+               }
+
+               api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+               return
+       }
+       defer rows.Close()
+
+       for rows.Next() {
+               var n tc.CDNNotification
+               if err = rows.Scan(&n.CDN, &n.LastUpdated, &n.User, 
&n.Notification); err != nil {
+                       api.HandleErr(w, r, tx, http.StatusInternalServerError, 
nil, errors.New("scanning cdn notifications: "+err.Error()))
+                       return
+               }
+               cdnNotifications = append(cdnNotifications, n)
+       }
+
+       api.WriteResp(w, r, cdnNotifications)
+}
+
+// Create is the handler for POST requests to /cdn_notifications.
+func Create(w http.ResponseWriter, r *http.Request) {
+       inf, sysErr, userErr, errCode := api.NewInfo(r, nil, nil)
+       tx := inf.Tx.Tx
+       if sysErr != nil || userErr != nil {
+               api.HandleErr(w, r, tx, errCode, userErr, sysErr)
+               return
+       }
+       defer inf.Close()
+
+       var n tc.CDNNotification

Review comment:
       Shouldn't this be a `tc.CDNNotificationRequest`? I mean, I think this 
works since you're using basic `QueryRow`, but I just assume this is the reason 
you made that struct.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to