Merge pull request #981 from theseyi/api-exception-handler

fixes deprecation update. adds api error handler. refactors misc sour…

wherehows-web/app/components/dataset-deprecation.ts

@@ -1,21 +1,14 @@
 import Component from '@ember/component';
-import { inject } from '@ember/service';
 import { getProperties, computed, set } from '@ember/object';
 import ComputedProperty, { oneWay } from '@ember/object/computed';
 import { baseCommentEditorOptions } from 'wherehows-web/constants';
-import Notifications, { NotificationEvent } from 'wherehows-web/services/notifications';
+import { action } from 'ember-decorators/object';
 
 export default class DatasetDeprecation extends Component {
   tagName = 'div';
 
   classNames = ['dataset-deprecation-toggle'];
 
-  /**
-   * References the application notifications service
-   * @memberof DatasetDeprecation
-   */
-  notifications = <ComputedProperty<Notifications>>inject();
-
   /**
    * Flag indicating that the dataset is deprecated or otherwise
    * @type {(null | boolean)}
@@ -68,11 +61,11 @@ export default class DatasetDeprecation extends Component {
   );
 
   /**
-   * The action to be completed when a save is initiated
-   * @type {Function}
+   * The external action to be completed when a save is initiated
+   * @type {(isDeprecated: boolean, updateDeprecationNode: string) => Promise<void>}
    * @memberof DatasetDeprecation
    */
-  onUpdateDeprecation: Function;
+  onUpdateDeprecation: (isDeprecated: boolean, updateDeprecationNode: string) => Promise<void> | void;
 
   editorOptions = {
     ...baseCommentEditorOptions,
@@ -81,42 +74,29 @@ export default class DatasetDeprecation extends Component {
     }
   };
 
-  actions = {
-    /**
-     * Toggles the boolean value of deprecatedAlias
-     */
-    toggleDeprecatedStatus(this: DatasetDeprecation) {
-      this.toggleProperty('deprecatedAlias');
-    },
+  /**
+   * Toggles the boolean value of deprecatedAlias
+   */
+  @action
+  toggleDeprecatedStatus(this: DatasetDeprecation) {
+    this.toggleProperty('deprecatedAlias');
+  }
 
-    /**
-     * Invokes the save action with the updated values for 
-     * deprecated and deprecationNote
-     */
-    async onSave(this: DatasetDeprecation) {
-      const { deprecatedAlias, deprecationNoteAlias, notifications: { notify } } = getProperties(this, [
-        'deprecatedAlias',
-        'deprecationNoteAlias',
-        'notifications'
-      ]);
-      const { onUpdateDeprecation } = this;
+  /**
+   * Invokes the save action with the updated values for
+   * deprecated and deprecationNote
+   * @return {Promise<void>}
+   */
+  @action
+  async onSave(this: DatasetDeprecation) {
+    const { deprecatedAlias, deprecationNoteAlias } = getProperties(this, ['deprecatedAlias', 'deprecationNoteAlias']);
+    const { onUpdateDeprecation } = this;
 
-      if (onUpdateDeprecation) {
-        const noteValue = deprecatedAlias ? deprecationNoteAlias : '';
+    if (onUpdateDeprecation) {
+      const noteValue = deprecatedAlias ? deprecationNoteAlias : '';
 
-        try {
-          await onUpdateDeprecation(deprecatedAlias, noteValue);
-          set(this, 'deprecationNoteAlias', noteValue);
-
-          notify(NotificationEvent.success, {
-            content: 'Successfully updated deprecation status'
-          });
-        } catch (e) {
-          notify(NotificationEvent.error, {
-            content: `An error occurred: ${e.message}`
-          });
-        }
-      }
+      await onUpdateDeprecation(deprecatedAlias, noteValue);
+      set(this, 'deprecationNoteAlias', noteValue);
     }
-  };
+  }
 }

wherehows-web/app/components/datasets/containers/dataset-compliance.ts

@@ -146,20 +146,20 @@ export default class DatasetComplianceContainer extends Component {
     setProperties(this, { schemaFieldNamesMappedToDataTypes, schemaless });
   });
 
-  @action
   /**
    * Persists the updates to the compliance policy on the remote host
    * @param {IComplianceInfo} complianceInfo
    * @return {Promise<void>}
    */
+  @action
   savePrivacyCompliancePolicy(complianceInfo: IComplianceInfo): Promise<void> {
     return saveDatasetComplianceByUrn(get(this, 'urn'), complianceInfo);
   }
 
-  @action
   /**
    * Resets the compliance information for the dataset with the previously persisted properties
    */
+  @action
   resetPrivacyCompliancePolicy() {
     get(this, 'getComplianceTask').perform();
   }

wherehows-web/app/components/datasets/containers/dataset-ownership.ts

@@ -68,12 +68,12 @@ export default class DatasetOwnershipContainer extends Component {
     set(this, 'ownerTypes', ownerTypes);
   });
 
-  @action
   /**
    * Persists the changes to the owners list
    * @param {Array<IOwner>} updatedOwners
    * @return {Promise<void>}
    */
+  @action
   saveOwnerChanges(updatedOwners: Array<IOwner>): Promise<void> {
     return updateDatasetOwnersByUrn(get(this, 'urn'), '', updatedOwners);
   }

wherehows-web/app/components/datasets/containers/dataset-properties.ts

@@ -1,7 +1,10 @@
 import Component from '@ember/component';
 import { get, setProperties } from '@ember/object';
+import ComputedProperty from '@ember/object/computed';
+import { inject } from '@ember/service';
 import { task } from 'ember-concurrency';
 import { action } from 'ember-decorators/object';
+import Notifications, { NotificationEvent } from 'wherehows-web/services/notifications';
 import { IDatasetView } from 'wherehows-web/typings/api/datasets/dataset';
 import { readDatasetByUrn } from 'wherehows-web/utils/api/datasets/dataset';
 import { updateDatasetDeprecationByUrn } from 'wherehows-web/utils/api/datasets/properties';
