-
Notifications
You must be signed in to change notification settings - Fork 985
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
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #9637 +/- ##
==========================================
+ Coverage 65.25% 65.50% +0.25%
==========================================
Files 630 630
Lines 60311 60475 +164
Branches 6776 7183 +407
==========================================
+ Hits 39356 39617 +261
+ Misses 20955 20858 -97 ☔ View full report in Codecov by Sentry. |
65d188e
to
48c5ccd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally seems good
.../private/components/threats/threat_actors_individual/AddThreatActorIndividualDemographic.tsx
Outdated
Show resolved
Hide resolved
.../private/components/threats/threat_actors_individual/AddThreatActorIndividualDemographic.tsx
Outdated
Show resolved
Hide resolved
...ate/components/threats/threat_actors_individual/AddThreatActorIndividualDemographicLines.tsx
Outdated
Show resolved
Hide resolved
const [currentTargets, setCurrentTargets] = useState<string[]>(initialTargets); | ||
|
||
const updateDelete = (store: RecordSourceSelectorProxy, path: string, rootId: string, deleteId: string) => { | ||
const node = store.get(rootId); | ||
const records = node?.getLinkedRecord(path); | ||
const edges = records?.getLinkedRecords('edges'); | ||
if (!records || !edges) { return; } | ||
const newEdges = edges.filter((n) => n.getLinkedRecord('node')?.getValue('toId') !== deleteId); | ||
records.setLinkedRecords(newEdges, 'edges'); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code is a duplicate and seems pretty generic.
In fact it seems to be the same as deleteNodeFromEdge
from store.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove it and use the function from store.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleteNodeFromEdge
uses "id" while updateDelete
uses "toId"
For now, I have moved the updateDelete
method to store.js
since it is used three times.
Later, we can see if deleteNodeFromEdge
and updateDelete
can be combined into a single method.
...ate/components/threats/threat_actors_individual/AddThreatActorIndividualDemographicLines.tsx
Outdated
Show resolved
Hide resolved
...ate/components/threats/threat_actors_individual/AddThreatActorIndividualDemographicLines.tsx
Outdated
Show resolved
Hide resolved
3d2daf6
to
e6b99e3
Compare
e6b99e3
to
bffcf3f
Compare
…aThreatActorIndividual
…sThreatActorIndividual
PR status: all comments have been taken into account, ready for a second round of reviews |
Proposed changes
AddPersonaThreatActorIndividual
andAddIndividualsThreatActorIndividual
and childs components :usePreloadedQuery
instead ofuseLazyLoadQuery
,useFragment
instead ofuseRefetchableFragment
+ removeuseEffect()
Related issues
Checklist
Further comments
othe