10
\$\begingroup\$

I have an app that makes calls to the Podio API. The Podio Api uses OAuth2 protocol , after user's authenticate with their podio username and password an access and refresh token is provided for subsequent requests. Podio has a Javascript SDK that handles authentication and API calls. I wrote a simple Session Manager utilizing the $localStorage object.

The app does the following:

  1. Allows Users to Login with their Podio credentials
  2. Makes an API call to get events list and allows users to click on event to get event details
  3. Makes an API call to get attendee list for specific event and attendee details
  4. Can add attendees to an event by scanning a Barcode Number

The app currently works and uses the Cordova Barcode Scanner Plugin. I am currently using Podio's authentication check everytime I transition from state to state to see if User is logged in. If not I send them to the login screen. This is placed in every controller. I am currently making an API call everytime a controller is loaded as well. I am looking for best practices for code construction, design, and how I could possibly make the code more readable in angular. Also, if my current way of checking authentication every time is correct and ways to reduce API calls. The latest code can be seen in my github repository here. But here are the main files (not including the events and attendees services):

Main app.js file

/**
 * The main Freedom Nation app module
 *
 * @type {angular.Module}
 */

angular.module('freedomnation', ['ionic', 'ngCordova','ngMessages', 'freedomnation.controllers', 'freedomnation.services','freedomnation.filters'])

.run(function($ionicPlatform,$rootScope,$state,$stateParams, $http,$localstorage) {

      /*
      * Local Storage Object Used*/
      $localstorage.get('password', function (token) {
        $http.defaults.headers.common['Authorization'] = 'OAuth2 ' + token.accessToken;
      });

      $rootScope.$state = $state;
      $rootScope.$stateParams = $stateParams;

  $ionicPlatform.ready(function() {
    // Hide the accessory bar by default (remove this to show the accessory bar above the keyboard
    // for form inputs)

    if (window.cordova && window.cordova.plugins && window.cordova.plugins.Keyboard) {
      cordova.plugins.Keyboard.hideKeyboardAccessoryBar(true);
      cordova.plugins.Keyboard.disableScroll(true);

    }
    if (window.StatusBar) {
      // org.apache.cordova.statusbar required
     StatusBar.styleLightContent();

    }
  });
})

.config(function($stateProvider, $urlRouterProvider) {

  // Ionic uses AngularUI Router which uses the concept of states
  // Learn more here: https://github.com/angular-ui/ui-router
  // Set up the various states which the app can be in.
  // Each state's controller can be found in controllers.js
  $stateProvider
  // setup an abstract state for the tabs directive
    .state('tab', {
    url: '/tab',
    abstract: true,
    templateUrl: 'views/tabs.html'
  })

  // Each tab has its own nav history stack:

  .state('tab.events', {
    url: '/events',
    views: {
      'tab-event': {
        templateUrl: 'views/events.html',
        controller: 'EventsCtrl',
        resolve: {
          events: function (EventService) {
            return EventService;
          }
        }
      }
    }
  })
  .state('tab.event-detail',{
    url:'/event/:eventId',
        views: {
          'tab-event':{
            templateUrl: 'views/event.html',
            controller: 'EventDetailsCtrl',
            resolve: {
              event : function(EventService) {
                return EventService;
              },
              attendee : function(AttendeeService) {
                return AttendeeService;
              }
            }
          }
        },
        params: {eventId: null}
  })
      .state('tab.attendees',{
        url: '/attendees/:attendeeIds',
        views: {
          'tab-event':{
            templateUrl: 'views/attendees.html',
            controller: 'AttendeesCtrl',
            resolve: {
              attendees : function(AttendeeService) {
                return AttendeeService;
              }
            }
          }
        },
        params: {
          eventId: null,
          attendeeIds: {array:true}
        }
      })
      .state('tab.attendee-detail',{
        url: '/attendee/:attendeeId',
        views: {
          'tab-event':{
            templateUrl: 'views/attendee.html',
            controller: 'AttendeeDetailCtrl',
            resolve: {
              attendee : function(AttendeeService) {
                return AttendeeService;
              }
            }
          }
        },
        params:{
          eventId: null,
          attendeeId: null,
          attending: true
        }
      })
      .state('login', {
        url: '/login',
        templateUrl: 'views/login.html',
        controller: 'authenticationFormCtrl'
      });

      $urlRouterProvider.otherwise('/tab/events');

});

