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.

Sign up for GitHub

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

Jump to bottom

[frontend/backend] add filters based on schema definition in the Add entities panels #6086

Merged
merged 4 commits into from
Feb 27, 2024
Merged

[frontend/backend] add filters based on schema definition in the Add entities panels #6086

merged 4 commits into from
Feb 27, 2024

Conversation

Archidoit
Copy link
Member

@Archidoit Archidoit commented Feb 22, 2024 •
edited
Loading

Add filters in some 'Add entities' panels :

  • container > Entities > add entities
  • container > observables > add entities
  • investigation > add entities

@Archidoit Archidoit added filigran team use to identify PR from the Filigran team filters & search Linked to search results and filtering engine labels Feb 22, 2024
@Archidoit Archidoit self-assigned this Feb 22, 2024
@Archidoit Archidoit marked this pull request as draft February 22, 2024 16:45
@Archidoit Archidoit changed the title [frontend/backend] add filters in the Add entities panels [frontend/backend] add filters based on schema definition in the Add entities panels Feb 22, 2024
@codecov Codecov
Copy link

codecov bot commented Feb 22, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.58%. Comparing base (bde14b4) to head (eb6c942).
Report is 4 commits behind head on master.

❗ Current head eb6c942 differs from pull request most recent head 66e07e4. Consider uploading reports for the commit 66e07e4 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #6086      +/-   ##
==========================================
- Coverage   66.73%   66.58%   -0.16%     
==========================================
  Files         540      540              
  Lines       64184    64270      +86     
  Branches     5241     5224      -17     
==========================================
- Hits        42833    42793      -40     
- Misses      21351    21477     +126

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

@Archidoit Archidoit marked this pull request as ready for review February 23, 2024 08:12
@Archidoit Archidoit marked this pull request as draft February 23, 2024 08:27
@Archidoit Archidoit marked this pull request as ready for review February 23, 2024 08:32
@lndrtrbn lndrtrbn self-requested a review February 26, 2024 15:16
jpkha
jpkha reviewed Feb 26, 2024
View reviewed changes
...i-front/src/private/components/workspaces/investigations/InvestigationAddStixCoreObjects.jsx
Comment on lines +184 to +181
handleAddFilter={helpers.handleAddFilter}
handleRemoveFilter={helpers.handleRemoveFilter}
handleSwitchLocalMode={helpers.handleSwitchLocalMode}
handleSwitchGlobalMode={helpers.handleSwitchGlobalMode}
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure about the impact as we had many children components that passed these props but we should try to remove it later.
With helpers we can use all the functions that is related to this..

Copy link
Member Author

Choose a reason for hiding this comment

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

yes it would be nice to refacto this later

Copy link
Member Author

Choose a reason for hiding this comment

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

(we pass those children + 'helpers' everywhere when listLines is called for the moment)

Copy link
Member Author

@Archidoit Archidoit Feb 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

this refacto was not done for the moment because there are about 100 files to modify and the release is soon

lndrtrbn
lndrtrbn reviewed Feb 26, 2024
View reviewed changes
...tform/opencti-front/src/private/components/common/containers/ContainerAddStixCoreObjects.jsx Show resolved Hide resolved
...tform/opencti-front/src/private/components/common/containers/ContainerAddStixCoreObjects.jsx Show resolved Hide resolved
...tform/opencti-front/src/private/components/common/containers/ContainerAddStixCoreObjects.jsx
};

const LOCAL_STORAGE_KEY = `container-${containerId}-add-objects`;
const { viewStorage, helpers, paginationOptions: addObjectsPaginationOptions } = usePaginationLocalStorage(
Copy link
Member

Choose a reason for hiding this comment

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

What is the goal of the hook? The filters should be saved in local storage?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. But not only the filters but there are some other utilities.

Copy link
Member

Choose a reason for hiding this comment

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

But we are not saving it in local storage here? Or at least we are not fetching what is in local storage when loading the page as if I refresh I don't see the previous filters I made

Copy link
Member

Choose a reason for hiding this comment

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

Yes, you are correct. I'm unsure about the intention here. The saving in localStorage is functional, but upon closing the drawer, we reset the state and clear all filters.
https://github.com/OpenCTI-Platform/opencti/pull/6086/files/eb6c9427dcab8747dfda8053e566206e15ad277a#diff-c501c533b095ec5c7f5b484df27e33d102e40b4369f12d850a58a82b465ea31bR419-R421

I believe we're utilizing the useLocalStoragePagination for the infinite loader rather than for storing the filter in localStorage.

Copy link
Member

Choose a reason for hiding this comment

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

In this history, we are using here a simple filter state. If we can confirm the behavior, I can change the code and use "useFilterState" instead.

Copy link
Member

Choose a reason for hiding this comment

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

See with the product. We will keep the state. I remove the resetState function

@Archidoit Archidoit modified the milestones: Long-term candidates, Release 6.0.0 Feb 26, 2024
@jpkha jpkha requested a review from lndrtrbn February 27, 2024 13:17
lndrtrbn
lndrtrbn approved these changes Feb 27, 2024
View reviewed changes
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.

Ok for me, need to resolve conflicts

@Kedae Kedae removed this from the Release 6.0.0 milestone Feb 27, 2024
@Kedae Kedae merged commit d419e6d into master Feb 27, 2024
4 of 6 checks passed
@Kedae Kedae deleted the issue/4939-3 branch February 27, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpkha jpkha jpkha left review comments

@lndrtrbn lndrtrbn lndrtrbn approved these changes

Assignees

@Archidoit Archidoit

Labels
filigran team use to identify PR from the Filigran team filters & search Linked to search results and filtering engine
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Archidoit @jpkha @lndrtrbn @Kedae