Skip to content

Commit

Permalink
BREAKING Typed NaturalAbstractDetail.data #9819
Browse files Browse the repository at this point in the history
`NaturalAbstractDetail.data` is implicitly typed with the result of
`service.resolve()`. If needed that type can be widened by specifying
a type for extra properties in the generic, like so:

```ts
class MyComponent extends NaturalAbstractDetail<MyService, {myProp: string}> {
}
```

`getConsolidatedForClient()` was dropped entirely without a replacement.
 It should never be used in our projects (and it was not).

`getDefaultForClient()` was renamed into `getFormExtraFieldDefaultValues()`
to better reflect its usage. It should never be called in projects, only
within Natural itself. Projects were updated to instead call
`getDefaultForServer()`.

The overall behavior of both `NaturalAbstractModelService` and
`NaturalAbstractDetail` is intact.
  • Loading branch information
PowerKiKi committed Aug 23, 2023
1 parent 633fee5 commit d90298c
Show file tree
Hide file tree
Showing 25 changed files with 143 additions and 80 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"prosemirror-view": "^1.31.5",
"rxjs": "^7.8.1",
"tslib": "^2.5.3",
"type-fest": "^4.2.0",
"zone.js": "~0.13.1"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion projects/natural-editor/tsconfig.lib.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"declarationMap": true,
"inlineSources": true,
"types": [],
"lib": ["dom", "es2018"]
"lib": ["dom", "ES2022"]
},
"exclude": ["src/test.ts", "**/*.spec.ts"]
}
39 changes: 32 additions & 7 deletions projects/natural/src/lib/classes/abstract-detail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,25 @@ import {kebabCase, merge, mergeWith, omit} from 'lodash-es';
import {NaturalAlertService} from '../modules/alert/alert.service';
import {NaturalAbstractPanel} from '../modules/panels/abstract-panel';
import {NaturalAbstractModelService} from '../services/abstract-model.service';
import {ExtractTcreate, ExtractTone, ExtractTupdate, Literal} from '../types/types';
import {ExtractResolve, ExtractTcreate, ExtractTone, ExtractTupdate, Literal} from '../types/types';
import {finalize} from 'rxjs/operators';
import {ifValid, validateAllFormControls} from './validators';
import {mergeOverrideArray} from './utility';
import {PaginatedData} from './data-source';
import {QueryVariables} from './query-variable-manager';
import {EMPTY, endWith, last, Observable, switchMap} from 'rxjs';

/**
* `Data` contains in `model` either the model fetched from DB or default values (without ID). And besides `model`,
* any other extra keys defined by Extra.
*/
type Data<TService, Extra> = {model: {id?: string}} & ExtractResolve<TService> & Extra;

// @dynamic
@Directive()
export class NaturalAbstractDetail<
TService extends NaturalAbstractModelService<
unknown,
{id: string},
any,
PaginatedData<Literal>,
QueryVariables,
Expand All @@ -28,16 +34,17 @@ export class NaturalAbstractDetail<
unknown,
any
>,
ExtraResolve extends Literal = Record<never, never>,
>
extends NaturalAbstractPanel
implements OnInit
{
/**
* Empty placeholder for data retrieved by the server
*/
public override data: any = {
model: {},
};
public override data: Data<TService, ExtraResolve> = {
model: this.service.getDefaultForServer(),
} as Data<TService, ExtraResolve>;

/**
* Form that manages the data from the controller
Expand Down Expand Up @@ -65,15 +72,23 @@ export class NaturalAbstractDetail<
*/
protected readonly route = inject(ActivatedRoute);

/**
* Once set, this must not change anymore, especially not right after the creation mutation,
* so the form does not switch from creation mode to update mode without an actual reload of
* model from DB (by navigating to update page).
*/
#updatePage = false;

public constructor(protected readonly key: string, public readonly service: TService) {
super();
}

public ngOnInit(): void {
if (!this.isPanel) {
this.route.data.subscribe(data => {
this.data = merge({model: this.service.getConsolidatedForClient()}, data[this.key]);
this.data = merge({model: this.service.getDefaultForServer()}, data[this.key]);
this.data = merge(this.data, omit(data, [this.key]));
this.#updatePage = !!this.data.model.id;
this.initForm();
});
} else {
Expand All @@ -85,8 +100,18 @@ export class NaturalAbstractDetail<
this.showFabButton = index === 0;
}

/**
* Returns whether `data.model` was fetched from DB, so we are on an update page, or if it is a new object
* with (only) default values, so we are on a creation page.
*
* This should be used instead of checking `data.model.id` directly, in order to type guard and get proper typing
*/
protected isUpdatePage(): this is {data: {model: ExtractTone<TService>}} {
return this.#updatePage;
}

public update(now = false): void {
if (!this.data.model.id) {
if (!this.isUpdatePage()) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion projects/natural/src/lib/classes/abstract-editable-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export class NaturalAbstractEditableList<
*/
public addItems(items: readonly T[]): void {
items.forEach(item => {
const completedItem = merge(this.service.getConsolidatedForClient(), item);
const completedItem = merge(this.service.getDefaultForServer(), item);
const lineFormGroup = this.service.getFormGroup(completedItem);
this.formArray.push(lineFormGroup);
});
Expand Down
16 changes: 3 additions & 13 deletions projects/natural/src/lib/classes/validators.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,4 @@
import {
available,
decimal,
deliverableEmail,
ifValid,
integer,
NaturalAbstractModelService,
unique,
urlValidator,
} from '@ecodev/natural';
import {available, decimal, deliverableEmail, ifValid, integer, unique, urlValidator} from '@ecodev/natural';
import {
AsyncValidatorFn,
FormControl,
Expand All @@ -19,6 +10,7 @@ import {
import {TestScheduler} from 'rxjs/testing';
import {concat, forkJoin, NEVER, Observable, of, Subject, tap} from 'rxjs';
import {first} from 'rxjs/operators';
import {UntypedModelService} from '../types/types';

function validate(validatorFn: ValidatorFn, expected: boolean, value: any): void {
const control = new FormControl();
Expand Down Expand Up @@ -97,9 +89,7 @@ describe('unique', () => {

cases.forEach(parameters => {
it('with ' + JSON.stringify(parameters), done => {
const service = jasmine.createSpyObj<
NaturalAbstractModelService<any, any, any, any, any, any, any, any, any, any>
>('NaturalAbstractModelService', ['count']);
const service = jasmine.createSpyObj<UntypedModelService>('NaturalAbstractModelService', ['count']);
service.count.and.returnValue(concat(of(parameters[1]), parameters[2] ? NEVER : NEVER));

const validator = unique('id', null, service);
Expand Down
4 changes: 2 additions & 2 deletions projects/natural/src/lib/classes/validators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ import {
} from '@angular/forms';
import {Observable, of, timer} from 'rxjs';
import {filter, first, map, switchMap} from 'rxjs/operators';
import {NaturalAbstractModelService} from '../services/abstract-model.service';
import {NaturalQueryVariablesManager, QueryVariables} from './query-variable-manager';
import {validTlds} from './tld';
import {FilterGroupCondition} from '../modules/search/classes/graphql-doctrine.types';
import {UntypedModelService} from '../types/types';

/**
* Returns an async validator function that checks that the form control value is unique
*/
export function unique<TService extends NaturalAbstractModelService<any, any, any, any, any, any, any, any, any, any>>(
export function unique<TService extends UntypedModelService>(
fieldName: string,
excludedId: string | null | undefined,
modelService: TService,
Expand Down
1 change: 1 addition & 0 deletions projects/natural/src/lib/modules/common/public-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ export {
NaturalSeoBasic,
NaturalSeoResolve,
NaturalSeoCallback,
NaturalSeoResolveData,
} from './services/seo.service';
export {provideSeo} from './services/seo.provider';
11 changes: 11 additions & 0 deletions projects/natural/src/lib/modules/common/services/seo.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,17 @@ export type NaturalSeoResolve = Robots & {
*/
export type NaturalSeoCallback = (routeData: Data) => NaturalSeoBasic;

/**
* Typically used to type the routing data received in the component, eg:
*
* ```ts
* class MyComponent extends NaturalAbstractDetail<MyService, NaturalSeoResolveData> {}
* ```
*/
export type NaturalSeoResolveData = {
seo: NaturalSeoBasic;
};

interface Robots {
/**
* If given will be used as robots meta tag, otherwise fallback on default value
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {Component} from '@angular/core';
import {NaturalQueryVariablesManager} from '../../../classes/query-variable-manager';
import {NaturalAbstractModelService} from '../../../services/abstract-model.service';
import {Literal} from '../../../types/types';
import {UntypedModelService, Literal} from '../../../types/types';
import {NaturalHierarchicConfiguration} from '../../hierarchic-selector/classes/hierarchic-configuration';
import {OrganizedModelSelection} from '../../hierarchic-selector/hierarchic-selector/hierarchic-selector.service';
import {FilterGroupConditionField} from '../../search/classes/graphql-doctrine.types';
Expand All @@ -24,7 +23,7 @@ export type HierarchicFiltersConfiguration<T = Literal> = Array<HierarchicFilter

export interface TypeHierarchicSelectorConfiguration {
key: string;
service: NaturalAbstractModelService<any, any, any, any, any, any, any, any, any, any>;
service: UntypedModelService;
config: NaturalHierarchicConfiguration[];
filters?: HierarchicFiltersConfiguration;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {Component} from '@angular/core';
import {NaturalAbstractModelService} from '../../../services/abstract-model.service';
import {FilterGroupConditionField} from '../../search/classes/graphql-doctrine.types';
import {ExtractTone, ExtractVall} from '../../../types/types';
import {ExtractTone, ExtractVall, UntypedModelService} from '../../../types/types';
import {AbstractAssociationSelectComponent} from '../abstract-association-select-component.directive';
import {EMPTY, Observable} from 'rxjs';
import {NaturalSelectComponent} from '../../select/select/select.component';
Expand All @@ -11,9 +10,7 @@ import {MatSelectModule} from '@angular/material/select';
import {MatFormFieldModule} from '@angular/material/form-field';
import {FormsModule, ReactiveFormsModule} from '@angular/forms';

export interface TypeSelectNaturalConfiguration<
TService extends NaturalAbstractModelService<any, any, any, any, any, any, any, any, any, any>,
> {
export interface TypeSelectNaturalConfiguration<TService extends UntypedModelService> {
service: TService;
placeholder: string;
filter?: ExtractVall<TService>['filter'];
Expand All @@ -33,7 +30,7 @@ export interface TypeSelectNaturalConfiguration<
],
})
export class TypeNaturalSelectComponent<
TService extends NaturalAbstractModelService<any, any, any, any, any, any, any, any, any, any>,
TService extends UntypedModelService,
> extends AbstractAssociationSelectComponent<TypeSelectNaturalConfiguration<TService>> {
public getCondition(): FilterGroupConditionField {
if (!this.isValid()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import {Type} from '@angular/core';
import {QueryVariables} from '../../../classes/query-variable-manager';
import {NaturalAbstractModelService} from '../../../services/abstract-model.service';
import {UntypedModelService} from '../../../types/types';

type GenericModelService = NaturalAbstractModelService<any, any, any, any, any, any, any, any, any, any>;

export interface NaturalHierarchicConfiguration<T extends GenericModelService = GenericModelService> {
export interface NaturalHierarchicConfiguration<T extends UntypedModelService = UntypedModelService> {
/**
* An AbstractModelService to be used to fetch items
*/
Expand Down Expand Up @@ -66,7 +64,7 @@ export interface NaturalHierarchicConfiguration<T extends GenericModelService =
displayWith?: (item: any) => string;
}

export interface NaturalHierarchicServiceConfiguration<T extends GenericModelService = GenericModelService>
export interface NaturalHierarchicServiceConfiguration<T extends UntypedModelService = UntypedModelService>
extends NaturalHierarchicConfiguration<T> {
injectedService: T;
}
28 changes: 16 additions & 12 deletions projects/natural/src/lib/services/abstract-model.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,6 @@ export abstract class NaturalAbstractModelService<
protected readonly deleteMutation: DocumentNode | null,
) {}

public getConsolidatedForClient(): (Vcreate['input'] | Vupdate['input']) & Literal {
return Object.assign(this.getDefaultForServer(), this.getDefaultForClient());
}

/**
* List of individual fields validators
*/
Expand Down Expand Up @@ -98,7 +94,7 @@ export abstract class NaturalAbstractModelService<
}

public getFormConfig(model: Literal): FormControls {
const values = this.getConsolidatedForClient();
const values = {...this.getDefaultForServer(), ...this.getFormExtraFieldDefaultValues()};
const validators = this.getFormValidators(model);
const asyncValidators = this.getFormAsyncValidators(model);
const controls: FormControls = {};
Expand Down Expand Up @@ -455,13 +451,13 @@ export abstract class NaturalAbstractModelService<
/**
* Resolve model and items related to the model, if the id is provided, in order to show a form
*/
public resolve(id: string): Observable<Resolve<Tone>> {
public resolve(id: string): Observable<Resolve<Tone | Vcreate['input']>> {
// Load model if id is given
let observable: Observable<Tone>;
let observable: Observable<Tone | Vcreate['input']>;
if (id) {
observable = this.getOne(id);
} else {
observable = of(this.getConsolidatedForClient() as Tone);
observable = of(this.getDefaultForServer() as Tone);
}

return observable.pipe(
Expand Down Expand Up @@ -528,16 +524,24 @@ export abstract class NaturalAbstractModelService<
*
* This is typically useful when showing a form for creation
*/
protected getDefaultForServer(): Vcreate['input'] | Vupdate['input'] {
public getDefaultForServer(): Vcreate['input'] {
return {};
}

/**
* Return empty object with some default values from frontend perspective
* You probably **should not** use this.
*
* If you are trying to *call* this method, instead you probably want to call `getDefaultForServer()` to get default
* values for a model, or `getFormConfig()` to get a configured form that includes extra form fields.
*
* If you are trying to *override* this method, instead you probably want to override `getDefaultForServer()`.
*
* Where empty object must respect graphql XXXInput type, may need some default values for other fields
* The only and **very rare** reason to override this method is if the client needs extra form fields that cannot be
* accepted by the server (not part of `XXXInput` type) and that are strictly for the client form needs. In that case,
* then you can return default values for those extra form fields, and the form returned by `getFormConfig()` will
* include those extra fields.
*/
protected getDefaultForClient(): Literal {
protected getFormExtraFieldDefaultValues(): Literal {
return {};
}

Expand Down
8 changes: 5 additions & 3 deletions projects/natural/src/lib/testing/item.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ export interface Item {
readonly parent: Item | null;
}

export type ItemInput = Omit<Item, '__typename' | 'id'>;

@Injectable({
providedIn: 'root',
})
Expand All @@ -29,9 +31,9 @@ export class ItemService extends NaturalAbstractModelService<
PaginatedData<Item>,
QueryVariables,
Item,
{input: Item},
{input: ItemInput},
Item,
{id: string; input: Literal},
{id: string; input: Partial<ItemInput>},
boolean,
{ids: string[]}
> {
Expand Down Expand Up @@ -103,7 +105,7 @@ export class ItemService extends NaturalAbstractModelService<
return of(this.getItem(true, 2, id));
}

protected override getDefaultForClient(): Literal {
protected override getFormExtraFieldDefaultValues(): Literal {
return {
name: '',
description: '',
Expand Down
2 changes: 1 addition & 1 deletion projects/natural/src/lib/testing/null.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class NullService extends NaturalAbstractModelService<
super(apollo, naturalDebounceService, 'user', null, null, createPost, null, null);
}

protected override getDefaultForServer(): PostInput {
public override getDefaultForServer(): PostInput {
return {
slug: '',
blog: '',
Expand Down
2 changes: 1 addition & 1 deletion projects/natural/src/lib/testing/post.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export class PostService extends NaturalAbstractModelService<
super(apollo, naturalDebounceService, 'post', postQuery, postsQuery, createPost, updatePost, deletePosts);
}

protected override getDefaultForServer(): PostInput {
public override getDefaultForServer(): PostInput {
return {
slug: '',
blog: '',
Expand Down
Loading

0 comments on commit d90298c

Please sign in to comment.