Skip to content

Commit

Permalink
BREAKING ModelService never merge mutation results into input #9844
Browse files Browse the repository at this point in the history 
This made it hard to work with readonly input coming from Apollo, and it
can easily be added in our projects in the very few places that could
benefit from that mechanic.
  • Loading branch information
@PowerKiKi
PowerKiKi committed Mar 24, 2024
1 parent ffad3e0 commit 0723d9f
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 44 deletions.
73 changes: 47 additions & 26 deletions projects/natural/src/lib/services/abstract-model.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,23 @@ describe('NaturalAbstractModelService', () => {
result?.subscribe(v => (actual = v));
tick(1000);

// The input must have been mutated with whatever is coming from API
expect(actual).toBe(object);
// The input must have been left untouched
expect(object).toEqual({
slug: 'foo',
blog: '123',
});

// The result must be merged into the original object
expect(Object.keys(object)).toContain('id');
expect(Object.keys(object)).toContain('creationDate');
// The result is straight from API, wihtout any modification on the way
expect(actual).toEqual({
id: '456',
slug: 'test string',
creationDate: 'test string',
__typename: 'Post',
});

// The result must **not** be merged into the original object
expect(Object.keys(object)).not.toContain('id');
expect(Object.keys(object)).not.toContain('creationDate');
}));

it('should not create with observable', fakeAsync(() => {
Expand Down Expand Up @@ -216,34 +227,34 @@ describe('NaturalAbstractModelService', () => {
}));

it('should create or update', fakeAsync(() => {
const object: PostInput & {id?: string} = {
const input: PostInput & {id?: string} = {
slug: 'foo',
blog: '123',
};

// Create, should receive temporary id immediately
const creation = service.createOrUpdate(object, true);
creation.subscribe();
let creationResult: any;
const creation = service.createOrUpdate(input, true);
creation.subscribe(v => (creationResult = v));

// After create, should be usual object after creation
tick();
expect(object.id).toEqual('456');
expect('updateDate' in object).toBeFalse();
expect(creationResult.id).toEqual('456');
expect('updateDate' in creationResult).toBeFalse();

// Create or update again
const update = service.createOrUpdate(object, true);
expect('updateDate' in object).toBeFalse();
update.subscribe();
let updateResult: any;
const update = service.createOrUpdate(creationResult, true);
update.subscribe(v => (updateResult = v));

// should show created + updated objects merged
tick();
expect('updateDate' in object).toBeTrue();
expect('updateDate' in updateResult).toBeTrue();

flush();
}));

it('should wait for the first creation, then update the object', fakeAsync(() => {
const object: PostInput & {id?: string} = {
const input: PostInput = {
slug: 'foo',
blog: '123',
};
Expand All @@ -252,24 +263,34 @@ describe('NaturalAbstractModelService', () => {
let repeatedResult: any = null;

// Create, should be cached
const creation = service.createOrUpdate(object, true);
const creation = service.createOrUpdate(input, true);
creation.subscribe(res => (result = res));

// Repeated create should wait for the first creation, then update the object
const repeatedCreation = service.createOrUpdate(object, true);
const repeatedCreation = service.createOrUpdate(input, true);
repeatedCreation.subscribe(res => (repeatedResult = res));

tick(5000);

// After create, both result must be the exact same object (and not a clone)
expect(result).not.toBeNull();
expect(result.id).toEqual('456');
expect('updateDate' in result).toBeTrue();
expect(repeatedResult).not.toBeNull();
expect(repeatedResult).toBe(result);
// After create, both result must be similar, but not equals
expect(result).toEqual({
id: '456',
slug: 'test string',
creationDate: 'test string',
__typename: 'Post',
});
expect(repeatedResult).toEqual({
id: '456',
slug: 'test string',
updateDate: 'test string',
__typename: 'Post',
});

// After create, object should be equivalent to result
expect(object).toEqual(result);
// After create, input is **not** changed
expect(input).toEqual({
slug: 'foo',
blog: '123',
});
}));

it('should count existing values', fakeAsync(() => {
Expand Down
25 changes: 7 additions & 18 deletions projects/natural/src/lib/services/abstract-model.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ import {Apollo, gql, MutationResult} from 'apollo-angular';
import {FetchResult, NetworkStatus, WatchQueryFetchPolicy} from '@apollo/client/core';
import {AbstractControl, AsyncValidatorFn, UntypedFormControl, UntypedFormGroup, ValidatorFn} from '@angular/forms';
import {DocumentNode} from 'graphql';
import {defaults, merge, mergeWith, pick} from 'lodash-es';
import {defaults, merge, pick} from 'lodash-es';
import {catchError, combineLatest, EMPTY, first, from, Observable, of, OperatorFunction} from 'rxjs';
import {debounceTime, filter, map, shareReplay, startWith, switchMap, takeWhile, tap} from 'rxjs/operators';
import {NaturalQueryVariablesManager, QueryVariables} from '../classes/query-variable-manager';
import {Literal} from '../types/types';
import {makePlural, mergeOverrideArray, relationsToIds, upperCaseFirstLetter} from '../classes/utility';
import {makePlural, relationsToIds, upperCaseFirstLetter} from '../classes/utility';
import {PaginatedData} from '../classes/data-source';
import {NaturalDebounceService} from './debounce.service';
import {ApolloQueryResult} from '@apollo/client/core/types';
Expand Down Expand Up @@ -285,10 +285,7 @@ export abstract class NaturalAbstractModelService<
* Uses regular update/updateNow and create methods.
* Used mainly when editing multiple objects in same controller (like in editable arrays)
*/
public createOrUpdate(
object: Vcreate['input'] | WithId<Vupdate['input']>,
now = false,
): Observable<Tcreate | Tupdate> {
public createOrUpdate(object: Vcreate['input'] | Vupdate['input'], now = false): Observable<Tcreate | Tupdate> {
this.throwIfObservable(object);
this.throwIfNotQuery(this.createMutation);
this.throwIfNotQuery(this.updateMutation);
Expand All @@ -297,8 +294,8 @@ export abstract class NaturalAbstractModelService<
const pendingCreation = this.creatingCache.get(object);
if (pendingCreation) {
return pendingCreation.pipe(
switchMap(() => {
return this.update(object as WithId<Vupdate['input']>);
switchMap(created => {
return this.update({id: (created as WithId<Tcreate>).id, ...object});
}),
);
}
Expand Down Expand Up @@ -348,13 +345,7 @@ export abstract class NaturalAbstractModelService<
.pipe(
map(result => {
this.apollo.client.reFetchObservableQueries();
const newObject = this.mapCreation(result);

if (newObject) {
return mergeWith(object, newObject, mergeOverrideArray);
} else {
return newObject;
}
return this.mapCreation(result);
}),
);
}
Expand Down Expand Up @@ -395,9 +386,7 @@ export abstract class NaturalAbstractModelService<
.pipe(
map(result => {
this.apollo.client.reFetchObservableQueries();
const mappedResult = this.mapUpdate(result);

return mergeWith(object, mappedResult, mergeOverrideArray);
return this.mapUpdate(result);
}),
);
}
Expand Down

0 comments on commit 0723d9f

Please sign in to comment.