Skip to content

Conversation

@RaduCristianPopescu
Copy link
Contributor

@RaduCristianPopescu RaduCristianPopescu commented Aug 14, 2025

Summary

Added a new tab in Settings, allowing users to create custom cron schedules.

Will affect visual aspect of the product

YES

Screenshots

CleanShot 2025-08-14 at 16 04 44@2x CleanShot 2025-08-14 at 17 42 41@2x

Closes https://github.com/Codeinwp/feedzy-rss-feeds-pro/issues/913

@RaduCristianPopescu RaduCristianPopescu self-assigned this Aug 14, 2025
@RaduCristianPopescu RaduCristianPopescu added the pr-checklist-skip Allow this Pull Request to skip checklist. label Aug 14, 2025
@pirate-bot pirate-bot added the pr-checklist-complete The Pull Request checklist is complete. (automatic label) label Aug 14, 2025
@pirate-bot
Copy link
Contributor

pirate-bot commented Aug 14, 2025

Plugin build for 8b553ae is ready 🛎️!

Note

You can preview the changes in the Playground

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a new "Schedules" tab to the Feedzy RSS Feeds settings page, allowing users to create and manage custom cron schedules. The feature provides a UI for adding schedules with interval (in seconds), display name, and internal name, along with the ability to delete existing schedules.

Key Changes:

  • Added a new "Schedules" tab with form inputs for creating custom cron schedules
  • Implemented JavaScript functionality for adding/removing schedules dynamically
  • Added backend processing to save custom schedules and integrate them with WordPress cron system

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
includes/layouts/settings.php Added the schedules tab UI with form inputs and table display
js/feedzy-setting.js Added JavaScript handlers for adding and deleting custom schedules
includes/admin/feedzy-rss-feeds-admin.php Added backend processing for saving schedules and WordPress cron integration
includes/feedzy-rss-feeds.php Registered the custom cron schedules filter hook
css/settings.css Added styling for the schedules table and form elements

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

public function append_custom_cron_schedules( $schedules ) {

$saved_settings = apply_filters( 'feedzy_get_settings', array() );
if ( ! empty( $saved_settings['custom_schedules'] ) || ! is_array( $saved_settings['custom_schedules'] ) ) {
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logical operator should be && instead of ||. Currently, the condition will always be true because ! empty($saved_settings['custom_schedules']) and ! is_array($saved_settings['custom_schedules']) cannot both be false simultaneously. This means the code will execute even when custom_schedules is empty or not an array.

Suggested change
if ( ! empty( $saved_settings['custom_schedules'] ) || ! is_array( $saved_settings['custom_schedules'] ) ) {
if ( ! empty( $saved_settings['custom_schedules'] ) && is_array( $saved_settings['custom_schedules'] ) ) {

Copilot uses AI. Check for mistakes.
const display = $('#fz-schedule-display').val();
const name = $('#fz-schedule-name').val();

if (!interval || !display || !name) {
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The validation only checks for truthy values but doesn't validate that the interval is a positive number or that the name doesn't contain special characters that could cause issues with HTML attributes or form names.

Suggested change
if (!interval || !display || !name) {
// Validate interval is a positive number
const intervalNum = Number(interval);
const nameIsValid = /^[A-Za-z0-9_]+$/.test(name);
if (
!interval ||
!display ||
!name ||
isNaN(intervalNum) ||
intervalNum <= 0 ||
!nameIsValid
) {
// Optionally, show a user-friendly error message here

Copilot uses AI. Check for mistakes.
</button>
</td>
<input type="hidden" value="${interval}" name="fz-custom-schedule-interval[${name}][interval]">
<input type="hidden" value="${display}" name="fz-custom-schedule-interval[${name}][display]">
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name variable is inserted directly into HTML without escaping, which could lead to XSS vulnerabilities if the name contains malicious content. The values should be escaped before being inserted into the HTML.

Suggested change
<input type="hidden" value="${display}" name="fz-custom-schedule-interval[${name}][display]">
<tr data-schedule="${escapeHtml(name)}">
<td class="fz-schedule-attributes">
<strong>${escapeHtml(name)}</strong>
</td>
<td class="fz-schedule-attributes">
${escapeHtml(interval)}
</td>
<td class="fz-schedule-attributes">
${escapeHtml(display)}
</td>
<td class="fz-schedule-attributes">
<button type="button" class="btn btn-outline-primary fz-delete-schedule fz-is-destructive" data-schedule="${escapeHtml(name)}">
Delete
</button>
</td>
<input type="hidden" value="${escapeHtml(interval)}" name="fz-custom-schedule-interval[${escapeHtml(name)}][interval]">
<input type="hidden" value="${escapeHtml(display)}" name="fz-custom-schedule-interval[${escapeHtml(name)}][display]">

Copilot uses AI. Check for mistakes.
</button>
</td>
<input type="hidden" value="${interval}" name="fz-custom-schedule-interval[${name}][interval]">
<input type="hidden" value="${display}" name="fz-custom-schedule-interval[${name}][display]">
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name variable is inserted directly into the data-schedule attribute without escaping, which could lead to attribute injection vulnerabilities.

Suggested change
<input type="hidden" value="${display}" name="fz-custom-schedule-interval[${name}][display]">
<button type="button" class="btn btn-outline-primary fz-delete-schedule fz-is-destructive" data-schedule="${escapedName}">
Delete
</button>
</td>
<input type="hidden" value="${interval}" name="fz-custom-schedule-interval[${escapedName}][interval]">
<input type="hidden" value="${display}" name="fz-custom-schedule-interval[${escapedName}][display]">

Copilot uses AI. Check for mistakes.
css/settings.css Outdated
font-size: 13px;
}

.fz-schedules-table {
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an extra space before the CSS selector .fz-schedules-table which should be removed for consistency.

Suggested change
.fz-schedules-table {
.fz-schedules-table {

Copilot uses AI. Check for mistakes.
@Soare-Robert-Daniel Soare-Robert-Daniel changed the title feat: added a new tab for schedules feat: add custom cron schedules workflow Aug 14, 2025
@ineagu ineagu added the doc-needed This issue requires documentation updates or additions once it has been completed. label Aug 15, 2025
@Soare-Robert-Daniel Soare-Robert-Daniel merged commit 38d0231 into development Aug 18, 2025
9 checks passed
@Soare-Robert-Daniel Soare-Robert-Daniel deleted the feat/schedule_tab branch August 18, 2025 10:26
@pirate-bot
Copy link
Contributor

🎉 This PR is included in version 5.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@pirate-bot pirate-bot added the released Indicate that an issue has been resolved and released in a particular version of the product. label Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-needed This issue requires documentation updates or additions once it has been completed. pr-checklist-complete The Pull Request checklist is complete. (automatic label) pr-checklist-skip Allow this Pull Request to skip checklist. released Indicate that an issue has been resolved and released in a particular version of the product.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants