alan-turing-institute / AssurancePlatform

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

[BUG] Duplicate Identifiers before "Reset Identifiers" request #537

Closed cptanalatriste closed 1 day ago

cptanalatriste commented 3 weeks ago

Summary

When editing an Assurance Case, we end up with Property Claims with the same ID (see screenshot). While this is fixed when calling "Reset Identifiers" , this shouldn't be the case

Steps to reproduce

For the following case:

image

I did the following:

  1. Created G1
  2. Created S1
  3. Created S2
  4. Created P1, under S1
  5. Created a Priority Claim under S2, ending on a duplicate P1

What you expected to happen/ what really did happen

The Property Claim created in 5. should have been P2.

Version/ platform

Issue detected on staging on July 10th.

cptanalatriste commented 3 weeks ago

@chrisdburr , please prioritise and assign it to a Sprint.

chrisdburr commented 2 weeks ago

Thanks, @cptanalatriste. I think we may need to extend the current sprint as it fits within the current priorities.

RichGriff commented 2 weeks ago

Currently this issue occurs because we are attempting to set the name of the element based on its siblings, when adding the new element. There are two options to fix this:

I think the first option would work best as it keeps all the identifier logic in a single place.

@chrisdburr @cptanalatriste thoughts?

cptanalatriste commented 2 weeks ago

@RichGriff the problem with option one is that we can still end up with duplicate IDs. Imagine the following scenario:

  1. Create E1
  2. Create E2
  3. Delete E1
  4. Create new evidence: The "Update Identifier" algorithm should assign E2, and we now have a duplicate with the evidence created in 2.

This problem can be solved by updating all the identifiers, when creating/deleting elements. We tried this before (although with a page reload) and it's not a great user experience. And personally, I don't like that a minor change have an effect in the whole document (like when adding an image in Word, it affects the whole document structure).

I would suggest a simpler approach: Just have global counters for Priorities, Strategies, and Evidence. Every new Priority, wherever is created, gets assigned a unique identifier starting with P1, P2, P3 ... (even if it's nested). This way, we ensure the following:

Then, if the user wants our elaborate "Update Identifier" logic, they can click the button and get, among others, nested priority names (like P1.1).

@chrisdburr what do you think?

cptanalatriste commented 1 week ago

@RichGriff as agreed, I modified the POST endpoints for creating elements, so they will return an adequate name for each case element (see https://github.com/alan-turing-institute/AssurancePlatform/pull/556 ). Please make the adequate frontend changes to show the returned values in the GUI.

RichGriff commented 6 days ago

@cptanalatriste has backend been updated to staging yet?

cptanalatriste commented 2 days ago

@RichGriff yes, staging is working now. I'll go ahead and move this issue to Review

chrisdburr commented 1 day ago

Seems to be working as I would expect.

Identifiers are set correctly in the order I add the elements (Figure 1)...

Image

And then get correctly updated when I click 'update identifiers' (Figure 2)

Image

Happy to close.