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

[backend] manage long text filter like description and add search operator #4940 #5633

Merged
merged 17 commits into from
Feb 15, 2024

Conversation

jpkha
Copy link
Member

@jpkha jpkha commented Jan 22, 2024

@jpkha
Copy link
Member Author

jpkha commented Jan 22, 2024

starts_with and ends_with work like contains . The nil operator does not work for an empty string "", for example. We should consider it as empty, but Elasticsearch checks if the field exists only.

Copy link

codecov bot commented Jan 22, 2024

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (e915e33) 65.44% compared to head (6f9ef98) 65.49%.
Report is 3 commits behind head on master.

Files Patch % Lines
...ti-platform/opencti-graphql/src/database/engine.js 83.51% 15 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5633      +/-   ##
==========================================
+ Coverage   65.44%   65.49%   +0.05%     
==========================================
  Files         539      539              
  Lines       63063    63179     +116     
  Branches     5050     5075      +25     
==========================================
+ Hits        41271    41382     +111     
- Misses      21792    21797       +5     

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

@richard-julien richard-julien added the filigran team use to identify PR from the Filigran team label Jan 22, 2024
@jpkha jpkha force-pushed the issue/4940 branch 6 times, most recently from 4767fa0 to 4e54fc1 Compare January 29, 2024 10:30
@jpkha jpkha force-pushed the issue/4940 branch 2 times, most recently from 00fc190 to 539cfea Compare February 6, 2024 13:13
@Archidoit Archidoit linked an issue Feb 9, 2024 that may be closed by this pull request
@jpkha jpkha marked this pull request as ready for review February 9, 2024 15:40
@jpkha
Copy link
Member Author

jpkha commented Feb 9, 2024

!missing test to do

@jpkha jpkha changed the title [backend] manage long text filter like description [backend] manage long text filter like description #4940 Feb 12, 2024
@jpkha jpkha requested a review from Archidoit February 12, 2024 10:09
filters: {
mode: 'and',
filters: [{
key: ['description'],
Copy link
Member

Choose a reason for hiding this comment

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

array not required, in the front, we rather write: "key: 'description'"

filters: {
mode: 'and',
filters: [{
key: ['description'],
Copy link
Member

@Archidoit Archidoit Feb 12, 2024

Choose a reason for hiding this comment

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

same here and below

expect(queryResult.data.reports.edges.length).toEqual(1);
});

it('should find entities according to search filters on short string unorder', async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Can you complete the test title with "unordered", if that was the intent ?

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 sure. I put unorder but I think I am not grammatically correct sorry.

@Goumies
Copy link
Member

Goumies commented Feb 15, 2024

I have noticed that the description and name options in the Add filter dropdown menu are the only ones in lowercase.
Can you capitalize it?

image

@jpkha
Copy link
Member Author

jpkha commented Feb 15, 2024

I have noticed that the description and name options in the Add filter dropdown menu are the only ones in lowercase. Can you capitalize it?

image

I fixed it from the translation. Looks like in the other language we already put an upper case by default.

@Archidoit
Copy link
Member

I have noticed that the description and name options in the Add filter dropdown menu are the only ones in lowercase. Can you capitalize it?
image

I fixed it from the translation. Looks like in the other language we already put an upper case by default.

It will be fixed in 4939 where we fetch labels from the back anyway. It's not so good to translate something without a first letter in upper case with something in upper case I think.

@@ -2163,7 +2163,7 @@
"Level": "Level",
"color": "color",
"Force now": "Force now",
"name": "name",
"name": "Name",
Copy link
Member

Choose a reason for hiding this comment

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

I don't think name should be translated to Name because it changes the case. (Knowing that the 'name' and 'description' label will be ok in 4939

@jpkha jpkha merged commit 63053fa into master Feb 15, 2024
8 checks passed
@SamuelHassine SamuelHassine deleted the issue/4940 branch February 16, 2024 15:35
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 filters temporarily used to identify filter issue to facilitate filter team’s work identification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a "contains" / "do not contains" operator in new filter
4 participants