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

Make it possible to filter events by a given list of event types #701

Closed

Conversation

johannaengland
Copy link
Contributor

@johannaengland johannaengland commented Sep 27, 2023

Closes #699.

@github-actions
Copy link

github-actions bot commented Sep 27, 2023

Test results

       7 files     511 suites   20m 1s ⏱️
   412 tests    411 ✔️ 1 💤 0
2 884 runs  2 877 ✔️ 7 💤 0

Results for commit bee6399.

♻️ This comment has been updated with latest results.

@codecov-commenter
Copy link

codecov-commenter commented Sep 27, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.47%. Comparing base (8a3e712) to head (bee6399).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #701      +/-   ##
==========================================
+ Coverage   79.43%   79.47%   +0.03%     
==========================================
  Files          73       73              
  Lines        3608     3615       +7     
==========================================
+ Hits         2866     2873       +7     
  Misses        742      742              

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

@johannaengland johannaengland force-pushed the filter-by-event-type-list branch from ae73098 to bb98af8 Compare September 27, 2023 13:48
@johannaengland
Copy link
Contributor Author

After a discussion with @hmpf and @stveit we've come to the conclusion that the user should be forbidden from posting a filter as such:

filter: {"event_types": []}

since that would lead to no events ever being sent as notifications, since every event has an event type.

This should be explicitly documented, both as a test as well in the API documentation.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@lunkwill42
Copy link
Member

lunkwill42 commented Sep 29, 2023

I guess the big question is whether this will be impacted by the conclusions of #700 - as having an incident filter that includes event attributes is kind of useless and confusing when in the main filter editing form: The incidents list page. Filtering on event attributes here would have no discernible effect on the incidents list.

During notification processing is the only time the make sense, really...

@lunkwill42
Copy link
Member

I guess the big question is whether this will be impacted by the conclusions of #700 - as

I might go as far as claiming this PR should be labelled as blocked until we have made this design decision...

@johannaengland johannaengland added discussion Requires developer feedback/discussion before implementation blocked Another thing/issue has to be resolved before tackling this labels Sep 29, 2023
@hmpf hmpf removed the blocked Another thing/issue has to be resolved before tackling this label Jan 29, 2024
@johannaengland johannaengland force-pushed the filter-by-event-type-list branch from 9f7f544 to 98b33e7 Compare January 31, 2024 09:23
@hmpf
Copy link
Contributor

hmpf commented Feb 8, 2024

This will be merged after towncrier so add a changelog fragment in the correct place.

Copy link

sonarqubecloud bot commented Feb 9, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@hmpf hmpf added the blocked Another thing/issue has to be resolved before tackling this label Feb 9, 2024
@johannaengland johannaengland added paused Assignee is busy with things of higher priority and removed blocked Another thing/issue has to be resolved before tackling this labels Feb 9, 2024
@lunkwill42 lunkwill42 added the API Affects Argus' REST API label Apr 8, 2024
@johannaengland johannaengland force-pushed the filter-by-event-type-list branch from 16d96be to 5b5fb2f Compare April 10, 2024 13:27
When filtering notifications we have the choice to filter one event type
This change makes it to filter by a list, so that we can get
notifications about events of multiple types
@johannaengland johannaengland force-pushed the filter-by-event-type-list branch from 5b5fb2f to bee6399 Compare April 10, 2024 13:57
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@johannaengland johannaengland removed the paused Assignee is busy with things of higher priority label Apr 10, 2024
@hmpf
Copy link
Contributor

hmpf commented May 14, 2024

This is as it stands not backwards compatible. I will continue this work in a new branch, reusing the commits.

@hmpf
Copy link
Contributor

hmpf commented May 14, 2024

Replaced by #813

@hmpf hmpf closed this May 14, 2024
@johannaengland johannaengland deleted the filter-by-event-type-list branch August 30, 2024 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Affects Argus' REST API discussion Requires developer feedback/discussion before implementation
Projects
None yet
4 participants