Skip to content

fix: match credential type names with substring search (AAP-81333) - #1677

Open
abhishek-paygavhan wants to merge 1 commit into
ansible:mainfrom
abhishek-paygavhan:AAP-81333
Open

fix: match credential type names with substring search (AAP-81333)#1677
abhishek-paygavhan wants to merge 1 commit into
ansible:mainfrom
abhishek-paygavhan:AAP-81333

Conversation

@abhishek-paygavhan

@abhishek-paygavhan abhishek-paygavhan commented Sep 10, 2026

Copy link
Copy Markdown

Summary

Update the EDA credential type name filter from istartswith to icontains so searches match text anywhere in a credential type name, regardless of case.

Jira: https://redhat.atlassian.net/browse/AAP-81333

What is being changed?

  • Change the name filter lookup in src/aap_eda/api/filters/credential_type.py to icontains.
  • Add an integration test covering substring matching, prefix matching, and a search with no matching results.

Why is this change needed?

Searching for Ansible does not return Red Hat Ansible Automation Platform because the previous filter only matched names beginning with the search term.

Users should be able to find credential types using partial names without manually browsing the full list.

How does this change address the issue?

The existing ?name=<term> parameter now performs case-insensitive substring matching. Searching for Ansible can therefore match Red Hat Ansible Automation Platform, while prefix searches continue to work.

Dependencies, blockers, and compatibility

  • No new dependencies or database migrations are introduced.
  • The name filter intentionally returns a broader set of matches than the previous prefix-only behavior.
  • End-to-end verification in the Create Credential UI remains pending.

Testing

  • Targeted regression test passed: test_list_credential_types_filter_name_contains.

  • Complete credential-type integration test file passed: 59 passed.

    poetry run python -m pytest tests/integration/api/test_credential_type.py -v
  • Lint checks passed, including the database migration consistency check.

    go-task lint
  • Verify the Create Credential workflow in the AAP UI: Automation Decisions → Infrastructure → Credentials → Create Credential. Search for Ansible in the Credential Type dropdown and confirm that Red Hat Ansible Automation Platform appears.

  • Verify lowercase and uppercase search terms in the UI.

Previous PRs

This PR replaces #1674 and #1675, which were closed before local validation was completed. The backend tests and lint checks listed above have now been completed successfully.

Summary by CodeRabbit

  • Bug Fixes
    • Credential type name searches now match case-insensitively across the full name, not only from the beginning.
    • Queries can return all credential types containing the specified text.

@abhishek-paygavhan
abhishek-paygavhan requested a review from a team as a code owner September 10, 2026 09:44
@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 2ce65b8c-4864-4158-85c7-5fd2c795e516

📥 Commits

Reviewing files that changed from the base of the PR and between d3f2069 and 4188704.

📒 Files selected for processing (2)
  • src/aap_eda/api/filters/credential_type.py
  • tests/integration/api/test_credential_type.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The credential type name filter now performs case-insensitive substring matching. Integration tests cover substring, prefix, and non-matching queries.

Changes

Credential type filtering

Layer / File(s) Summary
Case-insensitive name matching
src/aap_eda/api/filters/credential_type.py, tests/integration/api/test_credential_type.py
The name filter uses icontains instead of istartswith. Integration tests verify substring, prefix, and empty-result behavior.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: mkanoor, hsong-rh, andresberejnoi

Merge Risk: ⚪ Minimal · up to 41887

Credential type searches now support case-insensitive substring matching while retaining prefix matches, with coverage for the changed behavior and no current merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the change to use substring matching for credential type names.
Description check ✅ Passed The description explains what changed, why it was needed, how it resolves the issue, testing performed, compatibility impact, and the linked Jira issue. It also clearly identifies pending UI verificat…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

The credential type dropdown sent name=<term>, which used istartswith
and missed types such as "Red Hat Ansible Automation Platform" when
users searched for "Ansible". Use icontains so the filter matches
anywhere in the name.

Co-authored-by: Cursor <cursoragent@cursor.com>
@abhishek-paygavhan

Copy link
Copy Markdown
Author

Hello EDA maintainers,

Could you please review this PR and approve the pending CI workflows when you have a chance?

All 59 credential-type integration tests and local lint checks passed. The commit is now signed and shows as Verified on GitHub.

Could you also advise whether the CodeRabbit docstring coverage warning requires changes? The newly added test already includes a docstring.

Thank you for your time and guidance.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.39%. Comparing base (baa95b3) to head (e443ca4).
⚠️ Report is 1 commits behind head on main.

@@           Coverage Diff           @@
##             main    #1677   +/-   ##
=======================================
  Coverage   93.39%   93.39%           
=======================================
  Files         247      247           
  Lines       11698    11698           
=======================================
  Hits        10925    10925           
  Misses        773      773           
Flag Coverage Δ
unit-int-tests-3.12 93.39% <ø> (ø)

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

Files with missing lines Coverage Δ
src/aap_eda/api/filters/credential_type.py 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants