alan-turing-institute / AssurancePlatform

Project to facilitate creation of Assurance Cases
MIT License
17 stars 6 forks source link

[Bug] Moving property claims does not work on certain scenarios #481

Closed cptanalatriste closed 1 week ago

cptanalatriste commented 2 weeks ago

Summary

You're not able to move Property Claims to strategies in the following scenarios:

  1. Your property claim is a child of the top-level goal.
  2. Your property claim is a child of another property claim.

Steps to reproduce

In the following case:

image

It is not possible to move P2 or P1.1

NOTE: Moving property claims under other property claims will be addressed in other issue.

What you expected to happen/ what really did happen

You should be able to move P2 or P1.1 to either S1 or S2.

Version/ platform

Issue detected on staging, after merge https://github.com/alan-turing-institute/AssurancePlatform/commit/98d6e41d069f5d6b0fbdc3c69fc9310c06976800

RichGriff commented 2 weeks ago

I have managed to allow Property Claims to move to strategies or other claims. Two issues:

cptanalatriste commented 2 weeks ago

The following commit fixes the problem with Sub-Claims: https://github.com/alan-turing-institute/AssurancePlatform/commit/f002c727720559cec8ac02c79e7a40925a3a4c45

  1. On the Front-End side, we need to clear the values of the former parent (@RichGriff , please check that the change on NodeEdit.tsx makes sense).

  2. During the development, we found recursion errors. The issue was due to Property Claims that were parent-child of itself. I have implemented backend logic that prevents this, but it would be good to also disable this logic from the front end.

    image

    In the screenshot, it is possible to assign P1.1. to itself. @RichGriff what's the most straightforward way of dealing with this? Remove it from the combo-box? An alert message?

  3. I also noticed it is not yet possible to move Property Claims to the Top-Level Goal. @chrisdburr do you need this feature? I do not think it needs much effort.

RichGriff commented 2 weeks ago

@cptanalatriste I have updated this on our bug branch https://github.com/alan-turing-institute/AssurancePlatform/commit/e3fae246e896a4d9d6c1393ab4ad4d21dc1a3caf So that the current property claim doesn't appear in the list.

RichGriff commented 2 weeks ago

@cptanalatriste to look at the identifier bug below

Screenshot 2024-06-20 at 16 47 53
cptanalatriste commented 2 weeks ago

@chrisdburr @kallewesterling , the fix for the bug reported by @RichGriff is already in staging. Please verify and close accordingly.

chrisdburr commented 1 week ago

Almost there!

However, you can't move a property claim to a goal claim.

Please make sure that all relationships as defined in this standard are supported: https://hackmd.io/F0EeWVrxRGe8f93KGOIzjA#Relationships

cptanalatriste commented 1 week ago

The bug reported by @RichGriff was fixed in PR: https://github.com/alan-turing-institute/AssurancePlatform/pull/505/files

The issue with moving Property Claims will be addressed as part of: https://github.com/alan-turing-institute/AssurancePlatform/issues/482

@chrisdburr @kallewesterling I suggest to close this issue.

chrisdburr commented 1 week ago

Thanks, @cptanalatriste