Skip to content

AAP-86117 DRY up assignment classification and single-actor fallback - #1107

Open
AlanCoding wants to merge 1 commit into
ansible:develfrom
AlanCoding:AAP-86117-dry-bulk-fetch
Open

AAP-86117 DRY up assignment classification and single-actor fallback#1107
AlanCoding wants to merge 1 commit into
ansible:develfrom
AlanCoding:AAP-86117-dry-bulk-fetch

Conversation

@AlanCoding

@AlanCoding AlanCoding commented Aug 18, 2026

Copy link
Copy Markdown
Member

Summary

  • Extract _classify_assignment_type static method to deduplicate the global/team/direct/indirect classification logic that was repeated in both summarize_assignment_list and _bulk_fetch_assignments
  • Eliminate the single-actor fallback in get_object_role_assignments — it now always routes through _bulk_fetch_assignments, removing a separate code path that used queries.py helpers + summarize_assignment_list
  • Simplify the prefetch guard in get_object_role_assignments by always setting _prefetched_assignments when a parent exists

Follows up on #1096 which introduced _bulk_fetch_assignments but left the old per-actor path in place.

Test plan

  • test_app/tests/rbac/api/test_access_lists.py — 37 tests pass
  • test_app/tests/rbac/api/test_access_assignment_uuid_lookup.py — passes
  • test_app/tests/rbac/remote/test_remote_access_list.py — passes
  • test_app/tests/rbac/test_permission_evaluation.py — passes
  • flake8 clean

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Improvements
    • Improved consistency when displaying access and role assignments.
    • Streamlined processing of bulk assignments for more efficient results.
    • Improved performance when loading role assignments, including views that display individual objects.
    • Assignment summaries now provide more consistent classification across supported views.

@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b6a96471-e3e6-46c8-87a8-ce0c98744a2e

📥 Commits

Reviewing files that changed from the base of the PR and between 484f252 and d1e562d.

📒 Files selected for processing (1)
  • ansible_base/rbac/api/serializers.py

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


📝 Walkthrough

Walkthrough

AccessListMixin now shares assignment-type classification across summary and bulk assignment paths. Object-role assignment retrieval consistently uses cached parent results and bulk fetching.

Changes

RBAC assignment processing

Layer / File(s) Summary
Centralize assignment classification
ansible_base/rbac/api/serializers.py
_classify_assignment_type now supports assignment summaries and bulk assignment processing. Summary construction uses a list comprehension.
Use bulk object-role retrieval
ansible_base/rbac/api/serializers.py
Object-role assignment retrieval reuses parent-level bulk results and uses bulk fetching when no parent serializer exists.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to d1e56

The assignment classification and lookup paths are being simplified without any identified merge-blocking risk; the PR is merge-ready after normal checks and review.

Suggested reviewers: alexscorey, kdelee, lallen92

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the deduplication of assignment classification and removal of the single-actor fallback.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.78%. Comparing base (5d64aaf) to head (b7e19f9).

@@            Coverage Diff             @@
##            devel    #1107      +/-   ##
==========================================
+ Coverage   94.77%   94.78%   +0.01%     
==========================================
  Files         259      259              
  Lines       14485    14468      -17     
  Branches     2218     2211       -7     
==========================================
- Hits        13728    13714      -14     
+ Misses        757      754       -3     
Flag Coverage Δ
py312 94.75% <100.00%> (+0.01%) ⬆️
py312-sqlite 94.13% <100.00%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
ansible_base/rbac/api/serializers.py 93.67% <100.00%> (+0.71%) ⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d64aaf...b7e19f9. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AlanCoding
AlanCoding force-pushed the AAP-86117-dry-bulk-fetch branch from d1e562d to e461e00 Compare August 18, 2026 19:27
…in access list serializer

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@AlanCoding
AlanCoding force-pushed the AAP-86117-dry-bulk-fetch branch from e461e00 to b7e19f9 Compare August 20, 2026 16:19
@github-actions

Copy link
Copy Markdown

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

@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