Add date/time format overrides and query offset#73
Conversation
Bump plugin to 2.1.1 and add optional date/time format overrides and an offset control for query loops. Expose dateFormat and timeFormat block attributes with editor controls (including live preview and site-default placeholders), thread those values through block rendering (SE_Blocks) into se_event_get_* functions, and apply them in SE_Date_Display_Formatter via new set_date_format/set_time_format methods. Also add an Offset RangeControl to the query-loop-events variation so editors can skip a number of events. Tests and formatting use site defaults when overrides are empty.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis release bumps package and plugin versions to 2.1.1, adds optional dateFormat and timeFormat overrides (formatter setters, new parameters on event date helpers, block.json attributes, server render plumbing, and editor inspector inputs with preview), allows selecting the block wrapper tag via tagName, removes the query cache-buster and adds an Offset RangeControl to the query-loop-events variation, and introduces Playwright E2E seeds, specs, and config that auto-discovers WP_BASE_URL. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
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 `@src/classes/class-date-display-formatter.php`:
- Around line 193-206: The setters set_date_format and set_time_format currently
store raw user-provided strings which can include HTML/JS and later be rendered;
update both methods (set_date_format and set_time_format) to sanitize the
incoming $format before assigning to $this->date_format and $this->time_format
(e.g., use a WordPress-safe sanitizer like sanitize_text_field() or
wp_strip_all_tags() to strip tags/unsafe characters and optionally validate
against a whitelist/regex of allowed date format characters), so only cleaned
format strings are stored and later rendered.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 67c38ef6-67f1-44cc-ab47-3ccdde2828c1
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
package.jsonplugin.phpsrc/blocks/loop-event-info/block.jsonsrc/blocks/loop-event-info/index.jssrc/classes/class-date-display-formatter.phpsrc/classes/class-se-blocks.phpsrc/event-functions.phpsrc/variations/query-loop-events/block.js
| public function set_date_format( string $format ): void { | ||
| $this->date_format = $format; | ||
| } | ||
|
|
||
| /** | ||
| * Override the time format used by format_time(). Empty string restores the site default. | ||
| * | ||
| * @param string $format A PHP date format string. | ||
| * | ||
| * @return void | ||
| */ | ||
| public function set_time_format( string $format ): void { | ||
| $this->time_format = $format; | ||
| } |
There was a problem hiding this comment.
Sanitize format overrides before storing to prevent HTML injection in rendered date output.
Line 193 and Line 204 accept raw strings that later flow into frontend-rendered date/time text. A format like <script>... is treated as literal output by date formatting and can be rendered unescaped.
Suggested fix
public function set_date_format( string $format ): void {
- $this->date_format = $format;
+ $this->date_format = trim( wp_strip_all_tags( $format ) );
}
@@
public function set_time_format( string $format ): void {
- $this->time_format = $format;
+ $this->time_format = trim( wp_strip_all_tags( $format ) );
}🤖 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 `@src/classes/class-date-display-formatter.php` around lines 193 - 206, The
setters set_date_format and set_time_format currently store raw user-provided
strings which can include HTML/JS and later be rendered; update both methods
(set_date_format and set_time_format) to sanitize the incoming $format before
assigning to $this->date_format and $this->time_format (e.g., use a
WordPress-safe sanitizer like sanitize_text_field() or wp_strip_all_tags() to
strip tags/unsafe characters and optionally validate against a whitelist/regex
of allowed date format characters), so only cleaned format strings are stored
and later rendered.
Introduce a new tagName attribute for the loop-event-info block (enum: div, p, h1-h6; default: div). Add a SelectControl in the block editor to choose the HTML element and pass tagName through block attributes. Update server-side rendering to validate against allowed tags and use the selected tag as the wrapper (instead of always using a div), preserving existing wrapper attributes and classes. This enables better semantic markup control for the block.
|
Ive added some additional changes here I remove the cache busting as this was causing "unsaved changes" issues at times when no changes had been made. In doing this, i patched an issue with the editor in how well it handles date and order changes |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/e2e/specs/editor/query-loop-parity.spec.js (1)
2-3: ⚡ Quick winUse
execFileSyncwith argument array instead of shell-stringexecSync.Replace the shell command string with
execFileSyncand an arguments array to eliminate shell parsing and reduce command-injection risk.Affected location: lines 25-28
const out = execFileSync( 'npx', ['wp-env', 'run', 'cli', '--env-cwd=wp-content/plugins/simple-events', '--', 'wp', 'eval-file', 'tests/e2e/fixtures/seed-parity.php'], { encoding: 'utf8' } );🤖 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 `@tests/e2e/specs/editor/query-loop-parity.spec.js` around lines 2 - 3, The test uses child_process.execSync with a shell command string which is unsafe; replace the use of execSync (and the variable/assignment that captures its output, e.g., the const out = execSync(...)) with child_process.execFileSync and pass the command as the executable 'npx' plus an explicit arguments array (['wp-env','run','cli','--env-cwd=wp-content/plugins/simple-events','--','wp','eval-file','tests/e2e/fixtures/seed-parity.php']) and preserve the encoding option ({ encoding: 'utf8' }) so the output is identical but avoids shell parsing and injection risks.tests/e2e/playwright.config.js (1)
2-3: ⚡ Quick winUse argv-based process execution instead of shell-string
execSync.This pattern is shell-parsed and avoids shell parsing hazards. Prefer
execFileSync('npx', [...])for more explicit and safer command execution.Safer invocation pattern
-const { execSync } = require( 'child_process' ); +const { execFileSync } = require( 'child_process' ); ... - const url = execSync( - "npx wp-env run cli --env-cwd='wp-content/plugins/simple-events' -- wp option get home", - { cwd: path.join( __dirname, '../..' ), stdio: [ 'ignore', 'pipe', 'ignore' ] } - ) + const url = execFileSync( + 'npx', + [ + 'wp-env', + 'run', + 'cli', + '--env-cwd=wp-content/plugins/simple-events', + '--', + 'wp', + 'option', + 'get', + 'home', + ], + { cwd: path.join( __dirname, '../..' ), stdio: [ 'ignore', 'pipe', 'ignore' ] } + ) .toString()🤖 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 `@tests/e2e/playwright.config.js` around lines 2 - 3, The code currently requires execSync from child_process and uses shell-string execution; replace use of execSync with execFileSync to avoid shell parsing hazards: import/require execFileSync from 'child_process' instead of execSync, and change any execSync('npx ...') calls to execFileSync('npx', ['<cmd>', '<arg1>', ...], { stdio: 'inherit' }) (or appropriate args), preserving any environment/options; update references in this file from execSync to execFileSync and ensure arguments are passed as an argv array rather than a single shell string.
🤖 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 `@src/classes/class-se-block-variations.php`:
- Around line 127-141: The set_admin_query method currently forces
args['post_type'] to SE_Event_Post_Type::$event_date_post_type for every
rest_se-event_query request; restrict this by first checking the incoming
$request to ensure it targets the Query Loop "events" variation (e.g. check a
variation/blockName/queryLoop-specific request param such as 'variation' or
'blockName' used in your codebase), and only then set args['post_type'] and call
set_event_query_args; otherwise leave $args untouched and return them so
non-Query-Loop REST consumers aren’t affected.
In `@tests/e2e/fixtures/seed-parity.php`:
- Around line 21-39: The seed fixture currently calls get_posts(...) and
wp_delete_post(...) to permanently delete posts (se-event, se-event-date, and
pages matching $prefix) without any environment guard; add a protective check at
the top of this block (e.g., require WP_ENV/test-only, WP_LOCAL_DEV, or an env
var like getenv('WP_ENV') === 'test' or === 'local') and bail out (throw or
return) if the process is not running in a permitted ephemeral environment so
the destructive delete loops (the get_posts + wp_delete_post calls) only run in
safe contexts.
---
Nitpick comments:
In `@tests/e2e/playwright.config.js`:
- Around line 2-3: The code currently requires execSync from child_process and
uses shell-string execution; replace use of execSync with execFileSync to avoid
shell parsing hazards: import/require execFileSync from 'child_process' instead
of execSync, and change any execSync('npx ...') calls to execFileSync('npx',
['<cmd>', '<arg1>', ...], { stdio: 'inherit' }) (or appropriate args),
preserving any environment/options; update references in this file from execSync
to execFileSync and ensure arguments are passed as an argv array rather than a
single shell string.
In `@tests/e2e/specs/editor/query-loop-parity.spec.js`:
- Around line 2-3: The test uses child_process.execSync with a shell command
string which is unsafe; replace the use of execSync (and the variable/assignment
that captures its output, e.g., the const out = execSync(...)) with
child_process.execFileSync and pass the command as the executable 'npx' plus an
explicit arguments array
(['wp-env','run','cli','--env-cwd=wp-content/plugins/simple-events','--','wp','eval-file','tests/e2e/fixtures/seed-parity.php'])
and preserve the encoding option ({ encoding: 'utf8' }) so the output is
identical but avoids shell parsing and injection risks.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5a26b67-f3aa-4653-9827-877c27f7e3cd
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
.github/workflows/e2e-tests.yml.gitignoreREADME.mdpackage.jsonsrc/classes/class-se-block-variations.phpsrc/variations/query-loop-events/block.jstests/e2e/.env.exampletests/e2e/fixtures/seed-loop.phptests/e2e/fixtures/seed-parity.phptests/e2e/global-setup.jstests/e2e/playwright.config.jstests/e2e/specs/editor/query-loop-parity.spec.jstests/e2e/specs/frontend/loop-event-info.spec.js
💤 Files with no reviewable changes (1)
- .github/workflows/e2e-tests.yml
✅ Files skipped from review due to trivial changes (4)
- .gitignore
- README.md
- tests/e2e/fixtures/seed-loop.php
- tests/e2e/.env.example
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
gin0115
left a comment
There was a problem hiding this comment.
made a few other changes and added tests
Bump plugin to 2.1.1 and add optional date/time format overrides and an offset control for query loops. Expose dateFormat and timeFormat block attributes with editor controls (including live preview and site-default placeholders), thread those values through block rendering (SE_Blocks) into se_event_get_* functions, and apply them in SE_Date_Display_Formatter via new set_date_format/set_time_format methods. Also add an Offset RangeControl to the query-loop-events variation so editors can skip a number of events. Tests and formatting use site defaults when overrides are empty.
Changes proposed in this Pull Request
Summary by CodeRabbit
Chores
New Features
Bug Fixes
Tests
Documentation