Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

data.model is synced with Apollo cache #9844 #244

Merged
merged 29 commits into from
Mar 24, 2024
Merged

data.model is synced with Apollo cache #9844 #244

merged 29 commits into from
Mar 24, 2024

Conversation

PowerKiKi
Copy link
Member

@PowerKiKi PowerKiKi commented Nov 1, 2023

NaturalAbstractModelService.resolve() now resolves in two steps. First
it asks Apollo to fetch the object from network to force refresh Apollo
cache. Then only, the resolving is considered finished, and we emit an
observable that watches Apollo cache for the model. So the component can
receive the observable from the route, subscribe to it and immediately
receive an up-to-date model. And all subsequent updates to the model
will be emitted from Apollo cache too.

But the real breaking part is that previously the route resolved data
shape:

{
    permission: myPermissions,
    action: {
        model: myAction,
        statuses: myStatuses,
        priorities: myPriorities,
    }
}

Now, because NaturalAbstractModelService.resolve() only resolve the
model, it becomes the simpler:

{
    permission: myPermissions,
    model: myActionObservable,
    statuses: myStatuses,
    priorities: myPriorities,
}

So all overrides of resolve() must be migrate outside the model
service. Typically, something like:

export const actionResolvers: ResolveData = {
    model: resolveAction,
    statuses: () => inject(NaturalEnumService).get('ActionStatus'),
    priorities: () => inject(NaturalEnumService).get('Priority'),
}

TODO

  • unit test NaturalAbstractModelService.resolve
  • unit test NaturalAbstractModelService.watchOne
  • modifier + tester dans nos projets

`NaturalAbstractModelService.resolve()` now resolves in two steps. First
it asks Apollo to fetch the object from network to force refresh Apollo
cache. Then only, the resolving is considered finished, and we emit an
observable that watches Apollo cache for the model. So the component can
receive the observable from the route, subscribe to it and immediately
receive an up-to-date model. And all subsequent updates to the model
will be emitted from Apollo cache too.

But the real breaking part is that previously the route resolved data
shape:

```ts
{
    permission: myPermissions,
    action: {
        model: myAction,
        statuses: myStatuses,
        priorities: myPriorities,
    }
}
```

Now, because `NaturalAbstractModelService.resolve()` only resolve the
model, it becomes the simpler:

```ts
{
    permission: myPermissions,
    model: myActionObservable,
    statuses: myStatuses,
    priorities: myPriorities,
}
```

So all overrides of `resolve()` must be migrate outside the model
service, and use the key `model` for the main model. Typically,
something like:

```ts
export const actionResolvers: ResolveData = {
    model: resolveAction,
    statuses: () => inject(NaturalEnumService).get('ActionStatus'),
    priorities: () => inject(NaturalEnumService).get('Priority'),
}
```
At the same time the Seo service supports for updates on the model. So
if the model is mutated, the page title might get updated too.

Routing configuration before:

```ts
data: {
    seo: {resolveKey: 'user'} satisfies NaturalSeo,
}
```

After:

```ts
data: {
    seo: {resolve: true} satisfies NaturalSeo,
}
```
Apollo scheduling is always asynchronous, but we must be synchronous to
guarantee that `data` is immediately populated with real data.
`updatePartially()` was created because `updateNow()` always injected
default values, so we could not update a single field at once.
But `updateNow()` should never have behaved that way, because all our
GraphQL mutations are entirely partial, and thus it is never required to
submit all fields for an update. I think `updateNow()` became this way
by accident when we introduced default value for creation (which is a
valid use-case), and/or because we had a few changes, very early on, on
how the API dealt with optional arguments.

So `updateNow()` does not inject default values anymore, and thus
`updatePartially()` loses its reason to exist.
For instance:

```ts
service.update({id: 1, firstName: 'John'}).subscribe();
service.update({id: 1, lastName: 'Doe'}).subscribe();
```

Will wait 2 seconds to batch the two fields and to call the equivalent
of:

```ts
service.updateNow({id: 1, firstName: 'John', lastName: 'Doe'}).subscribe();
```
Fields that are never touched by humans are not re-submitted. However, a
field that is updated, then updated back to its original value will
actually be submitted as-is.
In dialog components, we might tolerate having a non-observable model,
because the model probably won't be changed from within the dialog
so we support that use-case too.
Because we need to support extra fields on the form (via
`getFormExtraFieldDefaultValues()`) that can be injected into partial
variables (via `getPartialVariablesForUpdate()`) for
`TransactionComponent`.
We have use-case where a value is programmatically set, and the human
must not change it, so the field is disabled. But we still need to
submit the value to the server. That's the case of `control.useEquipment`
In rare cases, the server requires all fields at once, as seen in
`updateOrderLine`.
Because we will submit form data for mutation, and because we merge the
mutation result back into the form data via
`NaturalAbstractModelService.updateNow()`, then we must make triple-sure
 that the form is not populated with readonly objects coming from our
 API. Otherwise, a structure like the following would raise JS errors
 after mutation:

 ```json
 {
    id: 123,
    name: 'foo',
    product: {
        id: 456,
        quantity: 5, // This used to fail if fetched from mutation
    }
}
 ```
As seen on Ichtus, if a dialog is open on root URL, then it is possible
that urlTree has children, but not _primary_ children, only
_secondary_ children
If `model` is found in the resolved data of a panel, then it will be
automatically subscribed to and `this.data` will be updated whenever it
emits.
This avoids duplicating critical logic in projects.
Because `NaturalAbstractPanel.initPanel()` is only ever called at most 1
time per instance of a component, we don't need to handle previous
subscriptions
@PowerKiKi PowerKiKi marked this pull request as ready for review March 24, 2024 04:43
@PowerKiKi PowerKiKi merged commit ffad3e0 into master Mar 24, 2024
22 checks passed
@PowerKiKi PowerKiKi changed the title WIP data.model is synced with Apollo cache #9844 data.model is synced with Apollo cache #9844 Mar 24, 2024
@PowerKiKi PowerKiKi deleted the issues-9844 branch April 15, 2024 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant