-
Notifications
You must be signed in to change notification settings - Fork 1k
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] Add a new relationship views in entities having an Attack Pattern Knowledge view (#8835) #9623
Conversation
9535c45
to
8574b42
Compare
f04aaec
to
8d1ee2f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/6.5.0 #9623 +/- ##
=================================================
+ Coverage 64.29% 64.33% +0.04%
=================================================
Files 653 653
Lines 62473 62475 +2
Branches 6920 6923 +3
=================================================
+ Hits 40166 40195 +29
+ Misses 22307 22280 -27 ☔ View full report in Codecov by Sentry. |
1b33fee
to
a58bb96
Compare
preloadedPaginationProps={preloadedPaginationProps} | ||
exportContext={{ entity_type: 'Attack-Pattern' }} | ||
additionalHeaderButtons={[ | ||
(<ToggleButton key="matrix" value="matrix" aria-label="matrix"> |
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.
would be nice if we could extract this list of buttons to avoid duplication with StixDomainObjectAttackPatternsKillChainMatrixInLine
8ac2996
to
fcf2a9c
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.
predefinedFilters.push({ key: 'elementWithTargetTypes', values: targetTypes }); | ||
} else if (direction === 'toEntity') { | ||
predefinedFilters.push({ key: 'toId', values: [entityId] }); | ||
// if (role) predefinedFilters.push({ key: 'toRole', values: [role] }); |
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.
Did you forget comments (line 302, 306)
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.
1e12ece
to
8c5bd5f
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 tested ok 👌
@@ -21,4 +22,22 @@ const useQueryLoading = <T extends OperationType>( | |||
return queryRef; | |||
}; | |||
|
|||
export const useQueryLoadingWithLoadQuery = <T extends OperationType>( | |||
query: GraphQLTaggedNode | PreloadableConcreteRequest<T>, |
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.
Do we really need to create a new method here, as the code is exactly the same as in useQueryLoading except for the return ?
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.
it's not exactly the same. We have returned loadQuery in addition to queryRef.
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.
yes, the return is different, loadQuery is returned in useQueryLoadingWithLoadQuery
but except that the method is exectly the same. I was wondering if we really need to have 2 methods
Proposed changes
Related issues
Checklist
Further comments