这是indexloc提供的服务，不要输入任何密码
Skip to content

BugFix: Wrong input date on reservation edit #654

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

BugFix: Wrong input date on reservation edit #654

wants to merge 1 commit into from

Conversation

@madyel
Copy link

@madyel madyel commented Jul 4, 2025 •
edited
Loading

Fix Applied

The issue occurred because the default date was being set unconditionally, even on the reservation.php page, causing a one-day offset.

To resolve this, the following condition was added:

{if $smarty.server.SCRIPT_NAME|basename != 'reservation.php'}
    {if $DefaultDate}
        var defaultDate = {dateFormat date=$DefaultDate};
        $controlId.val(defaultDate);
    {/if}
{/if}

This ensures that the default date is only applied on pages other than reservation.php, preventing it from interfering with the existing reservation values.

As a result, the start and end dates now correctly match the selected event when editing a reservation.

@JohnVillalovos JohnVillalovos requested a review from Copilot July 4, 2025 18:04
Copilot AI reviewed Jul 4, 2025
View reviewed changes
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 prevents the default date from overwriting existing reservation dates on the reservation edit page by only applying it on other pages.

  • Wraps the default date assignment in a conditional that skips when viewing reservation.php.
  • Ensures start/end dates on reservation edit correctly reflect the selected event.
Comments suppressed due to low confidence (2)

tpl/Controls/DateSetup.tpl:19

  • [nitpick] Add a comment above this condition explaining why the default date assignment must be skipped on reservation.php, so future maintainers understand the exception.
        {if $smarty.server.SCRIPT_NAME|basename != 'reservation.php'}

tpl/Controls/DateSetup.tpl:19

  • Consider adding a test to verify that defaultDate is not applied on reservation.php, ensuring this conditional behavior remains covered and preventing regressions.
        {if $smarty.server.SCRIPT_NAME|basename != 'reservation.php'}

@JohnVillalovos
Copy link
Contributor

JohnVillalovos commented Jul 4, 2025

@madyel Need to squash down and get rid of the bad commits. Adding more commits doesn't fix it. As the comittizen check is checking all the commit messages of all the commits.

If you need help on squashing commits please let me know.

@madyel
Copy link
Author

madyel commented Jul 5, 2025

@JohnVillalovos Thanks for the tip, really appreciated! You were right — adding more commits didn’t help. I squashed everything.

@JohnVillalovos
Copy link
Contributor

JohnVillalovos commented Jul 5, 2025

Thanks for the PR. Until others can reproduce the bug I am going to wait on merging in a fix. We need more information on how to reproduce. @lucs7 is working on investigating that.

@madyel
Copy link
Author

madyel commented Jul 5, 2025

Thanks for the update! Let me know if there's anything else I can help with in the meantime.

@lucs7
Copy link
Contributor

lucs7 commented Jul 6, 2025

@madyel thanks for the PR 🚀.

I found the issue in the date to string conversion in js for any user timezone UTC+ and changed the date conversion (#657). The issue might also occur outside of reservations so I guess its better to try to get the root cause here

@lucs7 lucs7 mentioned this pull request Jul 6, 2025
Merged
@JohnVillalovos
Copy link
Contributor

JohnVillalovos commented Jul 6, 2025

@madyel Thanks for working on this! Really appreciate it.

But I went with @lucs7 PR #657 to resolve the issue.

@madyel
Copy link
Author

madyel commented Jul 7, 2025

Got it - no worries! Glad the issue is resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@madyel @JohnVillalovos @lucs7