[WEBCLIENT-47] "Delete Concept" in I2B2 web app deletes the first occurrence of the concept, not the one selected Created: 16/Aug/13 Updated: 01/Jul/14 Resolved: 06/Jun/14 |
|
| Status: | Closed |
| Project: | i2b2 Web Client |
| Component/s: | Web Client |
| Affects Version/s: | 1.6.08 |
| Fix Version/s: | 1.7.02 |
| Type: | Bug | Priority: | Major |
| Reporter: | Matthew Hoag | Assignee: | Janice Donahoe |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Affects View/s: |
Query Tool
|
| i2b2 Sponsored Project/s: |
i2b2 Web Client
|
| Reproduction Notes: | Reproduced in the 1.7.02 test environment |
| Testing Notes: | Tested and it appears to be working correctly.
Browsers Tested: Chrome, Firefox, I.E., Safari |
| Participant/s: | |
| Build Number (Fixed): | 1.7.02.0005 |
| Description |
|
In the [Query Tool] Pane, adding the same concept twice and selecting the second to be deleted, will actually delete the first in the list. This is also more than just a visual annoyance to the user. Suppose one were to drop in two of the same numerically constrained concepts with differing parameters (e.g. Length of Stay concept with > 2 and < 10). In this case, if you select the second concept for delete, it will delete the first. The delete mechanism appears to always delete the first matched concept it encounters and there appears to be no work around. Reproduction Steps: 1. Open I2B2 instance with a numerically constrained concept 2. Add the numerically constrained concept to the [Query Tool] Pane with a value of 1 3. Add the same numerically constrained concept to the [Query Tool] Pane with a value of 2 4. Delete the numerically constrained concept with the value of 2. Expected Result: The concept with a value of 2 is deleted Actual Result: The concept with a value of 1 is deleted |
| Comments |
| Comment by Matthew Hoag [ 24/Sep/13 ] |
|
Diagnosis ------- After investigating this issue further the problem appeared to be one of uniqueness. Or rather, a disjoint between data and visual element identification. When attempting to delete a concept, the controller passes in the cdat.data.sdxInfo.sdxKeyValue which is (from my testing) the full path of the concept (a data level identification) instead of some key that could be used to uniquely identify the UI representation of the concept. Thus, while iterating over all of the items in the panel, the first element to match the data specified key is removed - and not always the UI element which was clicked. This is major issue because the the full path is not enough to uniquely identify a UI concept. With the use of concept parameters the, UI concept actually represents a concept + mod Values, and the full path does no differentiation with respect to modValues. Remedy ------- I believe the proper course of action is for the controller to pass a key to the delete method that can be used to uniquely identify the visual representation of the concept. After a little bit of digging around, I found the `htmlID` in the `renderData` which seems to be a unique identifier for the concept. It appears that this `htmlID` can be correlated to the `nodeid` on the tree viewer children (or the `htmlID` if you dig into the `data`.`renderData` of the child. My suggested code fix is as follows (with the caveat that matching to `htmlID` on the tree viewer children may be better than matching to `nodeid`): {code:title=webclient/js-i2b2/cells/CRC/CRC_view_QryTool.js|borderStyle=solid} @@ -184,7 +184,7 @@ switch(actionName) { case "delete": // delete item from the panel - cdat.ctrlr._deleteConcept(cdat.data.sdxInfo.sdxKeyValue, cdat.data); + cdat.ctrlr._deleteConcept(cdat.data.renderData.htmlID, cdat.data); break; case "labvalues": cdat.ctrlr.showLabValues(cdat.data.sdxInfo.sdxKeyValue, cdat.data); {code} {code:title=webclient/js-i2b2/cells/CRC/CRC_ctrlr_QryPanel.js|borderStyle=solid} @@ -777,24 +777,15 @@ } // ================================================================================================== // - this._deleteConcept = function(key) { + this._deleteConcept = function(htmlID) { var pd = i2b2.CRC.model.queryCurrent.panels[this.panelCurrentIndex]; $('infoQueryStatusText').innerHTML = ""; - // remove the concept from panel - for (var i=0; i< pd.items.length; i++) { - if ((pd.items[i].origData.key == key) - || (pd.items[i].origData.id == key)) { - // found the concept to remove - var rto = pd.items[i]; - break; - } - } - if (undefined===rto) { return; } + + if (undefined===htmlID) { return; } // remove the node in the treeview var tvChildren = pd.tvRootNode.children for (var i=0; i< tvChildren.length; i++) { - if ((tvChildren[i].data.i2b2_SDX.sdxInfo.sdxKeyValue===rto.origData.key) - || (tvChildren[i].data.i2b2_SDX.sdxInfo.sdxKeyValue===rto.origData.id)) { + if (tvChildren[i].data.nodeid===htmlID) { this.yuiTree.removeNode(tvChildren[i],false); this._redrawTree.call(this, pd); break; {code} |
| Comment by Janice Donahoe [ 14/May/14 ] |
|
This issue can be reproduced in the 1.7.01 version of i2b2. It will be evaluated by the i2b2 team for inclusion into version 1.7.02. |