-
-
Notifications
You must be signed in to change notification settings - Fork 72
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
Handle timezone in publication fields (effective and expires) #1192
base: main
Are you sure you want to change the base?
Conversation
@cekk thanks for creating this Pull Request and help improve Plone! To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass. Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:
With this simple comment all the jobs will be started automatically. Happy hacking! |
@jenkins-plone-org please run jobs |
✅ Deploy Preview for plone-restapi canceled.
|
Holy mother of... this is here for 2 years and nobody took care of it? 😄 I can confirm @cekk 's conclusions from here plone/volto#2597 (comment) How to test and reproduce:
|
is_publication_field = self.field.interface == IPublication | ||
if is_publication_field: | ||
# because IPublication datamanager strips timezones | ||
tzinfo = pytz.timezone(default_timezone()) |
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.
@cekk While this is way better than assuming UTC always, it can still have issues if the server has different timezone than the one from registry, UTC for example (the default behavior of plone/plone-backend
Docker image).
And I think @sneridagh can confirm that their installations doesn't provide the TZ environment variable to docker containers, thus UTC by default. While the default timezone registry entry when creating a new Plone instance is Europe/Berlin
.
How to test:
docker run -it --rm -p 8080:8080 -e SITE=Plone plone/plone-backend bash
git clone https://github.com/plone/plone.restapi.git
cd plone.restapi
git checkout handle_tz_in_publication_fields
cd /app
bin/pip install -e ./plone.restapi --no-deps
./docker-entrypoint.sh start
Now add a News Item with Effective Date via Volto and check portal_catalog entry.
We have been hit by this too, and I can confirm that with this changes the original datetime as entered in Volto interface is correctly saved in Plone. We configure our Zope instance passing the In all cases here, I have Item saved with the following datetime through Volto: 2023-08-28 10:00:00 Previous scenario (without this branch):
Current scenario (with this branch)
I have made a test without adding the Previous scenario (without this branch)
Current scenario (with this branch):
|
a6ece0a
to
25df696
Compare
This could fix plone/volto#2597
The failing test is because also if i customize env var, i can't save a DateTime value in that field with the wanted timezone.
Anyone have an idea on how to fix it? @buchi ?