Skip to content

Add firewall rule helpers and structured API errors - #29

Merged
tnware merged 2 commits into
mainfrom
fix/firewall-api-and-errors
Jun 15, 2026
Merged

tnware merged 2 commits into
mainfrom
fix/firewall-api-and-errors

Conversation

@tnware

@tnware tnware commented Jun 15, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Preserve structured UniFi controller error details on UnifiAPIError
  • Add UnifiFirewallRule with _extra_fields preservation
  • Add firewall rule list/create/update/delete helpers using existing auth/retry/request conventions
  • Document firewall-rule usage and mutating-operation caution
  • Add unit coverage for error bodies, raw/typed mapping, payload construction, and mutating helper endpoints

Notes

This intentionally does not add public live-controller smoke tests or environment-specific fixtures. Controller-specific validation remains a maintainer/private validation concern, while the public repo keeps deterministic unit tests.

Validation

  • python3 -m pytest -q → 23 passed
  • python3 -m ruff check . → all checks passed
  • python3 -m build → sdist and wheel built successfully

Closes #27
Closes #28

Summary by CodeRabbit

  • New Features
    • Added firewall rule management: retrieve, create, update, and delete firewall rules
  • Documentation
    • Updated with firewall rules section and usage examples
  • Bug Fixes
    • Improved error handling with detailed error context and clearer error messages
  • Tests
    • Added comprehensive test coverage for firewall rules and error handling

@coderabbitai

coderabbitai Bot commented Jun 15, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@tnware, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 51 minutes and 54 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fcdbd0a7-785d-41d6-ba09-5762faa0f097

📥 Commits

Reviewing files that changed from the base of the PR and between 6cff40a and 360ff3d.

📒 Files selected for processing (2)
  • tests/test_api_response_handling.py
  • unifi_controller_api/api_client.py
📝 Walkthrough

Walkthrough

Adds UnifiFirewallRule dataclass model and four UnifiController methods (get_*, create_*, update_*, delete_*) for firewall rule management. Simultaneously expands UnifiAPIError to capture structured HTTP context (method, URL, status code, response JSON/text) across all request helpers. Includes tests for both features and README documentation.

Changes

Firewall Rules & Enriched Error Handling

Layer / File(s) Summary
UnifiAPIError structured context
unifi_controller_api/exceptions.py, tests/test_api_error_details.py
UnifiAPIError gains a full __init__ accepting method, url, status_code, response_text, response_json, and response; derives missing fields from the response object; extracts controller messages via _extract_controller_message and _format_context. Tests verify behavior for JSON error responses, non-JSON responses, and network-level failures with no response object.
UnifiFirewallRule dataclass and exports
unifi_controller_api/models/firewall_rule.py, unifi_controller_api/models/__init__.py, unifi_controller_api/__init__.py
New UnifiFirewallRule dataclass with optional typed fields, _extra_fields for unknown payload keys, and to_dict() serialization. Wired into models/__init__.py and the top-level package __all__.
API client error enrichment and firewall CRUD
unifi_controller_api/api_client.py
_invoke_api_call, invoke_get_rest_api_call, and _process_api_response now pass method/url/response when raising UnifiAPIError. Adds _map_firewall_rules internal helper and get_unifi_site_firewallrule, create_unifi_site_firewallrule, update_unifi_site_firewallrule, delete_unifi_site_firewallrule methods targeting /api/s/{site}/rest/firewallrule.
Tests and README docs
tests/test_firewall_rules.py, tests/test_package_smoke.py, README.md
test_firewall_rules.py covers all six CRUD scenarios using fake session/response objects. test_package_smoke.py asserts UnifiFirewallRule is publicly available. README adds a Firewall Rules section with usage examples and a caution note on private API variability.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

minor

Poem

🐰 Hopping through the network walls,
New rules appear at the firewall halls!
get, create, update, delete in a row,
Errors now tell you what you need to know.
With _extra_fields tucked safely away,
This bunny ships clean code today! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately summarizes the two main objectives: adding firewall rule helpers and implementing structured API error handling.
Linked Issues check ✅ Passed All requirements from issues #27 and #28 are met: UnifiAPIError preserves structured error details (method, URL, status_code, response_text, response_json); firewall rule helpers (get, create, update, delete) are implemented with UnifiFirewallRule model; comprehensive unit tests cover all scenarios.
Out of Scope Changes check ✅ Passed All changes are directly aligned with linked issue objectives; no extraneous modifications or scope creep detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/firewall-api-and-errors

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 and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@unifi_controller_api/api_client.py`:
- Around line 501-507: The UnifiAPIError exception being raised in the
controller-side error response handling has a hardcoded method="GET" parameter,
which is incorrect when the actual HTTP operation was POST, PUT, or DELETE.
Replace the hardcoded "GET" string with the actual HTTP method variable that was
used for the request (this should be available in the scope of the code raising
this exception). This ensures the UnifiAPIError.method attribute accurately
reflects the original request method, providing correct diagnostic information
in error contexts.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a870da0f-1dc7-4695-aaca-a64726bcdf3d

📥 Commits

Reviewing files that changed from the base of the PR and between b7d6021 and 6cff40a.

📒 Files selected for processing (9)
  • README.md
  • tests/test_api_error_details.py
  • tests/test_firewall_rules.py
  • tests/test_package_smoke.py
  • unifi_controller_api/__init__.py
  • unifi_controller_api/api_client.py
  • unifi_controller_api/exceptions.py
  • unifi_controller_api/models/__init__.py
  • unifi_controller_api/models/firewall_rule.py

Comment thread unifi_controller_api/api_client.py
@tnware
tnware merged commit bb295b6 into main Jun 15, 2026
7 checks passed
@tnware
tnware deleted the fix/firewall-api-and-errors branch June 15, 2026 17:26
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.

Add first-class firewall rule helpers and typed model support Preserve structured UniFi API error details in UnifiAPIError

1 participant