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 #224: "Auto-detected date is day before receipt date" #246

Merged
merged 3 commits into from Mar 14, 2022
Merged

Conversation

a17t
Copy link
Contributor

@a17t a17t commented Mar 10, 2022

Fixing #224 by

  1. Adding the timezone to the parsed date from the document
  2. Adding corresponding assertion in consumption test

@a17t a17t requested a review from a team March 10, 2022 13:44
@paperless-ngx-secretary paperless-ngx-secretary bot added backend non-trivial Requires approval by several team members labels Mar 10, 2022
@bauerj
Copy link
Member

bauerj commented Mar 10, 2022

I would rather get rid of the time zone entirely 😕

We had some issues with time zones in paperless_app before, they can be very hard to debug.

IMO it doesn't really make sense to be able to change the time zone. If I move to Australia and change the paperless time zone accordingly, I still would like the creation date to stay the same.

Wouldn't it be possible to just store the date in the database without a time value at all?

@a17t
Copy link
Contributor Author

a17t commented Mar 10, 2022

Just from fixing that issue I can say, most used time/date APIs include the timezone. So getting rid of it might require major changes and workarounds to ignore the timezone.

But in general I agree, the topic of timezones is quite complex and we need to decide on whether we want to support timezones or just say "the date of a document is timezone-less".

What do you think about creating an issue to discuss that proposed change, so the team can discuss and decide on that?

@bauerj
Copy link
Member

bauerj commented Mar 10, 2022

What do you think about creating an issue to discuss that proposed change, so the team can discuss and decide on that?

Yes, I think that's a good idea.

@MMauro94
Copy link

@bauerj I would like to say my 2 cents about this timezone issue. Where should I write? Here, on #224 or elsewhere?

@bauerj bauerj changed the title Issue 224 Fix #224: "Auto-detected date is day before receipt date" Mar 14, 2022
@bauerj
Copy link
Member

bauerj commented Mar 14, 2022

Probably here: #350

Copy link
Member

@bauerj bauerj left a comment

Choose a reason for hiding this comment

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

Apart from the high-level discussion about time-zones, this looks good and fixes the issue.

@bauerj bauerj added small-change and removed non-trivial Requires approval by several team members labels Mar 14, 2022
@bauerj bauerj merged commit 1abd7cc into dev Mar 14, 2022
@bauerj bauerj deleted the issue-224 branch March 14, 2022 18:03
@qcasey qcasey added the bug Bug report or a Bug-fix label Apr 7, 2022
@github-actions
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backend bug Bug report or a Bug-fix small-change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants