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

5.0.0-beta.0 - normalization incomplete #375

Open
basz opened this issue Aug 19, 2020 · 5 comments
Open

5.0.0-beta.0 - normalization incomplete #375

basz opened this issue Aug 19, 2020 · 5 comments

Comments

@basz
Copy link
Contributor

basz commented Aug 19, 2020

I have a model with a cloudDrive status fragment that in turn has two more fragments. called context and failure.

Over a webstomp connection I receive updates to my models via store.pushPayload(payload). The model correctly updates except if it does not exists in the store. When it does exist in the store all properties are correctly updated except for the nested fragments... (note the state property is correctly updated.

"status" : {
"state": "checked-out",
"context": {
"adapter": "dropbox",
"identity_id": "xxx-xxx-xxx-xxx-xxx",
"order_number": "2.4",
"order_status": "cad",
"checkout_time": "2020-08-19T11:51:26.000Z"
},
"failure": {
"message_code": null,
"message": null,
"details": []
}
}

So. a button instructs the backend to perform a 'check-in'... This might fail due to some reason and the whole model is transmitted via webstomp as a json API payload. Pushing that payload does not update the failure fragment.

Any ideas where to look/debug. I have confirmed the received payload is correct.

"data": {
    "type": "dossier/order",
    "id": "xxx-xxx-xxx-xxx-xxx",
    "attributes": {
      // redacted
      "cloud-drive": {
        "state": "checked-out",
        "context": {
          "checkout_time": "2020-08-19T11:51:26+00:00",
          "identity_id": "xxx-xxx-xxx-xxx-xxx",
          "adapter": "dropbox",
          "order_status": "cad",
          "order_number": "2.4"
        },
        "failure": {
          "message_code": "dossier.checkInFailure.noMutationsDetected",
          "message": "No changes where detected. Has the cloud drive been fully synchronized?",
          "details": []
        }
      }
    },
  }
}
@basz
Copy link
Contributor Author

basz commented Aug 19, 2020

setting a fragment to null on a model seems to be the culprit here. I would guess the fragment is then either deleted or recreated from the defaultValue. Is that indeed what I may expect to happen?

@patocallaghan
Copy link
Contributor

@basz I think I'm seeing a similar issue to you. While it's not exactly the same it has the same characteristic where the fragment isn't populated after being null initially.

In our case we are creating a record which is only sparsely populated but the rest of the attributes are filled in by the response from the backend.

I've created a failing test in #384 to replicate it and have confirmed it did work before the 5.0 refactor (my tests passed).

@patocallaghan
Copy link
Contributor

patocallaghan commented Oct 11, 2021

So I've come back to this issue after a long time as we're in the midst of trying get ourselves unblocked to upgrade Ember Data (currently stuck on ED 3.12).

As I mentioned in the comment above I have some failing tests which replicate the buggy behaviour we are seeing. Essentially if you have a model/fragment which has a fragment array/fragment property set to null it will never update when a http response has those properties updated. I have confirmed running these failing tests on a v4 branch pass just fine.

I've tracked the bug down to the following lines of code in the FragmentRecordData#didCommit hook:

this.fragmentNames.forEach(key => {
fragment = this.serverFragments[key];
if (fragment) {
fragment._didCommit(attributes[key]);
}
// this is here so that the super call does not process this key
delete attributes[key];
});

Basically because the existing property is set to null the fragment = this.serverFragments[key]; evaluates to null. This means the if (fragment) check fails and we don't commit the new data to the fragment with fragment._didCommit(attributes[key]);.

I've been looking at it and what I'm imagining is that if the fragment doesn't exist we should recreate it using something like setupFragment or setupFragmentArray but I'm hitting a few stumbling blocks there. The API of createFragment, for example, is createFragment(store, declaredModelName, record, key, options, data) but I can't find any possible way to get access to the record at this point in the stacktrace so can't recreate the fragment.

image

@igorT @runspired as you folks have context on the RecordData implementation here, do you have any pointers?

patocallaghan added a commit to patocallaghan/ember-data-model-fragments that referenced this issue Oct 12, 2021
patocallaghan added a commit to patocallaghan/ember-data-model-fragments that referenced this issue Oct 15, 2021
patocallaghan added a commit to patocallaghan/ember-data-model-fragments that referenced this issue Oct 15, 2021
@patocallaghan
Copy link
Contributor

I've added a potential direction for a fix for this issue in #384 but it seems a bit hacky.

patocallaghan added a commit to patocallaghan/ember-data-model-fragments that referenced this issue Nov 26, 2021
patocallaghan added a commit to patocallaghan/ember-data-model-fragments that referenced this issue Dec 21, 2021
patocallaghan added a commit to patocallaghan/ember-data-model-fragments that referenced this issue Dec 21, 2021
patocallaghan added a commit to patocallaghan/ember-data-model-fragments that referenced this issue Dec 21, 2021
@patocallaghan
Copy link
Contributor

@basz I'm not sure if you're issue was exactly the same as mine but could you try out https://github.com/adopted-ember-addons/ember-data-model-fragments/releases/tag/v5.0.0-beta.3 where I shipped my fix to make sure null fragments are updated from the server?

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

No branches or pull requests

2 participants