-
Notifications
You must be signed in to change notification settings - Fork 214
fix: for custom time format store opening closing not working in vendor store settings #3090
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
fix: for custom time format store opening closing not working in vendor store settings #3090
Conversation
…or store settings
📝 WalkthroughWalkthroughServer-side time parsing calls to dokan_convert_date_format were simplified by removing explicit input/output format arguments; client templates decouple visible time inputs from submitted values (hidden fields); client JS adds a Moment.js-compatible i18n time format helper and uses it for parsing, formatting, and validation before submit. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser as Client JS
participant Server as PHP
participant DB
User->>Browser: Enter opening/closing times (visible inputs)
Browser->>Browser: Parse/format with dokan_get_i18n_time_format_for_moment_js (moment)\nPopulate hidden inputs with standardized values
User->>Browser: Submit form
Browser->>Server: POST with hidden time fields
Server->>Server: Convert/validate times (dokan_convert_date_format)
Server->>DB: Save store timings
DB-->>Server: Persisted
Server-->>Browser: Response (saved)
Browser-->>User: Show confirmation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@templates/settings/store-form.php`:
- Around line 340-344: The client-side time formatting currently uses
moment(...).format('hh:mm a') for openValueSelected and closeValueSelected which
produces leading zeros; update both calls to use format('h:mm a') to match the
server-side PHP expectation (ensure the two lines setting openValueSelected and
closeValueSelected are changed), so the values written via
$openInputHidden.val(...) and $closeInputHidden.val(...) will have no leading
zero.
In `@templates/settings/store-time.php`:
- Around line 26-32: The value attributes for the opening time inputs are
outputting the raw $opening_time variable and must be escaped to prevent XSS;
update both the visible input (id "opening-time[<?php echo esc_attr( $day_key );
?>]" / class "opening-time") and the hidden input (name "opening_time[<?php echo
esc_attr( $day_key ); ?>]" / class "opening-time-hidden") to wrap $opening_time
with esc_attr() so the attribute values are safely escaped before rendering.
🧹 Nitpick comments (1)
templates/settings/store-form.php (1)
378-385: Same format inconsistency and missing invalid date handling.
- Same
'hh:mm a'vs'h:mm a'issue as the change handler.- If
moment()parsing fails (e.g., empty or malformed input after manual edit),format()returns"Invalid date"which would be submitted to the server.Proposed fix
const formattedOpenValue = moment( openValue, timeFormatMoment ).format( 'HH:mm' ), formattedCloseValue = moment( closeValue, timeFormatMoment ).format( 'HH:mm' ); - let openValueSelected = moment(openValue, timeFormatMoment ).format('hh:mm a'); - let closeValueSelected = moment(closeValue, timeFormatMoment ).format('hh:mm a'); + const parsedOpen = moment( openValue, timeFormatMoment ); + const parsedClose = moment( closeValue, timeFormatMoment ); + + if ( ! parsedOpen.isValid() || ! parsedClose.isValid() ) { + self.find( 'input.dokan-form-control' ).css({ 'border-color': '#F87171', 'color': '#F87171' }); + self.find( '.opening-time' ).focus(); + e.preventDefault(); + return false; + } + + let openValueSelected = parsedOpen.format( 'h:mm a' ); + let closeValueSelected = parsedClose.format( 'h:mm a' ); $openInputHidden.val(openValueSelected); $closeInputHidden.val(closeValueSelected);
templates/settings/store-form.php
Outdated
| let openValueSelected = moment( openValue, timeFormatMoment ).format('hh:mm a' ); | ||
| let closeValueSelected = moment( closeValue, timeFormatMoment ).format('hh:mm a' ); | ||
|
|
||
| $openInputHidden.val( openValueSelected ); | ||
| $closeInputHidden.val( closeValueSelected ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use 'h:mm a' format to match server-side expectations.
The 'hh:mm a' format outputs times with leading zeros (e.g., "09:30 am"), but the PHP backend uses 'g:i a' which expects no leading zeros (e.g., "9:30 am"). Use moment's single 'h' to avoid potential parsing issues.
Proposed fix
- let openValueSelected = moment( openValue, timeFormatMoment ).format('hh:mm a' );
- let closeValueSelected = moment( closeValue, timeFormatMoment ).format('hh:mm a' );
+ let openValueSelected = moment( openValue, timeFormatMoment ).format('h:mm a' );
+ let closeValueSelected = moment( closeValue, timeFormatMoment ).format('h:mm a' );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let openValueSelected = moment( openValue, timeFormatMoment ).format('hh:mm a' ); | |
| let closeValueSelected = moment( closeValue, timeFormatMoment ).format('hh:mm a' ); | |
| $openInputHidden.val( openValueSelected ); | |
| $closeInputHidden.val( closeValueSelected ); | |
| let openValueSelected = moment( openValue, timeFormatMoment ).format('h:mm a' ); | |
| let closeValueSelected = moment( closeValue, timeFormatMoment ).format('h:mm a' ); | |
| $openInputHidden.val( openValueSelected ); | |
| $closeInputHidden.val( closeValueSelected ); |
🤖 Prompt for AI Agents
In `@templates/settings/store-form.php` around lines 340 - 344, The client-side
time formatting currently uses moment(...).format('hh:mm a') for
openValueSelected and closeValueSelected which produces leading zeros; update
both calls to use format('h:mm a') to match the server-side PHP expectation
(ensure the two lines setting openValueSelected and closeValueSelected are
changed), so the values written via $openInputHidden.val(...) and
$closeInputHidden.val(...) will have no leading zero.
All Submissions:
Changes proposed in this Pull Request:
Related Pull Request(s)
Closes
How to test the changes in this Pull Request:
Changelog entry
Title
Detailed Description of the pull request. What was previous behaviour
and what will be changed in this PR.
Before Changes
Describe the issue before changes with screenshots(s).
After Changes
Describe the issue after changes with screenshot(s).
Feature Video (optional)
Link of detailed video if this PR is for a feature.
PR Self Review Checklist:
FOR PR REVIEWER ONLY:
Summary by CodeRabbit
Improvements
New Features
✏️ Tip: You can customize this high-level summary in your review settings.