Fix PHP Warnings in Management Settings

[Squidly271]

Summary by CodeRabbit

• Bug Fixes
  • Enhanced remote access configuration handling to gracefully manage missing or undefined parameter values.
  • Improved overall system stability through safer default handling of optional settings that may not be present.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Two files updated to improve defensive programming. Connect.page now uses null coalescing operators to safely access potentially missing remote configuration values. UnraidCheck.php method signature updated to accept a nullable array parameter, improving type safety against missing data.

Changes

Cohort / File(s)	Summary
Remote config safety plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page	Replaces direct array accesses with null coalescing defaults (??) for dynamicRemoteAccessType, upnpEnabled, wanaccess, and wanport to avoid undefined index warnings when configuration keys are missing.
Method signature update plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.plugin.manager/include/UnraidCheck.php	Updates private method safe_http_get_contents() third parameter from array &$getinfo to ?array &$getinfo, allowing nullable array reference for defensive typing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Verify null coalescing operators use appropriate defaults (null vs. empty string) for each configuration key context
• Confirm nullable parameter change in UnraidCheck.php doesn't introduce type mismatches at call sites

Poem

🐰 When configs go missing, the code holds its ground,
With null checks and shields, no errors abound!
Safe access patterns make logic more bright,
Defensive coalescing—the rabbit's delight! ✨

Pre-merge checks and finishing touches

✅ 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 clearly and concisely summarizes the main objective: fixing PHP warnings in management settings, which aligns with the changeset modifications (adding null coalescing operators and nullable type hints to prevent undefined index/parameter warnings).
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.04%. Comparing base (31af99e) to head (3ffd05b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1805   +/-   ##
=======================================
  Coverage   52.04%   52.04%           
=======================================
  Files         874      874           
  Lines       50372    50372           
  Branches     5017     5017           
=======================================
  Hits        26214    26214           
  Misses      24083    24083           
  Partials       75       75           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page (1)

199-199: Consider using a numeric default for the WAN port.

The sprintf format specifier %u expects an unsigned integer, but defaults to an empty string "". PHP will convert this to 0, displaying "0/TCP" in the error message, which may confuse users.

Consider using the actual SSL port or a placeholder value:

-text: "<?=sprintf(_("The Unraid server is unreachable from outside your network. Be sure you have configured your router to forward port") . " <strong style='font-weight: bold'>%u/TCP</strong> " . _("to the Unraid server at") . " <strong style='font-weight: bold'>%s</strong> " . _("port") . " <strong style='font-weight: bold'>%u</strong>", $myServersFlashCfg['remote']['wanport']??"", htmlspecialchars($eth0['IPADDR:0']??''), $var['PORTSSL']??443)?>",
+text: "<?=sprintf(_("The Unraid server is unreachable from outside your network. Be sure you have configured your router to forward port") . " <strong style='font-weight: bold'>%u/TCP</strong> " . _("to the Unraid server at") . " <strong style='font-weight: bold'>%s</strong> " . _("port") . " <strong style='font-weight: bold'>%u</strong>", $myServersFlashCfg['remote']['wanport']??$var['PORTSSL']??443, htmlspecialchars($eth0['IPADDR:0']??''), $var['PORTSSL']??443)?>",

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31af99e and 3ffd05b.

📒 Files selected for processing (2)

• plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page (3 hunks)
• plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.plugin.manager/include/UnraidCheck.php (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: elibosley
Repo: unraid/api PR: 969
File: web/justfile:7-9
Timestamp: 2024-11-27T15:30:02.252Z
Learning: In the Unraid Connect project, the different implementations of the `setup` commands in `web/justfile` and `api/justfile` are intentional and correct behavior.

Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch is used to remove the old jGrowl notification system from Unraid pages, as notifications are handled by a new system implemented on a different page.

Learnt from: zackspear
Repo: unraid/api PR: 1079
File: web/scripts/deploy-dev.sh:51-54
Timestamp: 2025-01-29T00:59:26.633Z
Learning: For the Unraid web components deployment process, JS file validation isn't required in auth-request.php updates since the files come from the controlled build pipeline where we are the source.

Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/__test__/snapshots/DefaultPageLayout.php.modified.snapshot.php:203-207
Timestamp: 2025-02-03T17:21:26.738Z
Learning: The project uses patches to override existing Unraid pages rather than modifying them directly.

Learnt from: elibosley
Repo: unraid/api PR: 1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.

📚 Learning: 2025-09-04T18:42:53.531Z

Learnt from: pujitm
Repo: unraid/api PR: 1658
File: plugin/plugins/dynamix.unraid.net.plg:73-79
Timestamp: 2025-09-04T18:42:53.531Z
Learning: In the dynamix.unraid.net plugin, versions 6.12.1-6.12.14 and 6.12.15 prereleases are intentionally allowed to install with warnings (rather than immediate cleanup) to provide users with a grace period and notice before functionality is completely removed. This is a deliberate UX decision to avoid immediately breaking existing setups while encouraging upgrades.

Applied to files:

• plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.plugin.manager/include/UnraidCheck.php
• plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page

📚 Learning: 2025-05-08T19:31:52.417Z

Learnt from: elibosley
Repo: unraid/api PR: 1381
File: plugin/source/dynamix.unraid.net/usr/local/share/dynamix.unraid.net/install/scripts/verify_install.sh:11-16
Timestamp: 2025-05-08T19:31:52.417Z
Learning: The `dynamix.my.servers` namespace is still valid and should not be changed to `dynamix.unraid.net` in file paths, as both namespaces coexist in the codebase.

Applied to files:

• plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page

🔇 Additional comments (3)

plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.plugin.manager/include/UnraidCheck.php (1)

111-111: LGTM! Type safety improvement.

The nullable type hint ?array correctly resolves the type mismatch between the array type hint and the NULL default value, preventing PHP warnings while maintaining backward compatibility with existing call sites.

plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page (2)

40-42: LGTM! Good defensive programming.

The null coalescing operators correctly prevent undefined array index warnings. The logic in lines 45-53 properly handles null values by defaulting to 'OFF' when configuration values are missing.

---

71-71: LGTM! Safe null handling for JavaScript output.

The null coalescing operator prevents undefined index warnings, and outputting null as an empty string in JavaScript is appropriate for this context.