@@ -31,6 +34,13 @@ export default class DatasetPropertiesContainer extends Component {
    */
   properties: Array<never> = [];
 
+  /**
+   * References the application notifications service
+   * @memberof DatasetPropertiesContainer
+   * @type {ComputedProperty<Notifications>}
+   */
+  notifications = <ComputedProperty<Notifications>>inject();
+
   constructor() {
     super(...arguments);
     this.deprecationNote || (this.deprecationNote = '');
@@ -55,15 +65,33 @@ export default class DatasetPropertiesContainer extends Component {
     setProperties(this, { deprecated, deprecationNote });
   });
 
-  @action
   /**
    * Persists the changes to the dataset deprecation properties upstream
    * @param {boolean} isDeprecated
    * @param {string} updatedDeprecationNote
    * @return {Promise<void>}
    */
-  async updateDeprecation(isDeprecated: boolean, updatedDeprecationNote: string): Promise<void> {
-    await updateDatasetDeprecationByUrn(get(this, 'urn'), isDeprecated, updatedDeprecationNote);
-    get(this, 'getDeprecationPropertiesTask').perform();
+  @action
+  async updateDeprecation(
+    this: DatasetPropertiesContainer,
+    isDeprecated: boolean,
+    updatedDeprecationNote: string
+  ): Promise<void> {
+    const { notify } = get(this, 'notifications');
+
+    try {
+      await updateDatasetDeprecationByUrn(get(this, 'urn'), isDeprecated, updatedDeprecationNote);
+
+      notify(NotificationEvent.success, {
+        content: 'Successfully updated deprecation status'
+      });
+    } catch (e) {
+      notify(NotificationEvent.error, {
+        content: `An error occurred: ${e.message}`
+      });
+    } finally {
+      // set current state
+      get(this, 'getDeprecationPropertiesTask').perform();
+    }
   }
 }

wherehows-web/app/components/datasets/containers/datasets-list.js

@@ -1,23 +0,0 @@
-import Component from '@ember/component';
-import { connect } from 'ember-redux';
-
-/**
- * A Selector function that takes the Redux Store and applies
- *   store state to props
- * @param {Object} datasets is the slice of the store containing datasets
- *   and related state
- * @return {{datasets: (any[]|Array), isFetching: boolean}}
- */
-const stateToComputed = ({ datasets }) => {
-  const { byPage, byId, currentPage, isFetching = false } = datasets;
-  // List of datasets for the current Page
-  const pagedDatasetIds = byPage[currentPage] || [];
-
-  return {
-    // Takes the normalized list of ids and maps to dataset objects
-    datasets: pagedDatasetIds.map(datasetId => byId[datasetId]),
-    isFetching
-  };
-};
-
-export default connect(stateToComputed)(Component.extend({}));

wherehows-web/app/constants/errors/errors.ts

@@ -0,0 +1,14 @@
+import { ApiStatusNumber } from 'wherehows-web/utils/api/shared';
+
+/**
+ * Returns a default msg for a given status
+ * @param {ApiStatusNumber} status
+ * @returns {string}
+ */
+const apiErrorStatusMessage = (status: ApiStatusNumber): string =>
+  (<{ [prop: number]: string }>{
+    [ApiStatusNumber.NotFound]: 'Could not find the requested resource',
+    [ApiStatusNumber.InternalServerError]: 'An error occurred with the server'
+  })[status];
+
+export { apiErrorStatusMessage };

wherehows-web/app/utils/api/datasets/properties.ts

@@ -25,8 +25,6 @@ interface IPropertyItem {
  */
 const datasetPropertiesUrlById = (id: number) => `${datasetUrlById(id)}/properties`;
 
-const datasetDeprecationUrlById = (id: number) => `${datasetUrlById(id)}/deprecate`;
-
 /**
  * Returns the url for a dataset deprecation endpoint by urn
  * @param {string} urn
@@ -143,7 +141,7 @@ const readNonPinotProperties = async (id: number): Promise<Array<IPropertyItem>>
 };
 
 /**
- * Describes the inteface of object returned from the api request to get pinot properties
+ * Describes the interface of object returned from the api request to get pinot properties
  * @interface IDatasetSamplesAndColumns
  */
 interface IDatasetSamplesAndColumns {
@@ -184,26 +182,6 @@ const readPinotProperties = async (id: number) => {
   }
 };
 
-/**
- * Updates the properties on the dataset for deprecation
- * @param {number} id the id of the dataset
- * @param {boolean} deprecated flag indicating deprecation
- * @param {string} [deprecationNote=''] optional note accompanying deprecation change
- */
-const updateDatasetDeprecation = async (id: number, deprecated: boolean, deprecationNote: string = '') => {
-  const { status, msg } = await putJSON<{ status: ApiStatus; msg: string }>({
-    url: datasetDeprecationUrlById(id),
-    data: {
-      deprecated,
-      deprecationNote
-    }
-  });
-
-  if (status !== ApiStatus.OK) {
-    throw new Error(msg);
-  }
-};
-
 /**
  * Persists the changes to a datasets deprecation properties by urn
  * @param {string} urn
@@ -211,24 +189,13 @@ const updateDatasetDeprecation = async (id: number, deprecated: boolean, depreca
  * @param {string} deprecationNote
  * @return {Promise<void>}
  */
-const updateDatasetDeprecationByUrn = (
-  urn: string,
-  deprecated: boolean,
-  deprecationNote: string = ''
-): Promise<void> => {
-  return putJSON<void>({
+const updateDatasetDeprecationByUrn = (urn: string, deprecated: boolean, deprecationNote: string = ''): Promise<void> =>
+  putJSON<void>({
     url: datasetDeprecationUrlByUrn(urn),
     data: {
       deprecated,
       deprecationNote
     }
   });
-};
 
-export {
-  readDatasetProperties,
-  readNonPinotProperties,
-  readPinotProperties,
-  updateDatasetDeprecation,
-  updateDatasetDeprecationByUrn
-};
+export { readDatasetProperties, readNonPinotProperties, readPinotProperties, updateDatasetDeprecationByUrn };

wherehows-web/app/utils/api/errors/errors.ts

@@ -0,0 +1,21 @@
+import { apiErrorStatusMessage } from 'wherehows-web/constants/errors/errors';
+
+/**
+ * Wraps a Response object, pass through json response if no api error,
+ * otherwise raise exception with error message
+ * @template T
+ * @param {Response} response
+ * @returns {Promise<T>}
+ */
+const throwIfApiError = async <T>(response: Response): Promise<T> => {
+  const { status, ok } = response;
+
+  if (!ok) {
+    const { msg = apiErrorStatusMessage(status) } = await response.json();
+    throw new Error(msg);
+  }
+
+  return response.json();
+};
+
+export { throwIfApiError };

wherehows-web/app/utils/api/fetcher.ts

@@ -1,4 +1,5 @@
 import fetch from 'fetch';
+import { throwIfApiError } from 'wherehows-web/utils/api/errors/errors';
 
 /**
  * Describes the attributes on the fetch configuration object
@@ -10,7 +11,7 @@ interface FetchConfig {
 }
 
 /**
- * Desribes the available options on an option bag to be passed into a fetch call
+ * Describes the available options on an option bag to be passed into a fetch call
  * @interface IFetchOptions
  */
 interface IFetchOptions {
@@ -38,11 +39,11 @@ const withBaseFetchHeaders = (headers: FetchConfig['headers']): { headers: Fetch
  * Sends a HTTP request and resolves with the JSON response
  * @template T
  * @param {string} url the url for the endpoint to request a response from
- * @param {object} fetchConfig 
- * @returns {Promise<T>} 
+ * @param {object} fetchConfig
+ * @returns {Promise<T>}
  */
 const json = <T>(url: string = '', fetchConfig: IFetchOptions = {}): Promise<T> =>
-  fetch(url, fetchConfig).then<T>(response => response.json());
+  fetch(url, fetchConfig).then<T>(response => throwIfApiError(response));
 
 /**
  * Conveniently gets a JSON response using the fetch api
@@ -59,8 +60,8 @@ const getJSON = <T>(config: FetchConfig): Promise<T> => {
 /**
  * Initiates a POST request using the Fetch api
  * @template T
- * @param {FetchConfig} config 
- * @returns {Promise<T>} 
+ * @param {FetchConfig} config
+ * @returns {Promise<T>}
  */
 const postJSON = <T>(config: FetchConfig): Promise<T> => {
   const requestBody = config.data ? { body: JSON.stringify(config.data) } : {};
@@ -120,22 +121,4 @@ const getHeaders = async (config: FetchConfig): Promise<Headers> => {
   throw new Error(statusText);
 };
 
-/**
- * Wraps a request Promise, pass-through response if successful, otherwise handle the error and rethrow if not api error
- * @template T
- * @param {Promise<T>} fetcher the api request to wrap
- * @param {T} defaultValue
- * @returns {Promise<T|null>}
- */
-const fetchAndHandleIfApiError = async <T>(fetcher: Promise<T>, defaultValue: T): Promise<T | null> => {
-  let result = typeof defaultValue === 'undefined' ? null : defaultValue;
-  try {
-    result = await fetcher;
-  } catch (e) {
-    // TODO: if error is an api error, display notification and allow default return
-    // otherwise throw
-  }
-  return result;
-};
-
-export { getJSON, postJSON, deleteJSON, putJSON, getHeaders, fetchAndHandleIfApiError };
+export { getJSON, postJSON, deleteJSON, putJSON, getHeaders };

wherehows-web/app/utils/api/shared.ts

@@ -21,3 +21,13 @@ export enum ApiStatus {
   FAILED = 'failed',
   ERROR = 'error'
 }
+
+/**
+ * Enumerates the currently available Api statuses
+ * @type {number}
+ */
+export enum ApiStatusNumber {
+  NotFound = 404,
+  UnAuthorized = 401,
+  InternalServerError = 500
+}

wherehows-web/tests/integration/components/dataset-deprecation-test.js

@@ -5,13 +5,7 @@ import hbs from 'htmlbars-inline-precompile';
 import { run } from '@ember/runloop';
 
 moduleForComponent('dataset-deprecation', 'Integration | Component | dataset deprecation', {
-  integration: true,
-
-  beforeEach() {
-    this.register('service:notifications', notificationsStub);
-
-    this.inject.service('notifications');
-  }
+  integration: true
 });
 
 test('it renders', function(assert) {

wherehows-web/tests/unit/utils/api/errors/errors-test.js

@@ -0,0 +1,15 @@
+import { throwIfApiError } from 'wherehows-web/utils/api/errors/errors';
+import { module, test } from 'qunit';
+
+module('Unit | Utility | api/errors/errors');
+
+test('throwIfApiError exists', function(assert) {
+  assert.ok(typeof throwIfApiError === 'function', 'throwIfApiError exists as a function');
+});
+
+test('throwIfApiError returns a Promise / thennable', function(assert) {
+  assert.ok(
+    typeof throwIfApiError({ status: 200, ok: true, json: () => Promise.resolve() }).then === 'function',
+    'invocation returns a Promise object / thennable'
+  );
+});

wherehows-web/tests/unit/utils/api/fetcher-test.js

@@ -1,11 +1,4 @@
-import {
-  getJSON,
-  postJSON,
-  deleteJSON,
-  putJSON,
-  getHeaders,
-  fetchAndHandleIfApiError
-} from 'wherehows-web/utils/api/fetcher';
+import { getJSON, postJSON, deleteJSON, putJSON, getHeaders } from 'wherehows-web/utils/api/fetcher';
 import { module, test } from 'qunit';
 import sinon from 'sinon';
 
@@ -20,7 +13,7 @@ module('Unit | Utility | api/fetcher', {
 });
 
 test('each http request function exists', function(assert) {
-  [getJSON, postJSON, deleteJSON, putJSON, getHeaders, fetchAndHandleIfApiError].forEach(httpRequest =>
+  [getJSON, postJSON, deleteJSON, putJSON, getHeaders].forEach(httpRequest =>
     assert.ok(typeof httpRequest === 'function', `${httpRequest} is a function`)
   );
 });