Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[frontend] roll back investigation graph after expanding a node (#3167) #6468

Merged
merged 11 commits into from Apr 8, 2024

Conversation

ValentinBouzinFiligran
Copy link
Member

@ValentinBouzinFiligran ValentinBouzinFiligran commented Mar 25, 2024

Proposed changes

  • sessionStorage history created on InvestigationGraph file
  • new button added on InvestigationGraphBar
  • translation files updated

Related issues

Checklist

  • I consider the submitted work as finished
  • I tested the code for its functionality
  • I wrote test cases for the relevant uses case
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

Copy link

codecov bot commented Mar 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.70%. Comparing base (2d71d35) to head (c02f29b).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6468   +/-   ##
=======================================
  Coverage   67.69%   67.70%           
=======================================
  Files         532      532           
  Lines       65144    65144           
  Branches     5470     5470           
=======================================
+ Hits        44102    44104    +2     
+ Misses      21042    21040    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@labo-flg labo-flg changed the title [frontend] roll back to pre expansion state added with session storag… [frontend] roll back investigation graph after expanding a node (#3167) Mar 25, 2024
@ValentinBouzinFiligran ValentinBouzinFiligran marked this pull request as ready for review March 28, 2024 08:05
@ValentinBouzinFiligran ValentinBouzinFiligran added filigran team use to identify PR from the Filigran team feature use for describing a new feature to develop labels Mar 28, 2024
@SouadHadjiat
Copy link
Member

I just tested locally :

  • Expand an entity -> I can see the button rollback expand -> rollback works very well
  • Expand an entity -> then add another entity -> then I rollback : rollback of expand is all right, but I can still see the entity that I have added after. I guess it might be ok ?
  • Expand an entity -> expand a second entity -> rollback : it rollbacks the last expand all right -> rollback again : it rollbacks the first expand which is all right too.

@labo-flg
Copy link
Member

labo-flg commented Apr 2, 2024

Expand an entity -> then add another entity -> then I rollback : rollback of expand is all right, but I can still see the entity that I have added after. I guess it might be ok ?

Rollback shall put your graph into the state before the last expansion, so any entity added after the expand shall be lost. @ValentinBouzinFiligran Am I right ?

@Archidoit
Copy link
Member

Archidoit commented Apr 4, 2024

The state after a restore doesn't always correspond to the state before the expansion

Scénario 1:
Investigation with an entity A.
Expand an entity A, add an entity B.
Click on 'restore'.
The expansion of A and the added entity B are removed -> state before the expansion.

Scénario 2:
Investigation with an entity A and an entity B.
Expand an entity A, remove an other entity B.
Click on 'restore'.
The expansion is removed but the entity B is not added -> not really the state before the expansion.

@Archidoit
Copy link
Member

Everything works well :)

Just 2 missing translations in investigation graphs that can be added in this PR :
image

image

@ValentinBouzinFiligran ValentinBouzinFiligran merged commit 8c4a973 into master Apr 8, 2024
5 checks passed
@ValentinBouzinFiligran ValentinBouzinFiligran deleted the issue/3167 branch April 8, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature use for describing a new feature to develop filigran team use to identify PR from the Filigran team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants