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 saving time for date TVs created in MODX 2.x #16505

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

halftrainedharry
Copy link
Contributor

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

Copy link

codecov bot commented Dec 14, 2023

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 21.68%. Comparing base (73bfd27) to head (68d3fb9).
Report is 105 commits behind head on 3.x.

Files Patch % Lines
core/src/Revolution/Processors/Resource/Create.php 0.00% 1 Missing ⚠️
core/src/Revolution/Processors/Resource/Update.php 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##                3.x   #16505   +/-   ##
=========================================
  Coverage     21.68%   21.68%           
- Complexity    10496    10498    +2     
=========================================
  Files           561      561           
  Lines         31703    31703           
=========================================
  Hits           6875     6875           
  Misses        24828    24828           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Jako
Copy link
Collaborator

Jako commented Jan 7, 2024

Maybe it is better to prepare an upgrade script that changes the value.

@Mark-H Mark-H added this to the v3.0.5 milestone Feb 10, 2024
Copy link
Collaborator

@Mark-H Mark-H left a comment

Choose a reason for hiding this comment

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

I'm fine with either this approach and/or the the migration script to update values.

@Jako
Copy link
Collaborator

Jako commented Feb 14, 2024

I am also fine with that solution.

@Mark-H Mark-H added the pr/ready-for-merging Pull request reviewed and tested and ready for merging. label Feb 26, 2024
@opengeek opengeek changed the title [MODX3] Fix saving the time for date TVs that were created in MODX 2.x Fix saving time for date TVs created in MODX 2.x Mar 20, 2024
@opengeek opengeek merged commit 218e56c into modxcms:3.x Mar 20, 2024
7 checks passed
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
pr/ready-for-merging Pull request reviewed and tested and ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants