feat: ReplaceKey check in 14 day window of regExp #2071
Conversation
WalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant RK as ReplaceKey
participant FS as File System
Caller->>RK: check(forceCheck)
alt Condition met (forceCheck true or within 7-day window)
RK->>FS: Read existing JSON (if file exists)
RK->>FS: writeJsonFile(file, data+ts, append=true)
else Condition not met
RK-->>Caller: Exit check without update
end
sequenceDiagram
participant RK as ReplaceKey
participant FS as File System
RK->>FS: Check file existence and append flag
alt File exists & append true
FS-->>RK: Return existing JSON data
RK->>RK: Merge existing data with new data
RK->>FS: Write merged JSON data to file
else
RK->>FS: Write new JSON data to file
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
emhttp/plugins/dynamix/include/ReplaceKey.php(2 hunks)
🔇 Additional comments (2)
emhttp/plugins/dynamix/include/ReplaceKey.php (2)
150-162: Improved file manipulation with append capability.The addition of the
$appendparameter to thewriteJsonFilemethod enhances its flexibility, allowing it to merge new data with existing content rather than always overwriting files. This is particularly useful for maintaining a history of events or errors.However, note that
array_merge_recursivewill create arrays of values when the same key exists in both arrays. For error logging this may be appropriate, but in other contexts you might want to consider if this behavior is desired for all use cases of this method.
193-200: Enhanced error logging with timestamps and append support.Adding timestamps to error logs and using the append functionality is a good improvement that enables tracking errors over time instead of overwriting previous entries.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
emhttp/plugins/dynamix/include/ReplaceKey.php (1)
150-162: Enhanced JSON file writing with append capabilityThe implementation of the
writeJsonFilemethod has been improved to support appending to existing JSON files. This is a good enhancement that enables more flexible data handling.However, there are a couple of considerations:
array_merge_recursivemight create nested arrays when merging arrays with the same keys, which may not always be the desired behavior.- There's no error handling for the
file_get_contentscall.Consider these improvements:
private function writeJsonFile($file, $data, $append = false) { if (!is_dir(dirname($file))) { mkdir(dirname($file)); } if ($append && file_exists($file)) { - $existing = json_decode(file_get_contents($file), true) ?: []; + $content = @file_get_contents($file); + if ($content === false) { + // Handle file read error + error_log("Failed to read file: $file"); + $existing = []; + } else { + $existing = json_decode($content, true) ?: []; + } + // Use array_merge instead if you want to replace values rather than nest them $data = array_merge_recursive($existing, $data); } file_put_contents($file, json_encode($data, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
emhttp/plugins/dynamix/include/ReplaceKey.php(2 hunks)
🔇 Additional comments (2)
emhttp/plugins/dynamix/include/ReplaceKey.php (2)
164-180: Improved check logic with time window and force flagThe updated implementation correctly implements a 14-day window check (7 days before and 7 days after the registration expiration date) and adds a
$forceCheckparameter. This makes the system more flexible and addresses the issue identified in the previous review.The condition on line 178 correctly returns early only if the check is not forced AND the current time is outside the window, which aligns with the PR's objective.
197-199: Enhanced error logging with timestamp and append modeAdding a timestamp to the error data and enabling the append mode for error logging are good improvements. This will help with debugging by preserving the history of errors and providing temporal context.
|
Removed the changes to |
|
Closing for now as additional changes are being discussed. So I don't want this included only to be potentially changed pretty quickly |
- Handles auto-extensions key check and install of extend license key with new OS Updates Expiration date Related to unraid/webgui#2071 but not 100% dependent on them. @elibosley, do we want to use the `force` param on the `check()` method for either of these pages? Additionally, what do you think about any potential integration with `UnraidCheck.php` – which is used for the UPC's "Check for Updates" button and the user configured automatically scheduled update check? <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced plugin registration and update processes with an integrated key validation step that verifies system parameters automatically. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1209573221367688
1 participant
Summary by CodeRabbit