Cloning issues can lead to multiple NCIt Definitions

[safrant]

Found in version 2.5.1 When cloning a concept in NCIt the Definition is copied to the new concept automatically. The new Definition window is presented to the user during concept cloning and will add any entered Definition to the concept. This can lead to two NCI Definition properties in a single concept.

The user should be prevented from saving the new concept with two NCI Definition properties. Currently they are warned if they try to save with two NCI:PT. A similar message should appear when trying to save with two NCI Definitions.

[bdionne]

When cloning a concept in NCIt the Definition is copied to the new concept automatically. The new Definition window is presented to the user during concept cloning and will add any entered Definition to the concept.

Right, pretty sure this was the requirement for split/copy as well as creating a new class

The user should be prevented from saving the new concept with two NCI Definition properties. Currently they are warned if they try to save with two NCI:PT. A similar message should appear when trying to save with two NCI Definitions.

If so, this is new. There is no check an warning now. The NCI:PT restriction applies to the FULL_SYN

cc: @NCIEVS

[safrant]

I think it is exclusively enforced by business rule now, but this new dialog makes it much easier to mistakenly add a second NCI Definition.

[NCIEVS]

You mean by editing guidelines. And then flagged in QA.

@bdionne , yes, this is a new requirement. The old one was not to enforce single definitions because there were a number of concepts with multiple NCI defs.

[bdionne]

well we could remove it from the complex ops and keep it on the new class dialog. Perhaps we were too hasty to add it as a convenience for the modelers.

I think it should be consistent though. If we warn them in the complex operations we should also warn them when they add multiple definitions in the edit panel.

[safrant]

I would agree that they should always be warned when adding multiple Definitions.

[singhik]

Adding Email Feedback from editors ::

Lori:: As far as I know, there should only be on NCI DEF. Any other def should be an ALT_DEF.

Definitely in favor of a singe NCI DEF policy. Liz or Nicole if you can think of something I have overlooked please speak up but I can't see more than one NCI Def being good terminology practice.

Liz :: As long as this is only enforced at the Save (right now during Copy, Split and Merge a user could potentially have 2 NCI DEF during an edit session) and as long as any enforcement can accommodate multiple ALT_DEF, I think we are okay.

[bdionne]

Liz :: As long as this is only enforced at the Save (right now during Copy, Split and Merge a user could potentially have 2 NCI DEF during an edit session) and as long as any enforcement can accommodate multiple ALT_DEF, I think we are okay.

This comment needs clarification:

• why does it matter when it's enforced? Is Liz just commenting on the existing behavior?
• do we want to allow them to create multiple DEFs during clone/split/etc.. and then edit one to change the source away from NCI?
• does want mean just warn or do we want to prevent them from doing it?

So if we decide to do this, is the requirement:

• There can be only one DEFINITION property with a source of NCI?

cc: @hahndane

[NCIEVS]

• I think it's just a comment.
• maybe don't allow creating multiple defs during a complex edit; else like you said they'd have to go back and choose the right one. perhaps the best thing is to let the editor edit the single def that gets copied in a copy/split. Then the merge would need a notice to select & edit the "winning" def and reminding them to drop the "losing" def.
• as far as i'm reading it, prevent them in the appropriate cases (e.g. split/copy) and warn in the rest (e.g. merge)

[hahndane]

I am commenting about existing behavior. When you Copy, Split or Merge the new (Copy, Split) or surviving (Merge) concept is assigned 2 NCI DEF as part of the process. (I could demo this for you.)

For Split and Merge, editor input is needed to choose the NCI DEF that should persist in the new (Split) or surviving (Merge) concept, but we do want the editor to make this choice and only keep one NCI DEF and remove the other. (In theory if a new DEF is added during a Copy that is the DEF that should persist in the new concept but it may be difficult to have this enforced just for Copy.)

We are hoping to prevent the Save of a concept with 2 NCI DEF.

Finally, we don’t want ALT_DEF to be restricted in this way.

[hahndane]

Note the original statement "Currently they are warned if they try to save with two NCI:PT. " is not correct. Currently editors are prevented from Saving if they try to Save with two NCI:PT and a warning message appears. We would want something similar to happen if there are two NCI:DEF.

[bdionne]

Note the original statement "Currently they are warned if they try to save with two NCI:PT. " is not correct.

Yes, I noted that. We only warn about FULL_SYN, not DEFINITION

So the requirement is:

• There can be only one DEFINITION property with a source of NCI

To me the best approach would be to:

• In the normal property table, let them do it, but then prevent it at save time, as we do for FULL_SYN

• In the split/copy complex operations, pre-populate the definition in the dialog box with the existing one and let the user tweak it if needed, and then only use that one. In other words don't copy the original as a second definition.

  • In the merge, use the surviving definition. If the user wants they can always go ahead and edit it separately

[hahndane]

In the copy we could use the definition the user adds to the dialog box and not copy the definition from the existing concept.

However, in the split and merge the user may want the original def in either the old or new concept (Split) or in the surviving concept (Merge), so we should copy the original DEF (the one from the old concept or the retiring partner). I guess we could make an exception for the new concept from a split if a new definition is added in the dialog box but we can't assume which NCI DEF an editor would keep for the surviving partner of a merge.

But it may be easier just to allow the current process to occur (with the potential creation of 2 NCI DEF) and then prevent the retention 2 NCI DEF at the Save as we do for the NCI PT.

[bdionne]

But it may be easier just to allow the current process to occur (with the potential creation of 2 NCI DEF) and then prevent the retention 2 NCI DEF at the Save as we do for the NCI PT.

