Problem/Motivation
I suppose there is a bug about time zone handling with the Publication plugin. The dates are saved as UTC (DateTimeItemInterface::STORAGE_TIMEZONE) in the DB. But processSchedulerQuery() use a plain new DrupalDateTime() for comparing start and end dates. This is using the timezone from the current user which can be different than UTC. This is especially reproducible if using a datetime field (with this patch: #3184191: scheduler_field_type_publication not working if scheduled on same date) and not a date only field. But should happen for date only field as well in being to early or too late on publishing depening on given timezone.
Steps to reproduce
- Use a timezone different from UTC
- Use scheduler_field publication plugin on an entity
- Set a schedule for the publication
- Watch the publication being to late or too early with the offset of timezone used
Proposed resolution
Use UTC time zone for comparing, since database is also using UTC time.
Remaining tasks
create a patch
User interface changes
none
API changes
none
Data model changes
none
| Comment | File | Size | Author |
|---|
Issue fork scheduler_field-3250384
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
stefan.kornComment #3
stefan.kornThis needs some changes in the process-callback too, see this patch
Comment #4
interx commentedThanks, exactly what I needed.
Some remarks:
You removed the loading of the dates as DrupalDateTime objects. I think it would be better to keep that when the value is not null, e.g.
DrupalDateTime::createFromFormat(DateTimeItemInterface::DATETIME_STORAGE_FORMAT, $start_date, DateTimeItemInterface::STORAGE_TIMEZONE);Wouldn't it be easier in the conditions checks to see if date_begin or date_end are null first, in that case nothing needs to happen?
Comment #5
stefan.korn@interX: Thanks for looking into this.
the scheduler_field typ is extending from Core DateRangeItem. DateRangeItem is providing start_date and end_date computed properties which return a DrupalDateTime. So there is no need for loading this again with DrupalDateTime::createFromFormat() from the field values.
One could check for date_begin or date_end being NULL, but as far I can tell an entity with empty date_begin (for publishing) or empty date_end (for unpublishing) should never reach the process callback, because they would not get passed through processSchedulerQuery callback.
That said, there might be an edge case if someone removes the date while the entity has not yet been processed. Or otherwise if something went wrong. So for this case it might make sense to check for empty dates here too.
Comment #6
interx commentedApologies, you are right, this is indeed correct, ignore my earlier comment.
I tested the patch and it seems to work fine!
Comment #7
interx commentedComment #8
giuseppe87 commentedFrom my test, the patch doesn't work when in case of a paragraph which needs to be unpublished, if the cron run within 1 hour after the end-date publication time of the paragraph.
I've updated the related issue to consider time-zone, so I think this one could be considered a duplicate of that?
Comment #9
giuseppe87 commentedComment #10
giuseppe87 commentedComment #11
goz commentedCommited on duplicate #3184191. Can you confirm the commit solve your issue @stefan.korn ?
Comment #12
stefan.korn@goz: somewhat late reply, but I can confirm that this issue is now fixed in HEAD.
Still I would propose some small code changes like described in #5. Basically using the computed properties for getting the start and end date instead of creating them (again) from value
Putting these and some other small changes in the MR for this issue.
Comment #14
stefan.kornComment #17
goz commentedComment #19
goz commented