services.js

angular.module('freedomnation.services', [])
    .factory('Podio', ['$localstorage', function ($localstorage) {

      var podio = new PodioJS({
        authType: 'password',
        clientId: 'fnation',
        clientSecret: 'clientsecrethere'
      }, {
        sessionStore: $localstorage
      });

      return {
        podio: podio,
      };
    }])
    .factory('$localstorage', ['$window', function ($window) {
      return {
        set: function (podioOAuth, callback) {
          $window.localStorage.setItem('podioOAuth', JSON.stringify(podioOAuth));
          $window.location.reload();
        },
        get: function (authType, callback) {
          var podioOAuth = $window.localStorage.getItem('podioOAuth');
          if (podioOAuth) {
            podioOAuth = JSON.parse(podioOAuth);
          }
          callback(podioOAuth || {});
        },
        logOut: function () {
          $window.localStorage.removeItem('podioOAuth');
          $window.location.reload();
        }
      };
    }])
    .factory('utils', function () {
      return {
        convertDataUrl: function(rawImg) {
          var raw = '',
              bytes = new Uint8Array(rawImg.data),
              length = bytes.length;
          for (var i = 0; i < length; i++) {
            raw += String.fromCharCode(bytes[i]);
          }

          var b64 = btoa(raw);
          var dataURL = "data:image/jpeg;base64," + b64;

          return dataURL;
        }
      }

    });

login.controller.client.js

angular.module('freedomnation.controllers')
.controller('authenticationFormCtrl', ['$scope', '$state','Podio', '$ionicPopup', function ($scope,$state,Podio, $ionicPopup) {

    /*
    * Initialize Login Info
    * */
    $scope.auth = {
        email: '',
        password: ''
    };

    Podio.podio.isAuthenticated()
        .then(function () {
            $state.go('tab.events');
        });

    /*
    * Sign-in Behavior
    * @param {Object} Form Object
    */
    $scope.signIn = function(form) {
        if(form.$valid) {
            Podio.podio.authenticateWithCredentials(form.email.$modelValue, form.password.$modelValue, function (error) {
                console.log(error);
                if (error) {
                    console.log(error);
                    $scope.showAlert()
                } else {
                    console.log('no error')
                    $state.go('tab.events');
                }
            });
        }else{
            console.log('Form Invalid');
        }

    };


    /*
     * Login Error Alert Function
     */
    $scope.showAlert = function() {
        var alertPopup = $ionicPopup.alert({
            title: 'Login Unsuccesful!',
            cssClass: 'login-alert',
            template: 'Please Try Again!',
            okType: 'button-dark'
        });
    }
}]);

events.controller.client.js - For Event List

 /**
 * The Events controller for the app. The controller:
 * - retrieves and persists the events model from Podio API via the Event service
 * - exposes the model to the template
 */

angular.module('freedomnation.controllers')
.controller('EventsCtrl', ['$scope','Podio', '$ionicLoading', 'events', '$state', function ($scope, Podio, $ionicLoading,events,$state) {


    $ionicLoading.show({
        content: 'Loading',
        animation:'fade-in',
        showBackdrop: true
    });

    $scope.events = [];

    Podio.podio.isAuthenticated()
        .catch(function(error) {
            $ionicLoading.hide();
            $state.go('login');
        })
        .then(function () {
            return events.getEvents();
        })
        .then(function(response) {
            $scope.events = response;
        }).then(function() {
            $ionicLoading.hide();
        })
        .catch(function(error) {
            console.log(error);
        });

}]);

event.controller.client.js - for event details

 /**
 * The Event Detail controller for the app. The controller:
 * - retrieves and persists the events model from Podio API via the Event service
 * - exposes the model to the template
 */

angular.module('freedomnation.controllers')
.controller('EventDetailsCtrl', ['$scope','$cordovaBarcodeScanner', '$stateParams', '$state', 'Podio', '$ionicLoading','event','attendee',function ($scope,$cordovaBarcodeScanner, $stateParams, $state,Podio, $ionicLoading,event,attendee) {


    $ionicLoading.show({
        content: 'Loading',
        animation:'fade-in',
        showBackdrop: true
    });

    $scope.attendeeId = '';
    $scope.eventId = $stateParams.eventId;


    Podio.podio.isAuthenticated()
        .catch(function() {
            $ionicLoading.hide();
            $state.go('login');
        })
        .then(function() {
            return event.getEvent($scope.eventId);
        })
        .catch(function(error) {
            console.log(error);
        })
        .then(function(data) {
            $scope.event = data;
        })
        .then(function() {
            $ionicLoading.hide();
            $scope.attending = true;
        });


    $scope.scanBarcode = function () {

        //Scanbarcode
        $cordovaBarcodeScanner.scan()
            .then(function (imageData) {
                attendee.getAttendeeByBarcode(imageData.text)
                    .then(function(response) {
                        $state.go('tab.attendee-detail',
                            {
                                eventId: $scope.eventId,
                                attendeeId: response.id,
                                attending: false
                            });
                    })
                    .catch(function(error) {
                        alert(error);
                    });
            })
            .catch(function (error) {
                alert('An error occurred -> ' + error);
            });

    };

}]);

atteendees.controller.client.js - Controlling Attendee Listing

    /**
 * The Attendees controller for the app. The controller:
 * - retrieves and persists the attendees model from Podio API via the Attendee service
 * - exposes the model to the template
 */
angular.module('freedomnation.controllers')
    .controller('AttendeesCtrl', ['$scope', '$stateParams','Podio','$ionicLoading','$state','attendees', function ($scope,$stateParams,Podio,$ionicLoading,$state,attendees) {

        $ionicLoading.show({
            content: 'Loading',
            animation:'fade-in',
            showBackdrop: true
        });


        $scope.eventId = $stateParams.eventId;

        if($stateParams.attendeeIds !== undefined) {

            var attendeeIds = JSON.parse($stateParams.attendeeIds);

            Podio.podio.isAuthenticated()
                .catch(function(error) {
                    $state.go('login');
                    $ionicLoading.hide();
                    console.log(error);
                })
                .then(function() {
                    return attendees.getAttendees(attendeeIds);
                }).
                then(function(response) {
                    $scope.attendees = response;

                }).then(function () {
                $ionicLoading.hide();
            });
        } else {
            $ionicLoading.hide()
        }
    }]
);

attendee.controller.client.js - Getting Attendee Detail

/**
 * The Attendee controller for the app. The controller:
 * - retrieves and persists the individual attendee model data from Podio API via the Attendee service
 * - exposes the model to the template
 */