My sense is that it would be simpler, from the modeler perspective.

I suggested the previous approach assuming that copy really meant something like clone+tweak, in which case you'd want the original one, especially if it's lengthy, so that you could just tweak a few words. So you would pre-populate the dialog with the original and then use that or whatever tweaked version of it the user creates

[bdionne]

For now, we'll just check the rule at save time - you cannot have two definitions with NCI source

[singhik]

@NCIEVS :: Hello, QA has validated below scenarios, please second on coverage :

Scenario 1 : Creating a concept with more than one DEF pty with source as NCI, the concept should NOT be allowed to get saved .

Scenario 2 : Create a concept with more than one Full_SYN pty with source of NCI and Type PT, the concept should NOT be allowed to get saved .

Scenario 3 :: Create a concept with more than one ALT_DEF pty with source of NCI the concept should be allowed to get saved .

Scenario 4 :: Perform Copy operation from a concept already having one DEF (NCI) and the new one also has one DEF (NCI) added , the copied one(Cloned to) with 2 DEF(NCI) should NOT be allowed to get saved .

Scenario 5 :: Perform Copy operation from a concept already having one DEF (NCI) and in the new one add one DEF (Non- NCI), the copied one(Cloned to) with 2 DEF ( one with NCI and other Non NCI) should be allowed to get saved .

Scenario 6 ::

Perform Split operation from a concept already having one DEF (NCI) and in the new one also add one DEF (NCI), the Split To Concept with 2 DEF(NCI) should NOT be allowed to get saved .

Scenario 7 ::

Perform Split operation from a concept already having one DEF (NCI) and in the new one adding one DEF (Non- NCI), the Split To Concept with 2 DEF(NCI and Non NCI ) should be allowed to get saved .

Scenario 8 :: Perform Merge operation from one concept(Retiring) already having one DEF (NCI) and other Concept(Surviving) also having one DEF (NCI), the Surviving Concept (having two DEF with NCI) should NOT be allowed to get saved .

Scenario 9 : Performing Merge Operation between two concepts, where one has DEF with NCI source and other has DEF with Non-NCI Source , Merge should be successful and allowed to get saved .

Scenario 10 : Performing Merge Operation between two concepts, where one has ALT_DEF with NCI source and other has ALT_DEF with Non-NCI Source, Merge should be successful and allowed to get saved .

[NCIEVS]

@singhik , yes, this coverage looks good. In the future, we might modify scenario 3, but this is good for the present.

[hahndane]

These appear to be correct; although, some of these scenarios are not as "real" as others. I would add one more scenario.

Scenario 10 : Performing Merge Operation between two concepts, where one has ALT_DEF with NCI source and other has ALT_DEF with Non-NCI Source, Merge should be successful and allowed to get saved .

Thanks,

Liz

Elizabeth Hahn-Dantona, Ph.D. (Contractor) Scientific Information Specialist Enterprise Vocabulary Services (EVS) National Cancer Institute Center for Biomedical Informatics & Information Technology Medical Science & Computing 9609 Medical Center Dr., Rm 1W844 Rockville, MD 20850

HahnDanE@mail.nih.govmailto:HahnDanE@mail.nih.gov 240.276.5165

---

From: singhik notifications@github.com Sent: Tuesday, September 4, 2018 2:20:04 PM To: NCIEVS/nci-protege5 Cc: Hahn-Dantona, Elizabeth (NIH/NCI) [C]; Mention Subject: Re: [NCIEVS/nci-protege5] Cloning issues can lead to multiple NCIt Definitions (#398)

@NCIEVShttps://github.com/NCIEVS :: Hello, QA has validated below scenarios, please second on coverage :

Scenario 1 : Creating a concept with more than one DEF pty with source as NCI, the concept should NOT be allowed to get saved .

Scenario 2 : Create a concept with more than one Full_SYN pty with source of NCI and Type PT, the concept should NOT be allowed to get saved .

Scenario 3 :: Create a concept with more than one ALT_DEF pty with source of NCI the concept should be allowed to get saved .

Scenario 4 :: Perform Copy operation from a concept already having one DEF (NCI) and the new one also has one DEF (NCI) added , the copied one(Cloned to) with 2 DEF(NCI) should NOT be allowed to get saved .

Scenario 5 :: Perform Copy operation from a concept already having one DEF (NCI) and in the new one add one DEF (Non- NCI), the copied one(Cloned to) with 2 DEF ( one with NCI and other Non NCI) should be allowed to get saved .

Scenario 6 ::

Perform Split operation from a concept already having one DEF (NCI) and in the new one also add one DEF (NCI), the Split To Concept with 2 DEF(NCI) should NOT be allowed to get saved .

Scenario 7 ::

Perform Split operation from a concept already having one DEF (NCI) and in the new one adding one DEF (Non- NCI), the Split To Concept with 2 DEF(NCI and Non NCI ) should be allowed to get saved .

Scenario 8 :: Perform Merge operation from one concept(Retiring) already having one DEF (NCI) and other Concept(Surviving) also having one DEF (NCI), the Surviving Concept (having two DEF with NCI) should NOT be allowed to get saved .

Scenario 9 : Performing Merge Operation between two concepts, where one has DEF with NCI source and other has DEF with Non-NCI Source , Merge should be successful and allowed to get saved .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/NCIEVS/nci-protege5/issues/398#issuecomment-418468442, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AUW1EXgy40--tp4-ohURtN0adPnVdzzaks5uXsRUgaJpZM4VkdGn.

[singhik]

Thanks @NCIEVS and Liz, Scenario 10 has been QA validated as well , hence closing the ticket .