Fix Saving Changes in Devices Page.#1395
Conversation
📝 WalkthroughWalkthroughThe PR refactors API URL construction across two frontend files by replacing inline dynamic protocol/host/port combinations with centralized Changes
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
front/deviceDetailsTools.php (2)
224-225: Good refactoring, but note inconsistency.The use of
getApiBase()is correct, but this file now has mixed approaches to API URL construction across different functions.
268-268: Consider completing the API base URL refactoring for consistency and maintainability.Six functions in this file still construct API URLs inline using hardcoded protocol/host/port variables, while three other functions use the centralized
getApiBase()helper. This inconsistency violates the DRY principle and creates unnecessary maintenance burden across multiple functions.Refactor these functions to use
getApiBase():
speedtestcli()(line 268)traceroute()(line 319)nslookup()(line 369)wakeonlan()(line 453)copyFromDevice()(line 495)internetinfo()(line 678)This would consolidate the URL construction logic into a single, centralized location and align with the pattern already established in
manualnmapscan()and other refactored functions.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
front/deviceDetailsEdit.phpfront/deviceDetailsTools.php
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,php,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use environment variables (
NETALERTX_DB,NETALERTX_LOG, etc.) everywhere instead of hardcoding/data/dbor relative paths. Use/datafor persistent config/db,/tmpfor runtime logs/api/nginx state
Files:
front/deviceDetailsEdit.phpfront/deviceDetailsTools.php
🔇 Additional comments (4)
front/deviceDetailsEdit.php (2)
357-357: LGTM! Centralized API base URL improves consistency.The refactoring to use
getApiBase()instead of inline URL construction is a solid improvement for maintainability.
405-405: Correct usage of centralized API base.The URL construction properly leverages the
apiBasevariable defined earlier in the function.front/deviceDetailsTools.php (2)
594-594: Correct implementation of centralized API base.The refactoring properly replaces the inline URL construction.
644-644: Correct implementation of centralized API base.The change is consistent with the other refactored functions in this PR.
2 participants
Addresses #1394.
Can now save Changes from Devices Page.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.