angular.module('freedomnation.controllers')
    .controller('AttendeeDetailCtrl', ['$scope', '$state', '$stateParams', 'Podio', '$ionicLoading', '$http','attendee','$ionicPopup','$ionicHistory', function ($scope, $state, $stateParams, Podio, $ionicLoading, $http,attendee, $ionicPopup,$ionicHistory) {

        $ionicLoading.show({
            content: 'Loading',
            animation: 'fade-in',
            showBackdrop: true
        });


        var eventId = $stateParams.eventId,
            attendeeId = $stateParams.attendeeId;
        $scope.attending = true;
        if($stateParams.attending == false) {
            $scope.attending = false;
        }
        Podio.podio.isAuthenticated()
            .catch(function() {
                $ionicLoading.hide();
                $state.go('login');
            })
            .then(function() {
            return attendee.getAttendee(attendeeId);
            })
            .catch(function(error) {
                console.log(error);
            })
            .then(function(data) {
                $scope.attendee = data;
            })
            .then(function() {
                $ionicLoading.hide();
            });



        $scope.addToEvent = function () {
            $ionicLoading.show({
                content: 'Loading',
                animation: 'fade-in',
                showBackdrop: true
            });

            attendee.addToEvent(eventId,attendeeId)
                .then(function(response) {

                    if(response !== null) {
                        $ionicLoading.hide();
                        $ionicPopup.alert({
                            title: 'Confirmation',
                            template: 'Member Added'
                        })
                            .then(function(res) {
                                $ionicHistory.goBack();
                            });
                    }
                })
                .catch(function(error) {
                    console.log(error);
                })
        };

    }]
);
\$\endgroup\$
  • \$\begingroup\$ Where you catch() in mid-chain, remember to rethrow the error or return a default value. Otherwise, when an error is caught, a following .then's success handler will fire and will be passed undefined, (which is rarely what's needed). See, for example attendee.controller.client.js \$\endgroup\$ – Roamer-1888 Dec 4 '15 at 21:53
  • \$\begingroup\$ @Roamer-1888 Thanks I appreciate the help. I'll correct it. Is there anything else that I can improve upon? \$\endgroup\$ – inspired Dec 8 '15 at 11:44
  • \$\begingroup\$ Inspired, nothing else jumps out at me. \$\endgroup\$ – Roamer-1888 Dec 8 '15 at 18:39
  • \$\begingroup\$ @Roamer-1888 would you take a look at my events service ? \$\endgroup\$ – inspired Dec 9 '15 at 0:01
  • \$\begingroup\$ Looking at those catches again, I can't see any reason for any of them to be mid-chain. An error in a promise chain will drop straight through .then(successHandler) to be caught by the first .catch(errorHandler) it encounters. Even in .scanBarcode(), the outer catch() should alert an error arising from the inner chain. BUT WAIT - there's a missing return; the inner chain should read return attendee.getAttendeeByBarcode(...). \$\endgroup\$ – Roamer-1888 Dec 9 '15 at 0:41
2
\$\begingroup\$
if (window.cordova && window.cordova.plugins && window.cordova.plugins.Keyboard) {
      cordova.plugins.Keyboard.hideKeyboardAccessoryBar(true);
      cordova.plugins.Keyboard.disableScroll(true);

    }

Ionic has this feature builtin, and even has a plugin written specifically to make this process smoother and more reliable. I even wrote an answer on SO about it.


Your states look good, but your formatting is a little broken. You swap between four and two space indentation and some lines are completely wrong. I suggest using a cleaner, inbuilt in your IDE (like Atom's beautify), or online (like JSFiddle's tidy), but a cleaner would be good.

 .state('tab.attendee-detail',{
    url: '/attendee/:attendeeId',
    views: {
      'tab-event':{
        templateUrl: 'views/attendee.html',
        controller: 'AttendeeDetailCtrl',
        resolve: {
          attendee : function(AttendeeService) {
            return AttendeeService;
          }
        }
      }
    },
    params:{
      eventId: null,
      attendeeId: null,
      attending: true
    }
  })

The fact that you swap between two and four space indentation during your states makes me think you copy-pasted them in and just changed the values.

This isn't bad, but it could be, if you don't know what it does.


| improve this answer | |
\$\endgroup\$
4
\$\begingroup\$

The code looks pretty good to me.

I am rather new to AngularJS myself, but I did some a spot where you could get rid of some redundancy in your attendee.controller.client.js

change this

    $scope.attending = true;
    if($stateParams.attending == false) {
        $scope.attending = false;
    }

to a single line of code

$scope.attending = $stateParams.attending;
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ +1 That makes a lot of sense. I appreciate your help. I would like to reduce any JS code redundancies. \$\endgroup\$ – inspired Mar 19 '16 at 17:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.