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] Update demographic stixCoreRelationships from entity overview #9637

Merged
merged 16 commits into from
Feb 12, 2025

Conversation

marieflorescontact
Copy link
Member

@marieflorescontact marieflorescontact commented Jan 17, 2025

Proposed changes

  • Update of Citizenship / Nationality / Country of residence can be done directly from the overview tab. (alignment with others StixCoreRelationships)
  • Apply review remarks on AddPersonaThreatActorIndividual and AddIndividualsThreatActorIndividual and childs components :
    • use usePreloadedQuery instead of useLazyLoadQuery,
    • use useFragment instead of useRefetchableFragment + remove useEffect()

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 (coverage and e2e)
  • I added/update the relevant documentation (either on github or on notion)
  • Where necessary I refactored code to improve the overall quality

Further comments

othe

@github-actions github-actions bot added the filigran team use to identify PR from the Filigran team label Jan 17, 2025
@marieflorescontact marieflorescontact self-assigned this Jan 17, 2025
@marieflorescontact marieflorescontact marked this pull request as ready for review January 17, 2025 16:50
Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.28%. Comparing base (5266681) to head (3b6d100).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9637      +/-   ##
==========================================
+ Coverage   64.27%   64.28%   +0.01%     
==========================================
  Files         655      655              
  Lines       62935    62935              
  Branches     6960     6967       +7     
==========================================
+ Hits        40453    40460       +7     
+ Misses      22482    22475       -7     

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

@marieflorescontact marieflorescontact marked this pull request as draft January 20, 2025 08:09
@marieflorescontact marieflorescontact marked this pull request as ready for review January 20, 2025 08:22
@marieflorescontact marieflorescontact changed the title [frontend] Update demographic stixCoreRelationships from entity [frontend] Update demographic stixCoreRelationships from entity overview Jan 20, 2025
@lndrtrbn lndrtrbn self-requested a review January 20, 2025 09:16
Copy link
Member

@lndrtrbn lndrtrbn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionally seems good

@marieflorescontact marieflorescontact marked this pull request as draft January 20, 2025 13:07
@marieflorescontact marieflorescontact force-pushed the issue/8343 branch 2 times, most recently from e6b99e3 to bffcf3f Compare January 28, 2025 09:27
@marieflorescontact
Copy link
Member Author

PR status: all comments have been taken into account, ready for a second round of reviews

@Archidoit
Copy link
Member

Can you modify the space before '-' in case of empty citizenship / country of residence / nationality ?
image

The space is too big compared to 'date of birth' for instance

See 'labels' for model
image

@Archidoit
Copy link
Member

Can you add the possibility to also easily remove an added relationship
image

like it is the case located-at for instance
image

@Archidoit
Copy link
Member

When adding/removing a citizenship, it is also add/removed in the country of residence, and same for the other way round

image

@Archidoit
Copy link
Member

In the 'add citizenship' panel, the search bar is not working :
image

@Archidoit
Copy link
Member

When creating a country from this panel (see 'create country' button), the country is created but the list is not updated
image

Maybe consider displaying the list of countries in a data table here, like it is done in other relationships creation form? (only a suggestion, don't know if it will help solving the search bar issue etc) , exemple:
image

@marieflorescontact
Copy link
Member Author

Can you modify the space before '-' in case of empty citizenship / country of residence / nationality ? ![image](https://private-
The space is too big compared to 'date of birth' for instance

Fix => I've created a component to avoid multiple margin issue between labels and value
image

@marieflorescontact
Copy link
Member Author

Can you add the possibility to also easily remove an added relationship ![image](https://private-user-
like it is the case located-at for instance ![image](https://private-user-images.githubusercontent.com/75783086/409479210-

Good idea but it needs to be discussed with product

@marieflorescontact
Copy link
Member Author

PR status:
all the comments have been taken into account. except for the use of datatables and the addition of the ‘quick delete relationship’, which are outside the scope of the bug fix.
there is still some duplicated code but I've already spent a lot of time on this task so it'll come later. 😅:

<AddIndividualsThreatActorIndividualLines>
<AddPersonasThreatActorIndividualLines>
<AddThreatActorIndividualDemographicLines>

@marieflorescontact marieflorescontact force-pushed the issue/8343 branch 2 times, most recently from 05840ea to 589f9a2 Compare February 7, 2025 16:30
@marieflorescontact marieflorescontact merged commit 09aa9c6 into master Feb 12, 2025
9 checks passed
@marieflorescontact marieflorescontact deleted the issue/8343 branch February 12, 2025 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
filigran team use to identify PR from the Filigran team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent behavior on overview of demographics
3 participants