Merge hotfix/744 (#753)

Add graceful handling for attempting to load airport that does not exist

.eslintrc

@@ -21,6 +21,7 @@
         "space-before-function-paren": [2, "never"],
         "no-case-declarations": 1,
         "no-continue": 0,
+        "no-unused-vars": ["error", { "argsIgnorePattern": "event|error" }],
 
         # The rules below are only included to get through the first rounds of refactor. the
         # rules should be re-implemented and errors addressed at a later date

CHANGELOG.md

@@ -1,9 +1,15 @@
+## 5.5.1 (October 1, 2017)
+--
+### Hotfix
+- Checks if the airport in localStorage exists before loading it [#709](https://github.com/openscope/openscope/issues/709) and [#744](https://github.com/openscope/openscope/issues/744)
+- Updates `.eslint` rules to ignore unused `event` and `error` parameters. [#710](https://github.com/openscope/openscope/issues/710) and [#744](https://github.com/openscope/openscope/issues/744)
+
+
 ## 5.5.0 (October 1, 2017)
 ---
 ### New Features
 - Add `sa`, `saa`, `sh`, `sah`, `ss`, `sas` commands [#641](https://github.com/openscope/openscope/issues/641)
 - Add toggleable scope command bar, and lays foundation for the Scope, its commands, and its collections of radar targets. [#14](https://github.com/openscope/openscope/issues/14)
-- Checks if the airport in localStorage exists before loading it [#709](https://github.com/openscope/openscope/issues/709)
 - The mouse button to drag the radar screen is now right click [#564](https://github.com/openscope/openscope/issues/564)
 - Adds Ted Stevens Anchorage Intl. (PANC) [#637](https://github.com/openscope/openscope/issues/637)
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "openscope",
-  "version": "5.5.0",
+  "version": "5.5.1",
   "description": "An ATC simulator in HTML5",
   "engines": {
     "node": "7.0.0",

src/assets/scripts/client/App.js

@@ -2,6 +2,7 @@ import $ from 'jquery';
 import AppController from './AppController';
 import EventBus from './lib/EventBus';
 import TimeKeeper from './engine/TimeKeeper';
+import { DEFAULT_AIRPORT_ICAO } from './constants/airportConstants';
 import { EVENT } from './constants/eventNames';
 import { LOG } from './constants/logLevel';
 
@@ -49,63 +50,106 @@ export default class App {
         this.prop.log = LOG.DEBUG;
         this.prop.loaded = false;
 
-        return this.createHandlers()
-            .initiateDataLoad(airportLoadList, initialAirportToLoad);
+        return this.setupHandlers()
+            .loadInitialAirport(airportLoadList, initialAirportToLoad);
     }
 
     /**
      * Create event handlers
      *
      * @for App
-     * @method createHandlers
+     * @method setupHandlers
      * @chainable
      */
-    createHandlers() {
+    setupHandlers() {
+        this.loadDefaultAiportAfterStorageIcaoFailureHandler = this.loadDefaultAiportAfterStorageIcaoFailure.bind(this);
+        this.loadAirlinesAndAircraftHandler = this.loadAirlinesAndAircraft.bind(this);
+        this.setupChildrenHandler = this.setupChildren.bind(this);
+
         this.eventBus.on(EVENT.PAUSE_UPDATE_LOOP, this.updateRun);
 
         return this;
     }
 
     /**
-     * Lifecycle method. Should be called only once on initialization.
+     * Used to load data for the initial airport using an icao from
+     * either localStorage or `DEFAULT_AIRPORT_ICAO`
+     *
+     * If a localStorage airport cannot be found, we will attempt
+     * to load the `DEFAULT_AIRPORT_ICAO`
      *
-     * Used to load an initial data set from several sources.
+     * Lifecycle method. Should be called only once on initialization
      *
      * @for App
-     * @method setupChildren
+     * @method loadInitialAirport
      * @param airportLoadList {array<object>}  List of airports to load
      */
-    initiateDataLoad(airportLoadList, initialAirportToLoad) {
+    loadInitialAirport(airportLoadList, initialAirportToLoad) {
+        const initialAirportIcao = initialAirportToLoad.toLowerCase();
+
+        $.getJSON(`assets/airports/${initialAirportIcao}.json`)
+            .then((response) => this.loadAirlinesAndAircraftHandler(airportLoadList, initialAirportIcao, response))
+            .catch((error) => this.loadDefaultAiportAfterStorageIcaoFailureHandler(airportLoadList));
+    }
+
+    /**
+     * Used only when an attempt to load airport data with an icao in localStorage fails.
+     * In this case we attempt to load the default airport with this method
+     *
+     * Lifecycle method. Should be called only once on initialization
+     *
+     * @for App
+     * @method onLoadDefaultAirportAfterStorageIcaoFailure
+     * @param {array<object>} airportLoadList
+     */
+    loadDefaultAiportAfterStorageIcaoFailure(airportLoadList) {
+        $.getJSON(`assets/airports/${DEFAULT_AIRPORT_ICAO}.json`)
+            .then((defaultAirportResponse) => this.loadAirlinesAndAircraftHandler(
+                airportLoadList,
+                DEFAULT_AIRPORT_ICAO,
+                defaultAirportResponse
+            ));
+    }
+
+    /**
+     * Handler method called after data has loaded for the airline and aircraftTypeDefinitions datasets.
+     *
+     * Lifecycle method. Should be called only once on initialization
+     *
+     * @for App
+     * @method loadAirlinesAndAircraft
+     * @param {array>object>} airportLoadList
+     * @param {string} initialAirportIcao
+     * @param {object<string>} initialAirportResponse
+     */
+    loadAirlinesAndAircraft(airportLoadList, initialAirportIcao, initialAirportResponse) {
+        const airlineListPromise = $.getJSON('assets/airlines/airlines.json');
+        const aircraftListPromise = $.getJSON('assets/aircraft/aircraft.json');
+
         // This is provides a way to get async data from several sources in the app before anything else runs
-        // TODO: this is wrong. move this and make it less bad!
-        $.when(
-            $.getJSON(`assets/airports/${initialAirportToLoad.toLowerCase()}.json`),
-            $.getJSON('assets/airlines/airlines.json'),
-            $.getJSON('assets/aircraft/aircraft.json')
-        )
-            .done((airportResponse, airlineResponse, aircraftResponse) => {
-                this.setupChildren(
+        // we need to resolve data from two sources before the app can proceede. This data should always
+        // exist, if it doesn't, something has gone terribly wrong.
+        $.when(airlineListPromise, aircraftListPromise)
+            .done((airlineResponse, aircraftResponse) => {
+                this.setupChildrenHandler(
                     airportLoadList,
-                    airportResponse[0],
+                    initialAirportIcao,
+                    initialAirportResponse,
                     airlineResponse[0].airlines,
                     aircraftResponse[0].aircraft
                 );
-            })
-            .fail((jqXHR, textStatus, errorThrown) => {
-                console.error(textStatus);
             });
-
-        return this;
     }
 
     /**
      * Callback for a successful data load
      *
-     * An first load of data occurs on startup where we load the initial airport, airline definitions and
-     * aircraft definitiions. this method is called onComplete of that data load and is used to
+     * A first load of data occurs on startup where we load the initial airport, airline definitions and
+     * aircraft type definitiions. this method is called onComplete of that data load and is used to
      * instantiate various classes with the loaded data.
      *
-     * This method should run only on initial load of the app.
+     * This method will fire `.enable()` that will finish the initialization lifecycle and begine the game loop.
+     * Lifecycle method. Should be called only once on initialization
      *
      * @for App
      * @method setupChildren
@@ -114,9 +158,10 @@ export default class App {
      * @param airlineList {array}             List of all Airline definitions
      * @param aircraftTypeDefinitionList {array}  List of all Aircraft definitions
      */
-    setupChildren(airportLoadList, initialAirportData, airlineList, aircraftTypeDefinitionList) {
+    setupChildren(airportLoadList, initialAirportIcao, initialAirportData, airlineList, aircraftTypeDefinitionList) {
         this._appController.setupChildren(
             airportLoadList,
+            initialAirportIcao,
             initialAirportData,
             airlineList,
             aircraftTypeDefinitionList

src/assets/scripts/client/AppController.js

@@ -57,11 +57,11 @@ export default class AppController {
         this.canvasController = null;
 
         return this._init()
+            .setupHandlers()
             .enable();
     }
 
     /**
-     *
      * @for AppController
      * @method _init
      * @chainable
@@ -71,13 +71,26 @@ export default class AppController {
         return this;
     }
 
+    /**
+     * Create and bind handler methods
+     *
+     * @for AppController
+     * @method setupHandlers
+     * @chainable
+     */
+    setupHandlers() {
+        this.onAirportChangeHandler = this.onAirportChange.bind(this);
+
+        return this;
+    }
+
     /**
      * @for AppController
      * @method enable
      * @chainable
      */
     enable() {
-        this._eventBus.on(EVENT.AIRPORT_CHANGE, this.onAirportChange);
+        this._eventBus.on(EVENT.AIRPORT_CHANGE, this.onAirportChangeHandler);
 
         return this;
     }
@@ -88,9 +101,9 @@ export default class AppController {
      * @chainable
      */
     disable() {
-        this._eventBus.off(EVENT.AIRPORT_CHANGE, this.onAirportChange);
+        this._eventBus.off(EVENT.AIRPORT_CHANGE, this.onAirportChangeHandler);
 
-        return this;
+        return this.destroy();
     }
 
     /**
@@ -115,16 +128,20 @@ export default class AppController {
     }
 
     /**
+     * Create child instances and initialize singletons.
      *
+     * Called from `App.setupChildren()` only after all the required data has been retrieved
+     * This method will be called
      *
      * @for AppController
      * @method setupChildren
      * @param airportLoadList {array<object>}
+     * @param initialAirportIcao {string}
      * @param initialAirportData {object}
      * @param airlineList {array<object>}
      * @param aircraftTypeDefinitionList {array<object>}
      */
-    setupChildren(airportLoadList, initialAirportData, airlineList, aircraftTypeDefinitionList) {
+    setupChildren(airportLoadList, initialAirportIcao, initialAirportData, airlineList, aircraftTypeDefinitionList) {
         this.$canvasesElement = this.$element.find(SELECTORS.DOM_SELECTORS.CANVASES);
 
         // TODO: this entire method needs to be re-written. this is a temporary implemenation used to
@@ -138,7 +155,7 @@ export default class AppController {
         // The order in which the following classes are instantiated is extremely important. Changing
         // this order could break a lot of things. This interdependency is something we should
         // work on reducing in the future.
-        AirportController.init(initialAirportData, airportLoadList);
+        AirportController.init(initialAirportIcao, initialAirportData, airportLoadList);
 
         this.navigationLibrary = new NavigationLibrary(initialAirportData);
         this.airlineController = new AirlineController(airlineList);
@@ -237,7 +254,7 @@ export default class AppController {
 
 
     /**
-     * onChange callback fired from within the `AirportModel` when an airport is changed.
+     * `onChange` callback fired from within the `AirportModel` when an airport is changed.
      *
      * When an airport changes various classes need to clear and reset internal properties for
      * the new airport. this callback provides a way to orchestrate all that and send the classes

src/assets/scripts/client/airport/AirportController.js

@@ -64,10 +64,11 @@ class AirportController {
      *
      * @for AirportController
      * @method init
+     * @param InitialAirportIcao {string}
      * @param initialAirportData {object}
      * @param airportLoadList {array<object>}  List of airports to load
      */
-    init(initialAirportData, airportLoadList) {
+    init(initialAirportIcao, initialAirportData, airportLoadList) {
         this._airportListToLoad = airportLoadList;
 
         for (let i = 0; i < this._airportListToLoad.length; i++) {
@@ -76,7 +77,7 @@ class AirportController {
             this.airport_load(airport);
         }
 
-        this.ready(initialAirportData);
+        this.airport_set(initialAirportIcao, initialAirportData);
     }
 
     /**
@@ -107,8 +108,6 @@ class AirportController {
         const airportModel = new AirportModel({ icao, level, name, wip });
 
         this.airport_add(airportModel);
-
-        return airportModel;
     }
 
     /**
@@ -122,30 +121,6 @@ class AirportController {
         this.airports[airport.icao] = airport;
     }
 
-    /**
-     * Lifecycle method. Should run only once on App initialiazation
-     *
-     * @for AirportController
-     * @method ready
-     * @param initialAirportData {object}
-     */
-    ready(initialAirportData) {
-        let airportName = DEFAULT_AIRPORT_ICAO;
-
-        if (
-            _has(localStorage, STORAGE_KEY.ATC_LAST_AIRPORT) ||
-            _has(this.airports, _lowerCase(localStorage[STORAGE_KEY.ATC_LAST_AIRPORT]))
-        ) {
-            airportName = _lowerCase(localStorage[STORAGE_KEY.ATC_LAST_AIRPORT]);
-        }
-
-        if (airportName !== initialAirportData.icao.toLowerCase()) {
-            this.airport_set(airportName);
-        }
-
-        this.airport_set(airportName, initialAirportData);
-    }
-
     /**
      * Reset the instance
      *

test/airport/AirportController.spec.js

@@ -10,5 +10,5 @@ ava('throws when called to instantiate', (t) => {
 });
 
 ava('does not throw when .init() is called with initialization props', (t) => {
-    t.notThrows(() => AirportController.init(AIRPORT_JSON_KLAS_MOCK, AIRPORT_LOAD_LIST_MOCK));
+    t.notThrows(() => AirportController.init('klas', AIRPORT_JSON_KLAS_MOCK, AIRPORT_LOAD_LIST_MOCK));
 });

test/fixtures/airportFixtures.js

@@ -4,11 +4,11 @@ import StaticPositionModel from '../../src/assets/scripts/client/base/StaticPosi
 import { AIRPORT_JSON_KLAS_MOCK } from '../airport/_mocks/airportJsonMock';
 import { AIRPORT_LOAD_LIST_MOCK } from '../airport/_mocks/airportLoadListMocks';
 
-export const airportControllerFixture = () => AirportController.init(AIRPORT_JSON_KLAS_MOCK, AIRPORT_LOAD_LIST_MOCK);
+export const airportControllerFixture = () => AirportController.init('klas', AIRPORT_JSON_KLAS_MOCK, AIRPORT_LOAD_LIST_MOCK);
 export const resetAirportControllerFixture = () => AirportController.reset();
 
 export const airportControllerKlasFixture = AirportController;
-airportControllerKlasFixture.init(AIRPORT_JSON_KLAS_MOCK, AIRPORT_LOAD_LIST_MOCK);
+airportControllerKlasFixture.init('klas', AIRPORT_JSON_KLAS_MOCK, AIRPORT_LOAD_LIST_MOCK);
 airportControllerKlasFixture.airport_set('klas', AIRPORT_JSON_KLAS_MOCK);
 
 export const airportModelFixture = new AirportModel(AIRPORT_JSON_KLAS_MOCK);