Skip to content

refactor: enhance db existence validation #1350

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

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

Conversation

Alex-Izquierdo
Copy link
Collaborator

Some refactoring in validators. Some of them are not used anymore or they do check for the existence of the DB by fetching the record, which is inefficient. DRF validators are expected to raise ValidationError or return None. In case of the organization_id has been created a mixin to avoid code duplication.

Refactored:

  • organization
  • rulebook
  • credential_type
  • awx_token

Removed:

  • event_streams
  • extra_vars

Technically, in most of the cases a PrimaryKey field would be better, but that would require a more extended refactor to keep 100% backward compatibility, since we would have to implement custom error messages among other changes. There are multiple fields candidate to be a PrimaryKeyField and such a conversion is not the scope of this PR.
Here I only address the fields were we are duplicating DB queries, duplicating field definition and removing unused validators.

Signed-off-by: Alex <aizquier@redhat.com>
@Alex-Izquierdo Alex-Izquierdo requested a review from a team as a code owner June 23, 2025 12:13
Copy link

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.97%. Comparing base (f82dabc) to head (4aedfe8).

@@            Coverage Diff             @@
##             main    #1350      +/-   ##
==========================================
+ Coverage   93.91%   93.97%   +0.06%     
==========================================
  Files         320      320              
  Lines       18827    18802      -25     
==========================================
- Hits        17682    17670      -12     
+ Misses       1145     1132      -13     
Flag Coverage Δ
unit-int-tests-3.11 93.92% <100.00%> (+0.06%) ⬆️
unit-int-tests-3.12 93.97% <100.00%> (+0.06%) ⬆️

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

Files with missing lines Coverage Δ
src/aap_eda/api/serializers/activation.py 96.16% <100.00%> (-0.01%) ⬇️
...rc/aap_eda/api/serializers/decision_environment.py 100.00% <100.00%> (ø)
src/aap_eda/api/serializers/eda_credential.py 99.02% <100.00%> (-0.01%) ⬇️
src/aap_eda/api/serializers/mixins.py 100.00% <100.00%> (ø)
src/aap_eda/api/serializers/project.py 93.20% <100.00%> (-0.07%) ⬇️
src/aap_eda/api/serializers/team.py 100.00% <100.00%> (ø)
src/aap_eda/core/validators.py 92.43% <100.00%> (+5.28%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

3 participants