Added ICS Feed option#728
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an ICS feed type with uploaded-file storage, ICS event parsing and recurrence support, calendar admin settings, AJAX uploads, feed registration, and related admin UI updates. ChangesICS feed support
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant AdminFileInput
participant Ajax
participant Ics_Feed
AdminFileInput->>Ajax: submit FormData to simcal_upload_ics_feed
Ajax->>Ajax: verify nonce and calendar edit capability
Ajax->>Ics_Feed: save_uploaded_file(post_id, file)
Ics_Feed-->>Ajax: return saved filename or error
Ajax-->>AdminFileInput: return JSON upload result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.2.5)PHP Warning: require(/vendor/composer/../guzzlehttp/promises/src/functions_include.php): Failed to open stream: No such file or directory in /vendor/composer/autoload_real.php on line 39 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: 3
🧹 Nitpick comments (1)
assets/js/admin.js (1)
613-657: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUser-facing strings are hardcoded in English.
Messages like
'Uploading ICS file...','Current file: ', the reload prompt, the failure message, and the injectedRemove uploaded ICS filelabel bypass translation, whereas their server-rendered counterparts inics-feed-admin.phpuse__()/esc_html_e(). Consider passing localized strings through the existingsimcal_adminlocalized object (orwp.i18n) for consistency.🤖 Prompt for 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. In `@assets/js/admin.js` around lines 613 - 657, Replace the hardcoded English strings in the ICS upload flow with localized values supplied through the existing simcal_admin object (or the established wp.i18n mechanism), including the reload prompt, upload status, current-file prefix, upload failure fallback, and injected remove-file label. Add the corresponding translations where simcal_admin is localized, while preserving the existing success and error behavior.
🤖 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 `@includes/feeds/admin/ics-feed-admin.php`:
- Around line 126-172: Run Prettier with write mode on the changed files,
including the panel-rendering block around the
simcal_ics_feed_settings_fields_before action, to normalize mixed tab/space
indentation and resolve all formatting failures reported by Prettier --check.
- Around line 255-256: Update the _ics_feed_search_query handling in the POST
save flow to unslash the submitted value with wp_unslash() and sanitize it with
sanitize_text_field() before passing it to update_post_meta(). Replace
esc_attr() for this stored value so filter_events_by_search_query compares the
plain sanitized query against event text.
In `@includes/feeds/ics-feed.php`:
- Around line 638-673: Update the RRULE iteration around fast_forward_rrule(),
$matched, and the count_limit check so COUNT tracks every recurrence in the
series, including occurrences skipped before time_min. Derive and preserve the
skipped recurrence ordinal or increment the count while fast-forwarding, then
ensure visible occurrences stop once the original COUNT is exhausted.
---
Nitpick comments:
In `@assets/js/admin.js`:
- Around line 613-657: Replace the hardcoded English strings in the ICS upload
flow with localized values supplied through the existing simcal_admin object (or
the established wp.i18n mechanism), including the reload prompt, upload status,
current-file prefix, upload failure fallback, and injected remove-file label.
Add the corresponding translations where simcal_admin is localized, while
preserving the existing success and error behavior.
🪄 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: 95aa7086-45f2-410f-8706-ca9ae1282cfe
📒 Files selected for processing (8)
assets/css/admin.cssassets/js/admin.jsincludes/admin/ajax.phpincludes/feeds/admin/ics-feed-admin.phpincludes/feeds/ics-feed.phpincludes/installation.phpincludes/objects.phppackage.json
There was a problem hiding this comment.
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 `@includes/feeds/admin/ics-feed-admin.php`:
- Around line 127-135: Move the PHP opening tag before the docblock surrounding
the simcal_ics_feed_settings_fields_before hook, so the documentation remains a
PHP comment rather than being emitted as page content. Keep the hook invocation
and its $post_id argument unchanged.
🪄 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: a4ba575c-81d1-47b6-8c71-1ab6dc0ef5cc
📒 Files selected for processing (4)
assets/css/admin.cssassets/js/admin.jsincludes/feeds/admin/ics-feed-admin.phpincludes/feeds/ics-feed.php
🚧 Files skipped from review as they are similar to previous changes (2)
- assets/js/admin.js
- includes/feeds/ics-feed.php
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
includes/feeds/admin/ics-feed-admin.php (2)
324-325: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winEnforce the 2,500-event cap server-side.
The HTML
maxattribute is bypassable; a crafted save request can persist a value above 2500 and defeat the intended event-processing limit.Proposed fix
- $max_results = isset($_POST['_ics_feed_max_results']) ? absint(esc_attr($_POST['_ics_feed_max_results'])) : 2500; + $max_results = isset($_POST['_ics_feed_max_results']) + ? min(2500, absint(esc_attr($_POST['_ics_feed_max_results']))) + : 2500;🤖 Prompt for 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. In `@includes/feeds/admin/ics-feed-admin.php` around lines 324 - 325, Update the _ics_feed_max_results handling in the admin save flow to clamp the sanitized submitted value to a maximum of 2,500 before calling update_post_meta. Preserve the existing 2,500 default when the field is absent and ensure crafted requests above the cap are persisted as 2,500.
63-66: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRegister the ICS MIME allowance before AJAX upload validation.
wp_ajax_simcal_upload_ics_feedcallssave_uploaded_file()on the uploaded file, but theupload_mimesfilter is only registered from the calendar edit screen’s constructor. Onadmin-ajax.php, that screen guard is false, so.ics/.icaluploads can be rejected before the handler runs. Register this filter for the AJAX upload path before runningwp_handle_upload().🤖 Prompt for 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. In `@includes/feeds/admin/ics-feed-admin.php` around lines 63 - 66, Update the hook initialization around simcal_is_admin_screen() and register_hooks() so the ICS MIME allowance is also registered for the wp_ajax_simcal_upload_ics_feed path. Ensure the relevant upload_mimes filter is attached before save_uploaded_file() reaches wp_handle_upload(), while preserving the existing calendar-screen behavior.
🤖 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.
Outside diff comments:
In `@includes/feeds/admin/ics-feed-admin.php`:
- Around line 324-325: Update the _ics_feed_max_results handling in the admin
save flow to clamp the sanitized submitted value to a maximum of 2,500 before
calling update_post_meta. Preserve the existing 2,500 default when the field is
absent and ensure crafted requests above the cap are persisted as 2,500.
- Around line 63-66: Update the hook initialization around
simcal_is_admin_screen() and register_hooks() so the ICS MIME allowance is also
registered for the wp_ajax_simcal_upload_ics_feed path. Ensure the relevant
upload_mimes filter is attached before save_uploaded_file() reaches
wp_handle_upload(), while preserving the existing calendar-screen behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 867e6087-3ce8-49df-9319-fc39451ea501
📒 Files selected for processing (2)
assets/css/admin.cssincludes/feeds/admin/ics-feed-admin.php
🚧 Files skipped from review as they are similar to previous changes (1)
- assets/css/admin.css
2 participants
Description: I have added new feed type ie "ICS Feed" and add fields for that to display event via ICS feed.
Clikcup: https://app.clickup.com/t/1867958/86d3e1phg
After: https://drive.google.com/file/d/1lYjaemX1WqR8ogPzk7wmHJqlUR7slwYH/view?usp=drivesdk
Summary by CodeRabbit