Fixes: beta.9 (#167)

closes #163, 165
* Critical: data service update should send entity data, not Update<T> #165
* Allow Angular peer dependencies to update within v.6
See ChangeLog

lib/CHANGELOG.md

@@ -1,6 +1,39 @@
 # Angular ngrx-data library ChangeLog
 
-<a id="6.0.2-beta.7"></a>
+<a id="6.0.2-beta.9"></a>
+
+# 6.0.2-beta.9 (2018-07-02)
+
+Fixes #163. Allows Angular peer dependency to change as long as it is version 6.x.x
+
+```
+ -   "@angular/core": "6.0.0",
+ -   "@angular/common": "6.0.0",
+ +   "@angular/core": "^6.0.0",
+ +   "@angular/common": "^6.0.0",
+```
+
+Fixes issue #165 related to the breaking change in Beta.7 in which the
+return type of `EntityCollectionDataService.update()` changed:
+
+```
+-  update(update: Update<T>): Observable<Update<T>>;
++  update(update: Update<T>): Observable<T>;
+```
+
+The re-implementation of `DefaultDataService` accidentally sent the `Update<T>`
+to the server instead of just the entity data, T.
+
+This release corrects that error and adds confirming tests.
+
+It incidentally changes the handling of the changed indicator ()
+that is created by the `EntityEffects` after the update succeeds.
+
+* the indicator property name was renamed from `unchanged` to `changed`.
+* the `EntityEffects` _always sets it_ after update (used to set it only when changed).
+* adjusted `EntityChangeTracker` which consumed the `changed` property .
+
+<a id="6.0.2-beta.8"></a>
 
 # 6.0.2-beta.8 (2018-06-27)
 
@@ -616,6 +649,20 @@ The need for separating these concerns became apparent as we enriched the action
 This change breaks apps that registered collection reducers directly with the former `_EntityReducerFactory_`.
 Resolve by importing `EntityCollectionReducerRegistry` instead and calling the same registration methods on it.
 
+#### _EntityCollectionDataService.update()_ signature changed
+
+The return type of `EntityCollectionDataService.update()` changed,
+affecting implementation of `DefaultDataService.ts` and
+any override of that method in an application derived class
+(see issue #165).
+
+```
+-  update(update: Update<T>): Observable<Update<T>>;
++  update(update: Update<T>): Observable<T>;
+```
+
+This lead to a bad bug in the `DefaultDataService.update()`, corrected in [Beta 9](#6.0.2-beta.9)
+
 #### Renamed _DefaultEntityCollectionServiceFactory_ to _EntityCollectionServiceFactoryBase_.
 
 Renamed for consistence with other members of the _EntityServices_ family.

lib/package.json

@@ -1,11 +1,11 @@
 {
   "name": "ngrx-data",
-  "version": "6.0.2-beta.8",
+  "version": "6.0.2-beta.9",
   "repository": "https://github.com/johnpapa/angular-ngrx-data.git",
   "license": "MIT",
   "peerDependencies": {
-    "@angular/core": "6.0.0",
-    "@angular/common": "6.0.0",
+    "@angular/core": "^6.0.0",
+    "@angular/common": "^6.0.0",
     "@ngrx/effects": "^6.0.0",
     "@ngrx/entity": "^6.0.0",
     "@ngrx/store": "^6.0.0",

lib/src/actions/entity-action-guard.ts

@@ -1,5 +1,6 @@
 import { EntityAction } from './entity-action';
 import { IdSelector, Update } from '../utils/ngrx-entity-models';
+import { UpdateResponseData } from '../actions/update-response-data';
 
 /**
  * Guard methods that ensure EntityAction payload is as expected.
@@ -95,6 +96,36 @@ export class EntityActionGuard {
     return data;
   }
 
+  /** Throw if the action payload is not an update response with a valid key (id) */
+  mustBeUpdateResponse<T = any>(action: EntityAction<UpdateResponseData<T>>): UpdateResponseData<T> {
+    const data = this.extractData(action);
+    if (!data) {
+      this.throwError(action, `should be a single entity update`);
+    }
+    const { id, changes } = data;
+    const id2 = this.selectId(changes);
+    if (this.isNotKeyType(id) || this.isNotKeyType(id2)) {
+      this.throwError(action, `has a missing or invalid entity key (id)`);
+    }
+    return data;
+  }
+
+  /** Throw if the action payload is not an array of update responses with valid keys (ids) */
+  mustBeUpdateResponses<T = any>(action: EntityAction<UpdateResponseData<any>[]>): UpdateResponseData<T>[] {
+    const data = this.extractData(action);
+    if (!Array.isArray(data)) {
+      this.throwError(action, `should be an array of entity updates`);
+    }
+    data.forEach((item, i) => {
+      const { id, changes } = item;
+      const id2 = this.selectId(changes);
+      if (this.isNotKeyType(id) || this.isNotKeyType(id2)) {
+        this.throwError(action, `, item ${i + 1}, has a missing or invalid entity key (id)`);
+      }
+    });
+    return data;
+  }
+
   private extractData<T>(action: EntityAction<T>) {
     return action.payload && action.payload.data;
   }

lib/src/actions/update-response-data.ts

@@ -0,0 +1,19 @@
+/**
+ * Data returned in an EntityAction from the EntityEffects for SAVE_UPDATE_ONE_SUCCESS.
+ * Effectively extends Update<T> with a 'changed' flag.
+ * The is true if the server sent back changes to the entity data after update.
+ * Such changes must be in the entity data in changes property.
+ * Default is false (server did not return entity data; assume it changed nothing).
+ * See EntityEffects.
+ */
+export interface UpdateResponseData<T> {
+  /** Original key (id) of the entity */
+  id: number | string;
+  /** Entity update data. Should include the key (original or changed) */
+  changes: Partial<T>;
+  /**
+   * Whether the server made additional changes after processing the update.
+   * Such additional changes should be in the 'changes' object.
+   */
+  changed: boolean;
+}

lib/src/dataservices/default-data.service.spec.ts

@@ -97,6 +97,8 @@ describe('DefaultDataService', () => {
       const req = httpTestingController.expectOne(heroesUrl);
       expect(req.request.method).toEqual('GET');
 
+      expect(req.request.body).toBeNull('should not send data');
+
       // Respond with the mock heroes
       req.flush(expectedHeroes);
     });
@@ -163,6 +165,8 @@ describe('DefaultDataService', () => {
       // One request to GET hero from expected URL
       const req = httpTestingController.expectOne(heroUrlId1);
 
+      expect(req.request.body).toBeNull('should not send data');
+
       // Respond with the expected hero
       req.flush(expectedHero);
     });
@@ -207,6 +211,8 @@ describe('DefaultDataService', () => {
       const req = httpTestingController.expectOne(heroesUrl + '?name=B');
       expect(req.request.method).toEqual('GET');
 
+      expect(req.request.body).toBeNull('should not send data');
+
       // Respond with the mock heroes
       req.flush(expectedHeroes);
     });
@@ -256,12 +262,15 @@ describe('DefaultDataService', () => {
 
     it('should return expected hero with id', () => {
       expectedHero = { id: 42, name: 'A' };
+      const heroData: Hero = { id: undefined, name: 'A' };
 
-      service.add({ name: 'A' } as Hero).subscribe(hero => expect(hero).toEqual(expectedHero, 'should return expected hero'), fail);
+      service.add(heroData).subscribe(hero => expect(hero).toEqual(expectedHero, 'should return expected hero'), fail);
 
       // One request to POST hero from expected URL
       const req = httpTestingController.expectOne(r => r.method === 'POST' && r.url === heroUrl);
 
+      expect(req.request.body).toEqual(heroData, 'should send entity data');
+
       // Respond with the expected hero
       req.flush(expectedHero);
     });
@@ -285,6 +294,8 @@ describe('DefaultDataService', () => {
       // One request to DELETE hero from expected URL
       const req = httpTestingController.expectOne(r => r.method === 'DELETE' && r.url === heroUrlId1);
 
+      expect(req.request.body).toBeNull('should not send data');
+
       // Respond with empty nonsense object
       req.flush({});
     });
@@ -343,6 +354,8 @@ describe('DefaultDataService', () => {
       // One request to PUT hero from expected URL
       const req = httpTestingController.expectOne(r => r.method === 'PUT' && r.url === heroUrlId1);
 
+      expect(req.request.body).toEqual(updateArg.changes, 'should send update entity data');
+
       // Respond with the expected hero
       req.flush(expectedHero);
     });

lib/src/dataservices/default-data.service.ts

@@ -5,10 +5,8 @@ import { Observable, of, throwError } from 'rxjs';
 import { catchError, delay, map, tap, timeout } from 'rxjs/operators';
 
 import { DataServiceError } from './data-service-error';
-import { HttpMethods, QueryParams, RequestData } from './interfaces';
+import { EntityCollectionDataService, HttpMethods, QueryParams, RequestData } from './interfaces';
 import { HttpUrlGenerator, EntityHttpResourceUrls } from './http-url-generator';
-
-import { EntityCollectionDataService } from './entity-data.service';
 import { Update } from '../utils/ngrx-entity-models';
 
 // Pass the observable straight through
@@ -108,7 +106,7 @@ export class DefaultDataService<T> implements EntityCollectionDataService<T> {
 
   update(update: Update<T>): Observable<T> {
     const id = update && update.id;
-    const updateOrError = id == null ? new Error(`No "${this.entityName}" update data or id`) : update;
+    const updateOrError = id == null ? new Error(`No "${this.entityName}" update data or id`) : update.changes;
     return this.execute('PUT', this.entityUrl + id, updateOrError);
   }
 

lib/src/dataservices/entity-data.service.spec.ts

@@ -11,7 +11,7 @@ import { DefaultDataService, DefaultDataServiceFactory } from './default-data.se
 import { HttpUrlGenerator, EntityHttpResourceUrls } from './http-url-generator';
 
 import { EntityDataService } from './entity-data.service';
-import { EntityCollectionDataService } from './entity-data.service';
+import { EntityCollectionDataService } from './interfaces';
 import { QueryParams } from './interfaces';
 import { Update } from '../utils/ngrx-entity-models';
 

lib/src/dataservices/entity-data.service.ts

@@ -1,23 +1,7 @@
 import { Injectable } from '@angular/core';
 
-import { Observable } from 'rxjs';
-
-import { EntityAction } from '../actions/entity-action';
+import { EntityCollectionDataService } from './interfaces';
 import { DefaultDataServiceFactory } from './default-data.service';
-import { HttpUrlGenerator } from './http-url-generator';
-import { QueryParams } from './interfaces';
-import { Update } from '../utils/ngrx-entity-models';
-
-/** A service that performs REST-like HTTP data operations */
-export interface EntityCollectionDataService<T> {
-  readonly name: string;
-  add(entity: T): Observable<T>;
-  delete(id: number | string): Observable<number | string>;
-  getAll(): Observable<T[]>;
-  getById(id: any): Observable<T>;
-  getWithQuery(params: QueryParams | string): Observable<T[]>;
-  update(update: Update<T>): Observable<T>;
-}
 
 @Injectable()
 export class EntityDataService {
@@ -54,10 +38,7 @@ export class EntityDataService {
    *   registerService('Hero', myHeroDataService);
    *   registerService('Villain', myVillainDataService);
    */
-  registerService<T>(
-    entityName: string,
-    service: EntityCollectionDataService<T>
-  ) {
+  registerService<T>(entityName: string, service: EntityCollectionDataService<T>) {
     this.services[entityName.trim()] = service;
   }
 
@@ -71,9 +52,7 @@ export class EntityDataService {
    *     Villain: myVillainDataService
    *   });
    */
-  registerServices(services: {
-    [name: string]: EntityCollectionDataService<any>;
-  }) {
+  registerServices(services: { [name: string]: EntityCollectionDataService<any> }) {
     this.services = { ...this.services, ...services };
   }
 }

lib/src/dataservices/interfaces.ts

@@ -1,3 +1,17 @@
+import { Observable } from 'rxjs';
+import { Update } from '../utils/ngrx-entity-models';
+
+/** A service that performs REST-like HTTP data operations for an entity collection */
+export interface EntityCollectionDataService<T> {
+  readonly name: string;
+  add(entity: T): Observable<T>;
+  delete(id: number | string): Observable<number | string>;
+  getAll(): Observable<T[]>;
+  getById(id: any): Observable<T>;
+  getWithQuery(params: QueryParams | string): Observable<T[]>;
+  update(update: Update<T>): Observable<T>;
+}
+
 export type HttpMethods = 'DELETE' | 'GET' | 'POST' | 'PUT';
 
 export interface RequestData {

lib/src/dispatchers/entity-dispatcher-base.ts

@@ -3,6 +3,7 @@ import { Action, createSelector, select, Store } from '@ngrx/store';
 import { Observable, of, throwError } from 'rxjs';
 import { filter, first, map, mergeMap, shareReplay, withLatestFrom } from 'rxjs/operators';
 
+import { UpdateResponseData } from '../actions/update-response-data';
 import { CorrelationIdGenerator } from '../utils/correlation-id-generator';
 import { EntityDispatcherDefaultOptions } from './entity-dispatcher-default-options';
 import { defaultSelectId, toUpdateFactory } from '../utils/utilities';
@@ -15,7 +16,7 @@ import { EntityCollection } from '../reducers/entity-collection';
 import { EntityCommands } from './entity-commands';
 import { EntityDispatcher, PersistanceCanceled } from './entity-dispatcher';
 import { EntityOp, OP_ERROR, OP_SUCCESS } from '../actions/entity-op';
-import { IdSelector, Update, UpdateData } from '../utils/ngrx-entity-models';
+import { IdSelector, Update } from '../utils/ngrx-entity-models';
 import { MergeStrategy } from '../actions/merge-strategy';
 import { QueryParams } from '../dataservices/interfaces';
 
@@ -274,7 +275,7 @@ export class EntityDispatcherBase<T> implements EntityDispatcher<T> {
       this.guard.mustBeEntity(action);
     }
     this.dispatch(action);
-    return this.getResponseData$<UpdateData<T>>(options.correlationId).pipe(
+    return this.getResponseData$<UpdateResponseData<T>>(options.correlationId).pipe(
       // Use the update entity data id to get the entity from the collection
       // as might be different from the entity returned from the server
       // because the id changed or there are unsaved changes.

lib/src/effects/entity-effects.marbles.spec.ts

@@ -7,20 +7,18 @@ import { Observable, of, Subject } from 'rxjs';
 import { Actions } from '@ngrx/effects';
 import { provideMockActions } from '@ngrx/effects/testing';
 
+import { DataServiceError, EntityActionDataServiceError } from '../dataservices/data-service-error';
 import { EntityAction } from '../actions/entity-action';
 import { EntityActionFactory } from '../actions/entity-action-factory';
+import { EntityCollectionDataService } from '../dataservices/interfaces';
+import { EntityDataService } from '../dataservices/entity-data.service';
 import { EntityOp, makeErrorOp } from '../actions/entity-op';
-import { MergeStrategy } from '../actions/merge-strategy';
-
-import { EntityCollectionDataService, EntityDataService } from '../dataservices/entity-data.service';
-import { DataServiceError, EntityActionDataServiceError } from '../dataservices/data-service-error';
-import { PersistenceResultHandler, DefaultPersistenceResultHandler } from '../dataservices/persistence-result-handler.service';
 import { HttpMethods } from '../dataservices/interfaces';
-
-import { EntityEffects, ENTITY_EFFECTS_SCHEDULER } from './entity-effects';
-
 import { Logger } from '../utils/interfaces';
+import { PersistenceResultHandler, DefaultPersistenceResultHandler } from '../dataservices/persistence-result-handler.service';
 import { Update } from '../utils/ngrx-entity-models';
+
+import { EntityEffects, ENTITY_EFFECTS_SCHEDULER } from './entity-effects';
 import { TestHotObservable } from 'jasmine-marbles/src/test-observables';
 
 //////// Tests begin ////////
@@ -243,9 +241,10 @@ describe('EntityEffects (marble testing)', () => {
   it('should return a SAVE_UPDATE_ONE_SUCCESS (Optimistic) with the hero on success', () => {
     const updateEntity = { id: 1, name: 'A' };
     const update = { id: 1, changes: updateEntity } as Update<Hero>;
+    const updateResponse = { ...update, changed: true };
 
     const action = entityActionFactory.create('Hero', EntityOp.SAVE_UPDATE_ONE, update, { isOptimistic: true });
-    const completion = entityActionFactory.create('Hero', EntityOp.SAVE_UPDATE_ONE_SUCCESS, update, { isOptimistic: true });
+    const completion = entityActionFactory.create('Hero', EntityOp.SAVE_UPDATE_ONE_SUCCESS, updateResponse, { isOptimistic: true });
 
     actions = hot('-a---', { a: action });
     // delay the response 3 frames
@@ -259,9 +258,10 @@ describe('EntityEffects (marble testing)', () => {
   it('should return a SAVE_UPDATE_ONE_SUCCESS (Pessimistic) with the hero on success', () => {
     const updateEntity = { id: 1, name: 'A' };
     const update = { id: 1, changes: updateEntity } as Update<Hero>;
+    const updateResponse = { ...update, changed: true };
 
     const action = entityActionFactory.create('Hero', EntityOp.SAVE_UPDATE_ONE, update);
-    const completion = entityActionFactory.create('Hero', EntityOp.SAVE_UPDATE_ONE_SUCCESS, update);
+    const completion = entityActionFactory.create('Hero', EntityOp.SAVE_UPDATE_ONE_SUCCESS, updateResponse);
 
     actions = hot('-a---', { a: action });
     // delay the response 3 frames