Feedback from PR

specifically the scope that is used for attaching event listeners
for the $emit/$braodcast/$on that is used for routing socket messages.
Also removing some commented out code

awx/ui/client/src/app.js

@@ -802,7 +802,6 @@ var tower = angular.module('Tower', [
                     ConfigService.getConfig().then(function() {
                         Timer.init().then(function(timer) {
                             $rootScope.sessionTimer = timer;
-                            // $rootScope.$emit('OpenSocket');
                             SocketService.init();
                             pendoService.issuePendoIdentity();
                             CheckLicense.test();
@@ -847,10 +846,6 @@ var tower = angular.module('Tower', [
             // create a promise that will resolve when features are loaded
             $rootScope.featuresConfigured = $q.defer();
         }
-        if (!$rootScope.socketPromise) {
-            // create a promise that resolves when the socket connection is open
-            $rootScope.socketPromise = $q.defer();
-        }
         $rootScope.licenseMissing = true;
         //the authorization controller redirects to the home page automatcially if there is no last path defined. in order to override
         // this, set the last path to /portal for instances where portal is visited for the first time.

awx/ui/client/src/controllers/Home.js

@@ -28,7 +28,7 @@ export function Home($scope, $compile, $stateParams, $rootScope, $location, $log
 
     var dataCount = 0;
 
-    $rootScope.$on('ws-jobs', function () {
+    $scope.$on('ws-jobs', function () {
         Rest.setUrl(GetBasePath('dashboard'));
         Rest.get()
         .success(function (data) {

awx/ui/client/src/controllers/Jobs.js

@@ -65,7 +65,7 @@ export function JobsListController ($rootScope, $log, $scope, $compile, $statePa
         jobs_scope.viewJob = function (id) {
             $state.transitionTo('jobDetail', {id: id});
         };
-        
+
         jobs_scope.showJobType = true;
         LoadJobsScope({
             parent_scope: $scope,
@@ -110,17 +110,11 @@ export function JobsListController ($rootScope, $log, $scope, $compile, $statePa
            }
        };
 
-        if ($rootScope.removeJobStatusChange) {
+        $scope.$on('ws-jobs', function() {
-            $rootScope.removeJobStatusChange();
-        }
-        $rootScope.removeJobStatusChange = $rootScope.$on('ws-jobs', function() {
             $scope.refreshJobs();
         });
 
-        if ($rootScope.removeScheduleStatusChange) {
+        $scope.$on('ws-schedules', function() {
-            $rootScope.removeScheduleStatusChange();
-        }
-        $rootScope.removeScheduleStatusChange = $rootScope.$on('ws-schedules', function() {
             if (api_complete) {
                 scheduled_scope.search('schedule');
             }

awx/ui/client/src/controllers/Projects.js

@@ -86,11 +86,7 @@ export function ProjectsList ($scope, $rootScope, $location, $log, $stateParams,
         }
     });
 
-    // Handle project update status changes
+    $scope.$on(`ws-jobs`, function(e, data) {
-    if ($rootScope.removeJobStatusChange) {
-        $rootScope.removeJobStatusChange();
-    }
-    $rootScope.removeJobStatusChange = $rootScope.$on(`ws-jobs`, function(e, data) {
         var project;
         $log.debug(data);
         if ($scope.projects) {

awx/ui/client/src/inventories/manage/groups/groups-list.controller.js

@@ -101,10 +101,7 @@
             }));
         };
 
-        if ($rootScope.inventoryManageStatus) {
+        $scope.$on(`ws-jobs`, function(e, data){
-            $rootScope.inventoryManageStatus();
-        }
-        $rootScope.inventoryManageStatus = $rootScope.$on(`ws-jobs`, function(e, data){
             var group = Find({ list: $scope.groups, key: 'id', val: data.group_id });
             if(data.status === 'failed' || data.status === 'successful'){
                 $state.reload();

awx/ui/client/src/job-detail/host-summary/host-summary.controller.js

@@ -53,17 +53,14 @@
         }
         // emitted by the API in the same function used to persist host summary data
         // JobEvent.update_host_summary_from_stats() from /awx/main.models.jobs.py
-        $rootScope.removeJobSummaryComplete = $rootScope.$on('ws-jobs-summary', function(e, data) {
+        $scope.$on('ws-jobs-summary', function(e, data) {
             // discard socket msgs we don't care about in this context
             if (parseInt($stateParams.id) === data.unified_job_id){
                 init();
             }
         });
 
-        if ($rootScope.removeJobStatusChange) {
+        $scope.$on('ws-jobs', function(e, data) {
-            $rootScope.removeJobStatusChange();
-        }
-        $rootScope.removeJobStatusChange = $rootScope.$on('ws-jobs', function(e, data) {
             if (parseInt($stateParams.id) === data.unified_job_id){
                 $scope.status = data.status;
             }

awx/ui/client/src/job-detail/job-detail.controller.js

@@ -197,30 +197,22 @@ export default
                 "<p><i class=\"fa fa-circle changed-hosts-color\"></i> Changed</p>\n" +
                 "<p><i class=\"fa fa-circle unreachable-hosts-color\"></i> Unreachable</p>\n" +
                 "<p><i class=\"fa fa-circle failed-hosts-color\"></i> Failed</p>\n";
-            function openSocket() {
-                if ($rootScope.removeJobEventChange) {
-                    $rootScope.removeJobEventChange();
-                }
-                $rootScope.removeJobEventChange = $rootScope.$on(`ws-job_events-${job_id}`, function(e, data) {
-                    // update elapsed time on each event received
-                    scope.job_status.elapsed = GetElapsed({
-                        start: scope.job.created,
-                        end: Date.now()
-                    });
-                    if (api_complete && data.id > lastEventId) {
-                        scope.waiting = false;
-                        data.event = data.event_name;
-                        DigestEvent({ scope: scope, event: data });
-                    }
-                    UpdateDOM({ scope: scope });
-                });
-            }
-            openSocket();
 
-            if ($rootScope.removeJobStatusChange) {
+            scope.$on(`ws-job_events-${job_id}`, function(e, data) {
-                $rootScope.removeJobStatusChange();
+                // update elapsed time on each event received
-            }
+                scope.job_status.elapsed = GetElapsed({
-            $rootScope.removeJobStatusChange = $rootScope.$on(`ws-jobs`, function(e, data) {
+                    start: scope.job.created,
+                    end: Date.now()
+                });
+                if (api_complete && data.id > lastEventId) {
+                    scope.waiting = false;
+                    data.event = data.event_name;
+                    DigestEvent({ scope: scope, event: data });
+                }
+                UpdateDOM({ scope: scope });
+            });
+
+            scope.$on(`ws-jobs`, function(e, data) {
                 // if we receive a status change event for the current job indicating the job
                 // is finished, stop event queue processing and reload
                 if (parseInt(data.unified_job_id, 10) === parseInt(job_id,10)) {
@@ -234,10 +226,7 @@ export default
                 }
             });
 
-            if ($rootScope.removeJobSummaryComplete) {
+            scope.$on('ws-jobs-summary', function() {
-                $rootScope.removeJobSummaryComplete();
-            }
-            $rootScope.removeJobSummaryComplete = $rootScope.$on('ws-jobs-summary', function() {
                 // the job host summary should now be available from the API
                 $log.debug('Trigging reload of job_host_summaries');
                 scope.$emit('InitialLoadComplete');

awx/ui/client/src/job-templates/list/job-templates-list.controller.js

@@ -37,11 +37,7 @@ export default
             view.inject(list, { mode: mode, scope: $scope });
             $rootScope.flashMessage = null;
 
-
+            $scope.$on(`ws-jobs`, function () {
-            if ($rootScope.JobStatusChange) {
-                $rootScope.JobStatusChange();
-            }
-            $rootScope.JobStatusChange = $rootScope.$on(`ws-jobs`, function () {
                 $scope.search(list.iterator);
             });
 

awx/ui/client/src/organizations/linkout/controllers/organizations-job-templates.controller.js

@@ -21,10 +21,7 @@ export default ['$scope', '$rootScope', '$location', '$log',
             generator = GenerateList,
             orgBase = GetBasePath('organizations');
 
-        if ($rootScope.JobStatusChange) {
+        $scope.$on(`ws-jobs`, function () {
-            $rootScope.JobStatusChange();
-        }
-        $rootScope.JobStatusChange = $rootScope.$on(`ws-jobs`, function () {
             $scope.search(list.iterator);
         });
 

awx/ui/client/src/organizations/linkout/controllers/organizations-projects.controller.js

@@ -86,11 +86,7 @@ export default ['$scope', '$rootScope', '$location', '$log',
                         }
                     });
 
-                    // Handle project update status changes
+                    $scope.$on(`ws-jobs`, function(e, data) {
-                    if ($rootScope.removeJobStatusChange) {
-                        $rootScope.removeJobStatusChange();
-                    }
-                    $rootScope.removeJobStatusChange = $rootScope.$on(`ws-jobs`, function(e, data) {
                         var project;
                         $log.debug(data);
                         if ($scope.projects) {

awx/ui/client/src/portal-mode/portal-mode-jobs.controller.js

@@ -13,12 +13,9 @@ export function PortalModeJobsController($scope, $rootScope, GetBasePath, Genera
     defaultUrl = GetBasePath('jobs') + '?created_by=' + $rootScope.current_user.id,
     pageSize = 12;
 
-    if ($rootScope.removeJobStatusChange) {
+    $scope.$on('ws-jobs', function() {
-           $rootScope.removeJobStatusChange();
-    }
-   $rootScope.removeJobStatusChange = $rootScope.$on('ws-jobs', function() {
        $scope.search('job');
-   });
+    });
 
     $scope.iterator = list.iterator;
     $scope.activeFilter = 'user';

awx/ui/client/src/shared/socket/socket.service.js

@@ -5,9 +5,10 @@
  *************************************************/
 import ReconnectingWebSocket from 'reconnectingwebsocket';
 export default
-['$rootScope', '$location', '$log','$state',
+['$rootScope', '$location', '$log','$state', '$q',
-    function ($rootScope, $location, $log, $state) {
+    function ($rootScope, $location, $log, $state, $q) {
-        var needsResubscribing = false;
+        var needsResubscribing = false,
+        socketPromise = $q.defer();
         return {
             init: function() {
                 var self = this,
@@ -24,7 +25,7 @@ export default
 
                     self.socket.onopen = function () {
                         $log.debug("Websocket connection opened.");
-                        $rootScope.socketPromise.resolve();
+                        socketPromise.resolve();
                         self.checkStatus();
                         if(needsResubscribing){
                             self.subscribe(self.getLast());
@@ -60,22 +61,40 @@ export default
                 }
             },
             onMessage: function(e){
+                // Function called when messages are received on by the UI from
+                // the API over the websocket. This will route each message to
+                // the appropriate controller for the current $state.
+                e.data = e.data.replace(/\\/g, '');
+                e.data = e.data.substr(0, e.data.length-1);
+                e.data = e.data.substr(1);
                 $log.debug('Received From Server: ' + e.data);
+
                 var data = JSON.parse(e.data), str = "";
                 if(data.group_name==="jobs" && !('status' in data)){
                     // we know that this must have been a
-                    // summary complete message b/c status is missing
+                    // summary complete message b/c status is missing.
+                    // A an object w/ group_name === "jobs" AND a 'status' key
+                    // means it was for the event: status_changed.
                     $log.debug('Job summary_complete ' + data.unified_job_id);
-                    $rootScope.$emit('ws-jobs-summary', data);
+                    $rootScope.$broadcast('ws-jobs-summary', data);
                     return;
                 }
                 else if(data.group_name==="job_events"){
-                    str = `ws-${data.group_name}-${data.job}`;
+                    // The naming scheme is "ws" then a
+                    // dash (-) and the group_name, then the job ID
+                    // ex: 'ws-jobs-<jobId>'
+                    str = `ws-${data.group_name}-${data.job}`
                 }
                 else if(data.group_name==="ad_hoc_command_events"){
+                    // The naming scheme is "ws" then a
+                    // dash (-) and the group_name, then the job ID
+                    // ex: 'ws-jobs-<jobId>'
                     str = `ws-${data.group_name}-${data.ad_hoc_command}`;
                 }
                 else if(data.group_name==="control"){
+                    // As of Tower v. 3.1.0, there is only 1 "control"
+                    // message, which is for expiring the session if the
+                    // session limit is breached.
                     $log.debug(data.reason);
                     $rootScope.sessionTimer.expireSession('session_limit');
                     $state.go('signOut');
@@ -86,7 +105,7 @@ export default
                     // ex: 'ws-jobs'
                     str = `ws-${data.group_name}`;
                 }
-                $rootScope.$emit(str, data);
+                $rootScope.$broadcast(str, data);
             },
             disconnect: function(){
                 if(this.socket){
@@ -94,10 +113,18 @@ export default
                 }
             },
             subscribe: function(state){
+                // Subscribe is used to tell the API that the UI wants to
+                // listen for specific messages. A subscription object could
+                // look like {"groups":{"jobs": ["status_changed", "summary"]}.
+                // This is used by all socket-enabled $states
                 this.emit(JSON.stringify(state.socket));
                 this.setLast(state);
             },
             unsubscribe: function(state){
+                // Unsubscribing tells the API that the user is no longer on
+                // on a socket-enabled page, and sends an empty groups object
+                // to the API: {"groups": {}}.
+                // This is used for all pages that are socket-disabled
                 if(this.requiresNewSubscribe(state)){
                     this.emit(JSON.stringify(state.socket));
                 }
@@ -110,6 +137,9 @@ export default
                 return this.last;
             },
             requiresNewSubscribe(state){
+                // This function is used for unsubscribing. If the last $state
+                // required an "unsubscribe", then we don't need to unsubscribe
+                // again, b/c the UI is already unsubscribed from all groups
                 if (this.getLast() !== undefined){
                     if( _.isEmpty(state.socket.groups) && _.isEmpty(this.getLast().socket.groups)){
                         return false;
@@ -123,6 +153,7 @@ export default
                 }
             },
             checkStatus: function() {
+                // Function for changing the socket indicator icon in the nav bar
                 var self = this;
                 if(self){
                     if(self.socket){
@@ -144,9 +175,11 @@ export default
 
             },
             emit: function(data, callback) {
+                // Function used for sending objects to the API over the
+                // websocket.
                 var self = this;
                 $log.debug('Sent to Websocket Server: ' + data);
-                $rootScope.socketPromise.promise.then(function(){
+                socketPromise.promise.then(function(){
                     self.socket.send(data, function () {
                         var args = arguments;
                         self.scope.$apply(function () {
@@ -156,6 +189,28 @@ export default
                         });
                     });
                 });
+            },
+            addStateResolve: function(state, id){
+                // This function is used for add a state resolve to all states,
+                // socket-enabled AND socket-disabled, and whether the $state
+                // requires a subscribe or an unsubscribe
+                self = this;
+                socketPromise.promise.then(function(){
+                    if(!state.socket){
+                        state.socket = {groups: {}};
+                        self.unsubscribe(state);
+                    }
+                    else{
+                        if(state.socket.groups.hasOwnProperty( "job_events")){
+                            state.socket.groups.job_events = [id];
+                        }
+                        if(state.socket.groups.hasOwnProperty( "ad_hoc_command_events")){
+                            state.socket.groups.ad_hoc_command_events = [id];
+                        }
+                        self.subscribe(state);
+                    }
+                    return true;
+                });
             }
         };
     }];

awx/ui/client/src/shared/stateExtender.provider.js

@@ -2,29 +2,17 @@ export default function($stateProvider) {
     this.$get = function() {
         return {
             addSocket: function(state){
+                // The login route has a 'null' socket because it should
+                // neither subscribe or unsubscribe
                 if(state.socket!==null){
                     if(!state.resolve){
                         state.resolve = {};
                     }
-                    state.resolve.socket = ['SocketService', '$rootScope', '$stateParams',
+                    state.resolve.socket = ['SocketService', '$stateParams',
-                        function(SocketService, $rootScope, $stateParams) {
+                        function(SocketService, $stateParams) {
-                            $rootScope.socketPromise.promise.then(function(){
+                            SocketService.addStateResolve(state, $stateParams.id);
-                                if(!state.socket){
+                        }
-                                    state.socket = {groups: {}};
+                    ];
-                                    SocketService.unsubscribe(state);
-                                }
-                                else{
-                                    if(state.socket.groups.hasOwnProperty( "job_events")){
-                                        state.socket.groups.job_events = [$stateParams.id];
-                                    }
-                                    if(state.socket.groups.hasOwnProperty( "ad_hoc_command_events")){
-                                        state.socket.groups.ad_hoc_command_events = [$stateParams.id];
-                                    }
-                                    SocketService.subscribe(state);
-                                }
-                                return true;
-                            });
-                        }];
                 }
             },
 

awx/ui/client/src/standard-out/log/standard-out-log.controller.js

@@ -22,7 +22,7 @@ export default ['$log', '$rootScope', '$scope', '$state', '$stateParams', 'Proce
         function openSockets() {
             if ($state.current.name === 'jobDetail') {
                $log.debug("socket watching on job_events-" + job_id);
-               $rootScope.$on(`ws-job_events-${job_id}`, function() {
+               $scope.$on(`ws-job_events-${job_id}`, function() {
                    $log.debug("socket fired on job_events-" + job_id);
                    if (api_complete) {
                        event_queue++;
@@ -31,7 +31,7 @@ export default ['$log', '$rootScope', '$scope', '$state', '$stateParams', 'Proce
             }
             if ($state.current.name === 'adHocJobStdout') {
                 $log.debug("socket watching on ad_hoc_command_events-" + job_id);
-                $rootScope.$on(`ws-ad_hoc_command_events-${job_id}`, function() {
+                $scope.$on(`ws-ad_hoc_command_events-${job_id}`, function() {
                     $log.debug("socket fired on ad_hoc_command_events-" + job_id);
                     if (api_complete) {
                         event_queue++;
@@ -179,10 +179,7 @@ export default ['$log', '$rootScope', '$scope', '$state', '$stateParams', 'Proce
 
         // We watch for job status changes here.  If the job completes we want to clear out the
         // stdout interval and kill the live_event_processing flag.
-        if ($scope.removeJobStatusChange) {
+        $scope.$on(`ws-jobs`, function(e, data) {
-            $scope.removeJobStatusChange();
-        }
-        $scope.removeJobStatusChange = $rootScope.$on(`ws-jobs`, function(e, data) {
             if (parseInt(data.unified_job_id, 10) === parseInt(job_id,10)) {
                 if (data.status === 'failed' || data.status === 'canceled' ||
                         data.status === 'error' || data.status === 'successful') {

awx/ui/client/src/standard-out/standard-out.controller.js

@@ -25,10 +25,7 @@ export function JobStdoutController ($rootScope, $scope, $state, $stateParams,
 
     // Listen for job status updates that may come across via sockets.  We need to check the payload
     // to see whethere the updated job is the one that we're currently looking at.
-    if ($scope.removeJobStatusChange) {
+    $scope.$on(`ws-jobs`, function(e, data) {
-        $scope.removeJobStatusChange();
-    }
-    $scope.removeJobStatusChange = $rootScope.$on(`ws-jobs`, function(e, data) {
         if (parseInt(data.unified_job_id, 10) === parseInt(job_id,10) && $scope.job) {
             $scope.job.status = data.status;
         }
@@ -39,12 +36,6 @@ export function JobStdoutController ($rootScope, $scope, $state, $stateParams,
         }
     });
 
-    // Unbind $rootScope socket event binding(s) so that they don't get triggered
-    // in another instance of this controller
-    $scope.$on('$destroy', function() {
-        $scope.removeJobStatusChange();
-    });
-
     // Set the parse type so that CodeMirror knows how to display extra params YAML/JSON
     $scope.parseType = 'yaml';
 

awx/ui/tests/spec/socket/socket.service-test.js

@@ -22,7 +22,6 @@ describe('Service: SocketService', () => {
         it('should send to ws-jobs-summary', function(){
             event = {data : {group_name: "jobs"}};
             event.data = JSON.stringify(event.data);
-            console.log(event);
             SocketService.onMessage(event);
             expect(rootScope.$emit).toHaveBeenCalledWith('ws-jobs-summary', event.data);
         });