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

fix: [AAP-37813] Replace EDA filters with DAB rest filters #1234

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Dostonbek1
Copy link
Member

@Dostonbek1 Dostonbek1 commented Mar 11, 2025

This PR replaces our existing filters with rest-filters app that DAB provides, which is more capable and helps us align across platform.

AAP-37813

e2e tests are passing with updated tests and api client

@codecov-commenter
Copy link

codecov-commenter commented Mar 11, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.71%. Comparing base (1140ec4) to head (1f1c5d0).

@@            Coverage Diff             @@
##             main    #1234      +/-   ##
==========================================
- Coverage   93.81%   93.71%   -0.11%     
==========================================
  Files         284      273      -11     
  Lines       15913    15785     -128     
==========================================
- Hits        14929    14793     -136     
- Misses        984      992       +8     
Flag Coverage Δ
unit-int-tests-3.11 93.65% <100.00%> (-0.11%) ⬇️
unit-int-tests-3.12 93.71% <100.00%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/aap_eda/api/views/activation.py 97.50% <100.00%> (-0.05%) ⬇️
src/aap_eda/api/views/credential_type.py 100.00% <100.00%> (ø)
src/aap_eda/api/views/decision_environment.py 100.00% <100.00%> (ø)
src/aap_eda/api/views/eda_credential.py 91.08% <100.00%> (-7.95%) ⬇️
src/aap_eda/api/views/event_stream.py 89.79% <100.00%> (-0.21%) ⬇️
src/aap_eda/api/views/organization.py 79.48% <100.00%> (-1.01%) ⬇️
src/aap_eda/api/views/project.py 97.87% <100.00%> (-0.05%) ⬇️
src/aap_eda/api/views/rulebook.py 97.46% <100.00%> (-0.10%) ⬇️
src/aap_eda/api/views/team.py 100.00% <100.00%> (ø)
src/aap_eda/api/views/user.py 97.14% <100.00%> (-0.08%) ⬇️
... and 11 more
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Dostonbek1 Dostonbek1 force-pushed the fix/AAP-37813-use-DAB-filters branch from 93196fd to 0a36f65 Compare March 11, 2025 16:34
@Dostonbek1
Copy link
Member Author

e2e test run with updated api client: https://github.com/ansible/eda-server/actions/runs/13793345570

@Dostonbek1 Dostonbek1 force-pushed the fix/AAP-37813-use-DAB-filters branch 4 times, most recently from d491e97 to f24c84a Compare March 11, 2025 20:47
@Dostonbek1 Dostonbek1 marked this pull request as ready for review March 12, 2025 01:14
@Dostonbek1 Dostonbek1 requested a review from a team as a code owner March 12, 2025 01:14
@Dostonbek1
Copy link
Member Author

@lgalis this might be a breaking change for the UI depending on how it is using the current EDA filters. Could you help us confirm?

@Alex-Izquierdo
Copy link
Collaborator

@Dostonbek1 should we put it as draft until have confirmation it will not break the UI, just to prevent accidental merges?

@Dostonbek1 Dostonbek1 marked this pull request as draft March 12, 2025 20:02
@Dostonbek1 Dostonbek1 requested review from mkanoor and a team March 13, 2025 10:45
@Dostonbek1 Dostonbek1 force-pushed the fix/AAP-37813-use-DAB-filters branch from f24c84a to 1f1c5d0 Compare March 14, 2025 11:06
@mkanoor
Copy link
Contributor

mkanoor commented Apr 1, 2025

@Dostonbek1 this looks good but we need to ensure that it works with @lgalis's UI components. Plus she would have to make changes to take advantage of the new filters which can be combined which wasn't allowed earlier. Would it be possible to build a quay.io image for UI testing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants