Fix set_alias 404 handling: simplify failure check to prevent silent regression

[jokob-sk]

📌 Description

The set_alias endpoint had an overly-specific error check that would silently return HTTP 200 for any updateDeviceColumn failure that wasn't exactly "Device not found". Simplified to match the pattern used by the generic updateDeviceColumn endpoint.

Before:

if not result.get("success") and result.get("error") == "Device not found":
    return jsonify(result), 404

After:

if not result.get("success"):
    return jsonify(result), 404

---

🔍 Related Issues

---

📋 Type of Change

• 🐛 Bug fix
• ♻️ Code refactor

---

📷 Screenshots or Logs (if applicable)

N/A

---

🧪 Testing Steps

• Call set_alias with a non-existent MAC → expect HTTP 404
• Call set_alias with a valid MAC → expect HTTP 200 with {"success": true}

---

✅ Checklist

• I have read the Contribution Guidelines
• I have tested my changes locally
• I have updated relevant documentation (if applicable)
• I have verified my changes do not break existing behavior
• I am willing to respond to requested changes and feedback

---

🙋 Additional Notes

The generic updateDeviceColumn endpoint at line 465 already used if not result.get("success") — this change makes set_alias consistent with that pattern.

Summary by CodeRabbit

• Documentation

  • Clarified API semantics for device creation and field updates.
  • Added three new endpoints: field locking, bulk field unlocking, and device alias management.
  • Extended API documentation with new endpoint examples.

• Bug Fixes

  • Improved error handling and responses for device operations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 54fa67b7-10e7-4882-957e-69766625df98

📥 Commits

Reviewing files that changed from the base of the PR and between da8b694 and 16e2249.

📒 Files selected for processing (1)

• server/api_server/api_server_start.py

✅ Files skipped from review due to trivial changes (1)

• server/api_server/api_server_start.py

---

📝 Walkthrough

Walkthrough

Added three device endpoints and clarified POST semantics; expanded allowed device-column whitelist and renamed/changed defaults for lock/unlock request fields; set-alias endpoint now returns an explicit failure JSON when the underlying update fails.

Changes

Cohort / File(s)	Summary
API Documentation docs/API_DEVICE.md	Clarified POST /device/<mac> as full-replace; POST /device/<mac>/update-column as single-field partial-update with explicit columnName whitelist; documented new endpoints POST /device/<mac>/field/lock, POST /devices/fields/unlock, and POST /device/<mac>/set-alias with example curl calls and error cases.
Schema Whitelist & Request Shape server/api_server/openapi/schemas.py	Expanded ALLOWED_DEVICE_COLUMNS Literal to include many device columns (e.g., devIcon, devAlertEvents, devAlertDown, devSkipRepeated, devReqNicsOnline, devForceStatus, devParentPort, devParentRelType, devSSID, devSite, devVlan, devStaticIP, devIsNew, devIsArchived, devCustomProps); changed LockDeviceFieldRequest.lock default from True → False; renamed UnlockDeviceFieldsRequest.clear_all → clearAll (JSON key renamed, default False).
API Behavior server/api_server/api_server_start.py	api_device_set_alias now checks device_handler.updateDeviceColumn(mac, 'devName', alias) and, when result.get("success") is falsy, returns jsonify({"success": False, "error": err}) with a computed error message instead of returning result directly.

Poem

🐰 I hop through docs with a cheerful tune,
I toggle locks beneath the moon,
I clear and set with a whisker-snap,
I rename devices on my rabbit map,
New columns sprout — a carrot-sweet boon!

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: fixing the set_alias endpoint's 404 error handling by simplifying the failure check to prevent silent regressions where HTTP 200 was incorrectly returned for non-'Device not found' errors.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch next_release

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/API_DEVICE.md`:
- Around line 209-213: The docs state the `lock` default is true but runtime
defaults to false; update the API docs to reflect the actual behavior used by
api_device_field_lock (which calls data.get("lock", False)) by changing the
`lock` field default description to indicate it defaults to false (unlock) when
omitted, and optionally add a brief note referencing api_device_field_lock to
avoid future drift.
- Around line 247-252: The docs use the camelCase key `clearAll` but the API
schema defines snake_case `clear_all` in the UnlockDeviceFieldsRequest, causing
a contract mismatch; update the API_DEVICE.md table to use `clear_all` (and
describe that it is a boolean with same semantics: true clears all sources,
false/omitted clears only LOCKED/USER), or alternatively change the schema field
in UnlockDeviceFieldsRequest from `clear_all` to `clearAll` so names match—pick
one consistent naming approach, update the docs or schema accordingly, and
ensure examples and any references to `clearAll` are replaced with `clear_all`
(or vice versa) so the documentation matches the actual OpenAPI schema.
- Around line 289-293: The docs claim set-alias returns HTTP 404 for "Device not
found" but the handler api_device_set_alias currently returns jsonify(result)
with no status change; update the handler (api_device_set_alias) so that when
the backend response has success:false and error equals "Device not found" it
returns a JSON response with HTTP status 404 (e.g., jsonify(result), status=404)
to match the docs, or alternatively update docs/API_DEVICE.md to state the
handler returns 200 with a JSON error payload—choose and implement one of these
two consistent fixes and ensure the condition checks result['success'] and
result['error'] exactly as used in the existing code.

In `@server/api_server/openapi/schemas.py`:
- Around line 35-49: ALLOWED_DEVICE_COLUMNS currently includes "devCanSleep" and
"devReqNicsOnline" which are not present in the Devices DB schema and will cause
UPDATE failures; to fix, remove those two literals from the
ALLOWED_DEVICE_COLUMNS Literal so the whitelist matches the actual schema used
by server/models/device_instance.py (the code that builds "UPDATE Devices SET
{column_name}=?"), or alternatively add corresponding columns to the Devices
schema if they are intended fields — ensure ALLOWED_DEVICE_COLUMNS and the
Devices schema remain in sync.
- Around line 40-47: ALLOWED_DEVICE_COLUMNS currently permits sensitive fields
like devForceStatus, devIsArchived, and devIsNew without per-field checks;
update the device column update handler (the /device/<mac>/update-column
endpoint implementation—e.g., the function that consumes ALLOWED_DEVICE_COLUMNS)
to enforce field-level authorization: if the requested column is one of
{"devForceStatus","devIsArchived","devIsNew"} verify the caller has an elevated
permission/role (e.g., admin or specific device-state-update scope) using the
existing token auth/permission APIs, return 403 on failure, and log unauthorized
attempts; keep existing behavior for non-sensitive columns.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 39482fe6-7f44-48b4-a65e-38c557f6eed7

📥 Commits

Reviewing files that changed from the base of the PR and between 548d237 and b530a6e.

📒 Files selected for processing (2)

• docs/API_DEVICE.md
• server/api_server/openapi/schemas.py