Skip to content

Commit

Permalink
fix(sap.fhir.model.r4.FHIRListBinding): Trigger binding updates when …
Browse files Browse the repository at this point in the history 
…resource is removed or a rollback is performed (#331)

* fix(sap.fhir.model.r4.FHIRListBinding): Trigger binding updates when resource is removed or a rollback is performed 

This PR is for fixing the list binding still showing the removed items if submit changes is not done immediately.
Use another array to capture the client removed resources and fill it appropriately to decide whether during reset changes the contexts needs to be recreated (for those resources which are not yet submitted but was removed using the remove operation)

* tests: added test data

* refactor: review comments

* refactor: review comments

* refactor: review comments

* refactor: review comments

* refactor: review comments
  • Loading branch information
@AmirthavalliG
AmirthavalliG authored May 18, 2022
1 parent 1772dfd commit aa33548
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 19 deletions.
46 changes: 28 additions & 18 deletions src/sap/fhir/model/r4/FHIRListBinding.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,7 @@ sap.ui.define([
throw new Error("FHIR Server error: The \"total\" property is missing in the response for the requested FHIR resource " + this.sPath);
}
this.bDirectCallPending = false;
if (!this.aKeys) {
this.aKeys = [];
iStartIndex = 0;
} else {
iStartIndex = this.aKeys.length;
}
iStartIndex = this.aKeys.length;
if (oData.entry && oData.entry.length){
var oResource;
var oBindingInfo = this.oModel.getBindingInfo(this.sPath, this.oContext, this.bUnique);
Expand Down Expand Up @@ -237,6 +232,13 @@ sap.ui.define([
this.aKeys = FHIRUtils.deepClone(this.aKeysServerState);
}
iValuesLength = this.aKeys.length - this.iClientChanges;
var aClientRemovedResources = this.oModel.mRemovedResources[oBindingInfo.getResourceType()];
if (aClientRemovedResources){
this.aKeys = this.aKeys.filter(function (sResPath) {
return !aClientRemovedResources.includes(sResPath);
});
this.iTotalLength = this.aKeys.length;
}
} else {
this.iClientChanges = 0;
this.aKeys = this.aKeysServerState;
Expand Down Expand Up @@ -391,28 +393,36 @@ sap.ui.define([
FHIRListBinding.prototype.checkUpdate = function(bForceUpdate, mChangedEntities, sMethod) {
var oBindingInfo = this.oModel.getBindingInfo(this.sPath, this.oContext, this.bUnique);
var mResources = oBindingInfo && mChangedEntities && mChangedEntities[oBindingInfo.getResourceType()];
if (mResources && sMethod && sMethod !== HTTPMethod.GET){
for (var sId in mResources){
if (!this.isRelative()){
if (sMethod === HTTPMethod.DELETE){
this.iTotalLength--;
this.aKeysServerState.splice(this.aKeysServerState.indexOf(oBindingInfo.getResourceType() + "/" + sId), 1);
} else if (sMethod === HTTPMethod.POST){
if (mResources && sMethod && sMethod !== HTTPMethod.GET) {
var sResType = oBindingInfo.getResourceType();
for (var sId in mResources) {
var sResPath = sResType + "/" + sId;
if (!this.isRelative()) {
if (sMethod === HTTPMethod.DELETE) {
// check for context
if (this.oModel.mRemovedResources[sResType] && this.oModel.mRemovedResources[sResType].indexOf(sResPath) > -1) {
// client changes (not yet submitted to server)
this.aKeys.splice(this.aKeys.indexOf(oBindingInfo.getResourceType() + "/" + sId), 1);
} else {
// server changes (response from server after submitted the removed resources directly)
this.aKeysServerState.splice(this.aKeysServerState.indexOf(oBindingInfo.getResourceType() + "/" + sId), 1);
}
} else if (sMethod === HTTPMethod.POST) {
this.iTotalLength++;
this.aKeysServerState.unshift(oBindingInfo.getResourceType() + "/" + sId);
} else if (sMethod === HTTPMethod.PUT && oBindingInfo.getBinding().length === 3){ // in case of history entry update
} else if (sMethod === HTTPMethod.PUT && oBindingInfo.getBinding().length === 3) { // in case of history entry update
this.iTotalLength++;
this.aKeysServerState.unshift(oBindingInfo.getAbsolutePath().substring(1) + "/" + mResources[sId].meta.versionId);
}
} else if (sMethod === HTTPMethod.PUT){
} else if (sMethod === HTTPMethod.PUT) {
this.iTotalLength = undefined;
}
}
}
var sPath = this.oModel._getProperty(mChangedEntities, ["path", "lastUpdated"]);
if (oBindingInfo && (sPath && FHIRUtils.getNumberOfLevelsByPath(sPath) < 3 && sPath.indexOf(oBindingInfo.getResourceType()) > -1 || sPath === oBindingInfo.getAbsolutePath()) || bForceUpdate === true || sMethod){
if (oBindingInfo && (sPath && FHIRUtils.getNumberOfLevelsByPath(sPath) < 3 && sPath.indexOf(oBindingInfo.getResourceType()) > -1 || sPath === oBindingInfo.getAbsolutePath()) || bForceUpdate === true || sMethod) {
this._fireChange({
reason : ChangeReason.Change
reason: ChangeReason.Change
});
}
};
Expand Down Expand Up @@ -547,7 +557,7 @@ sap.ui.define([
* @since 1.0.0
*/
FHIRListBinding.prototype._resetData = function() {
this.aKeys = undefined;
this.aKeys = [];
this.aKeysServerState = [];
this.bResourceNotAvailable = false;
this.aContexts = undefined;
Expand Down
37 changes: 37 additions & 0 deletions src/sap/fhir/model/r4/FHIRModel.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ sap.ui.define([
this.mResourceGroupId = {};
this.mContexts = {};
this.mMessages = {};
this.mRemovedResources = {};
};

/**
Expand Down Expand Up @@ -1512,10 +1513,27 @@ sap.ui.define([
} else {
oRequestInfo = this._createRequestInfo(HTTPMethod.DELETE, oBindingInfo.getResourceServerPath());
this._setProperty(this.mChangedResources, FHIRUtils.deepClone(aResPath), oRequestInfo, true, sResourceGroupId && sResourceGroupId === sGroupId ? sGroupId : undefined);
this._addToRemovedResources(oBindingInfo, sResourcePath);
this.checkUpdate(true, this.mChangedResources, oBindingInfo, HTTPMethod.DELETE);
}
}
};

/**
* Adds the resource path to the removed resources map
*
* @param {sap.fhir.model.r4.lib.BindingInfo} oBindingInfo The binding info object
* @param {string} [sResourcePath] The resource path of the removed resource
* @private
*/
FHIRModel.prototype._addToRemovedResources = function (oBindingInfo, sResourcePath) {
if (!this.mRemovedResources[oBindingInfo.getResourceType()]) {
this.mRemovedResources[oBindingInfo.getResourceType()] = [sResourcePath.substring(1)];
} else {
this.mRemovedResources[oBindingInfo.getResourceType()].unshift(sResourcePath.substring(1));
}
};

/**
* Resets the model to the state when the model was synchronized with the server for the last time.
* Resetting means newly created resources are removed and changed resources are rolled backed to the earlier state.
Expand All @@ -1539,6 +1557,8 @@ sap.ui.define([
this._setProperty(this.oData, FHIRUtils.deepClone(aResPath));
this._setProperty(this.mResourceGroupId, FHIRUtils.deepClone(aResPath));
this._removeFromOrderResources(oBindingInfo);
} else if (oRequestInfo.method === HTTPMethod.DELETE){
this._removeFromRemovedResources(oBindingInfo);
}
this._setProperty(this.mChangedResources, FHIRUtils.deepClone(aResPath));
}.bind(this);
Expand Down Expand Up @@ -1572,6 +1592,7 @@ sap.ui.define([
* Removes a resource from the order resources map
*
* @param {sap.fhir.model.r4.lib.BindingInfo} oBindingInfo The binding info object
* @private
*/
FHIRModel.prototype._removeFromOrderResources = function(oBindingInfo){
var sType = oBindingInfo.getResourceType();
Expand All @@ -1583,6 +1604,22 @@ sap.ui.define([
}
};

/**
* Removes a resource from the removed resources map
*
* @param {sap.fhir.model.r4.lib.BindingInfo} oBindingInfo The binding info object
* @private
*/
FHIRModel.prototype._removeFromRemovedResources = function (oBindingInfo) {
var sType = oBindingInfo.getResourceType();
var sId = oBindingInfo.getResourceId();
var iIndex = this.mRemovedResources[sType].indexOf(sType + "/" + sId);
this.mRemovedResources[sType].splice(iIndex, 1);
if (this.mRemovedResources[sType].length === 0) {
delete this.mRemovedResources[sType];
}
};

/**
* Cannot get a shared context for a path. Contexts are created by bindings instead and there
* may be multiple contexts for the same path.
Expand Down
23 changes: 23 additions & 0 deletions test/data/Practitioner.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,29 @@
"resourceType": "Bundle",
"type": "transaction",
"entry": [
{
"fullUrl": "http://localhost:8080/fhir/R4/Practitioner/a4748",
"resource": {
"resourceType": "Practitioner",
"id": "a4748",
"meta": {
"versionId": "1",
"lastUpdated": "2018-09-19T16:05:18.620+02:00"
},
"name": [
{
"family": "Melisa",
"given": [
"Ray"
]
}
]
},
"request": {
"method": "PUT",
"url": "Practitioner/a4748"
}
},
{
"fullUrl": "http://localhost:8080/fhir/R4/Practitioner/a2533",
"resource": {
Expand Down
27 changes: 27 additions & 0 deletions test/qunit/model/FHIRModel.integration.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ sap.ui.define([
this.oFhirModel.aBindings = [];
this.oFhirModel.refresh();
this.oFhirModel.mChangedResources = {};
this.oFhirModel.mRemovedResources = {};
}
});

Expand Down Expand Up @@ -849,4 +850,30 @@ sap.ui.define([
this.oFhirModel.submitChanges("transaction", fnSuccessCallback);
});

QUnit.test("Test remove items from list and verify the contexts before and after submitting the changes", function (assert) {
var oListBinding = this.oFhirModel.bindList("/Practitioner", undefined, undefined, undefined, { groupId: "patientDetails" });
this.oFhirModel.aBindings.push(oListBinding);
var done = assert.async();
var fnAssertion = function () {
var iCurrentLength = oListBinding.getContexts().length;
var aContext = oListBinding.getContexts();
var sResPath = aContext[0].sPath;
this.oFhirModel.remove([sResPath], undefined, "patientDetails");
assert.deepEqual(oListBinding.getContexts().length, iCurrentLength - 1, "List doesn't show the removed items even if the changes are not submitted");
this.oFhirModel.resetChanges("patientDetails");
assert.deepEqual(oListBinding.getContexts().length, iCurrentLength, "List shows the previously removed items if the changes are not submitted and reset changes is triggered");
assert.deepEqual(this.oFhirModel.mRemovedResources["Practitioner"], undefined, "Removed Resources of the model is correctly cleared during reset changes");
assert.deepEqual(this.oFhirModel.mRemovedResources.hasOwnProperty("Practitioner"), false, "The key doesn't exists if there are no removed resources for a particular type");
this.oFhirModel.remove([sResPath], undefined, "patientDetails");
assert.deepEqual(this.oFhirModel.mRemovedResources["Practitioner"].length, 1, "Removed Resources of the model is correctly filled");
this.oFhirModel.submitChanges("patientDetails", function (aFHIRResource) {
assert.deepEqual(this.oFhirModel.mRemovedResources["Practitioner"], undefined, "Removed Resources of the model is correctly cleared after submitting changes");
assert.deepEqual(this.oFhirModel.mRemovedResources.hasOwnProperty("Practitioner"), false, "The key doesn't exists if there are no removed resources for a particular type");
done();
}.bind(this));
}.bind(this);
oListBinding.attachDataReceived(fnAssertion);
oListBinding.getContexts();
});

});
4 changes: 3 additions & 1 deletion test/qunit/model/FHIRModel.unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,12 @@ sap.ui.define([
assert.strictEqual(sSubmitMode, "Transaction", "The default submit mode is defined");
});

QUnit.test("Removing Resources from FHIR Model", function(assert) {
QUnit.test("Removing Resources from FHIR Model", function (assert) {
var mChangedResources = JSON.parse("{\"Patient\":{\"123\":{\"url\":\"/Patient/123\",\"method\":\"DELETE\"},\"abc\":{\"url\":\"/Patient/abc\",\"method\":\"DELETE\"}}}");
var mRemovedResources = JSON.parse("{\"Patient\":[\"Patient/abc\",\"Patient/123\"]}");
this.oFhirModel1.remove(["/Patient/123", "/Patient/abc"]);
assert.deepEqual(mChangedResources, this.oFhirModel1.mChangedResources, "The changed resources are equal");
assert.deepEqual(mRemovedResources, this.oFhirModel1.mRemovedResources, "The removed resources are equal");
});

QUnit.test("Call next link, which has no query params, and check that it is directly call", function(assert) {
Expand Down

0 comments on commit aa33548

Please sign in to comment.