Skip to content
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

Fix date TV parsing bug when time is hidden #16398

Merged
merged 2 commits into from
May 11, 2023

Conversation

smg6511
Copy link
Collaborator

@smg6511 smg6511 commented Feb 24, 2023

Why is it needed?

Dates without time were not rendering correctly for TVs

Some more context here.

How to test

  1. Create a new date TV with "Hide Time Option" enabled, assign it to a Template, and enter/save values in a Resource to verify correct behavior.
  2. Edit an existing date TV (with time enabled, and having already assigned/saved a value in your test Resource), enabling "Hide Time Option." Verify that the TV's saved date still shows as expected.

Related issue(s)/PR(s)

Resolves #16393

@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Feb 24, 2023
@smg6511 smg6511 added pr/review-needed Pull request requires review and testing. requires build Grunt build is required for integration labels Feb 25, 2023
@JoshuaLuckers
Copy link
Contributor

It sometimes still picks the current time instead of 00:00:00.
I have the default date and time set to: Today (midnight).

  1. Select a date and save (time is saved as 00:00:00).
  2. Empty the input and save (empty value is saved).
  3. Pick a new date (time is saved to the current time).

@smg6511
Copy link
Collaborator Author

smg6511 commented Feb 28, 2023

@JoshuaLuckers - Interesting, I'll take a look at that. It gets into how default values are evaluated and the question is: How to make the processor aware that hide time is enabled when saving that default value (might be easy, hopefully)? Will investigate in the next couple days...

@smg6511
Copy link
Collaborator Author

smg6511 commented Mar 2, 2023

@JoshuaLuckers - Alright, it was easy! Take a second look...

@codecov
Copy link

codecov bot commented Mar 2, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (e7607a2) 17.90% compared to head (8db3d61) 17.89%.

Additional details and impacted files
@@             Coverage Diff              @@
##                3.x   #16398      +/-   ##
============================================
- Coverage     17.90%   17.89%   -0.01%     
- Complexity    10480    10484       +4     
============================================
  Files           561      561              
  Lines         39240    39250      +10     
============================================
  Hits           7024     7024              
- Misses        32216    32226      +10     
Impacted Files Coverage Δ
core/src/Revolution/Processors/Resource/Create.php 55.27% <0.00%> (-0.80%) ⬇️
core/src/Revolution/Processors/Resource/Update.php 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@JoshuaLuckers JoshuaLuckers added this to the v3.0.4 milestone Mar 17, 2023
@rthrash
Copy link
Member

rthrash commented Mar 31, 2023

This pull request has been mentioned on MODX Community. There might be relevant details there:

https://community.modx.com/t/every-upgrade-from-2-to-3-ends-in-a-disaster/6587/32

@JoshuaLuckers JoshuaLuckers requested a review from Jako April 11, 2023 10:42
@JoshuaLuckers JoshuaLuckers added pr/ready-for-merging Pull request reviewed and tested and ready for merging. and removed pr/review-needed Pull request requires review and testing. labels May 5, 2023
Jim Graham and others added 2 commits May 11, 2023 08:31
Fixes date parsing issue when time is hidden

Update resource create and update processors

Ensure date TV value is saved with a zero default time when TV's hideTime enabled
@opengeek opengeek merged commit b16a856 into modxcms:3.x May 11, 2023
opengeek added a commit that referenced this pull request May 11, 2023
Update datetime.js

Fixes date parsing issue when time is hidden

Update resource create and update processors

Ensure date TV value is saved with a zero default time when TV's hideTime enabled
@smg6511 smg6511 deleted the 3.x-issue-16393 branch June 10, 2023 03:36
opengeek pushed a commit that referenced this pull request Mar 20, 2024
### What does it do?
Fixes the code in the "Resource/Create" and "Resource/Update" processors
(that was added in #16398) to also work for date TVs that were created
in MODX 2.x.

### Why is it needed?
In MODX 2.x, the input property **"hideTime"** of a Date-TV is stored as
the value "false" (or "true") -> `s:8:"hideTime";s:5:"false";`
In MODX 3, the same property is stored as the value "0" (or "1") ->
`s:8:"hideTime";s:1:"0";`

In MODX installations, that were updated from MODX 2.x to MODX 3, the
time part of the date-TV-value always gets deleted.

### How to test

- Create a TV of type "date".
- Change the value of "hideTime" from `s:1:"0";` to `s:5:"false";` in
the column "input_properties".
- Make sure, that the time part of the TV value still gets saved
correctly.

### Related topic in the MODX forum
https://community.modx.com/t/date-tv-time-wont-save/7335
opengeek pushed a commit that referenced this pull request Mar 20, 2024
### What does it do?
Fixes the code in the "Resource/Create" and "Resource/Update" processors
(that was added in #16398) to also work for date TVs that were created
in MODX 2.x.

### Why is it needed?
In MODX 2.x, the input property **"hideTime"** of a Date-TV is stored as
the value "false" (or "true") -> `s:8:"hideTime";s:5:"false";`
In MODX 3, the same property is stored as the value "0" (or "1") ->
`s:8:"hideTime";s:1:"0";`

In MODX installations, that were updated from MODX 2.x to MODX 3, the
time part of the date-TV-value always gets deleted.

### How to test

- Create a TV of type "date".
- Change the value of "hideTime" from `s:1:"0";` to `s:5:"false";` in
the column "input_properties".
- Make sure, that the time part of the TV value still gets saved
correctly.

### Related topic in the MODX forum
https://community.modx.com/t/date-tv-time-wont-save/7335
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed CLA confirmed for contributors to this PR. pr/ready-for-merging Pull request reviewed and tested and ready for merging. requires build Grunt build is required for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After enabling the hidden time option a Date TV looses its value in MODX 3.x
5 participants