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(server): do not return year with weird timezone info i.e before 1890 #15713

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alextran1502
Copy link
Contributor

@alextran1502 alextran1502 commented Jan 27, 2025

Some older years come with a timezone JS time doesn't recognize, rendering an invalid date. This PR adds the year cutoff for queries that could have trouble with the said years

Copy link
Contributor

@mertalev mertalev left a comment

Choose a reason for hiding this comment

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

You can move that to the subquery for generate_series, wrapping it in greatest.

@alextran1502 alextran1502 changed the title fix(server): do not return year with weird timezone info i.e before 1850 fix(server): do not return year with weird timezone info i.e before 1890 Jan 27, 2025
@alextran1502
Copy link
Contributor Author

You can move that to the subquery for generate_series, wrapping it in greatest.

I am not sure I completely get what you mean. Understand the part of moving it into generated_series, isn't greatest will get the latest year?

@mertalev
Copy link
Contributor

I mean that you can do something like greatest(1850, subquery), where the subquery returns the earliest year.

Copy link
Member

@bo0tzz bo0tzz left a comment

Choose a reason for hiding this comment

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

I don't think we should just straight up not return things like this. If there are timezones that JS can't handle, I think it'd be better if we just drop the timezone part there altogether. As it stands, this change would mean that assets will just disappear.

@danieldietzler
Copy link
Member

Strongly agree with @bo0tzz.

Copy link
Contributor

@jrasm91 jrasm91 left a comment

Choose a reason for hiding this comment

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

Maybe it would be better to just unset the timezone for these instead. Or, add more validation around what timezones we accept when the date is this old

@NicholasFlamy
Copy link
Member

NicholasFlamy commented Jan 30, 2025

According to Wikipedia the oldest driving photograph is from 1826:
History of the Camera:
"View from the Window at Le Gras (1826), the earliest surviving photograph[10]"

Edit: 1890 is a good cutoff though because realistically no one had a camera until after that.

@bo0tzz
Copy link
Member

bo0tzz commented Jan 30, 2025

I think there's more to the consideration than just what might be the oldest camera photograph. As an example, people might want to store scans of older art or documents, not to mention the possibility of a file just having bad metadata with a year far in the past - assets disappearing unexpectedly will always cause issues somewhere.

@Snowknight26
Copy link
Contributor

Are there any examples of the dates/timezone combinations that supposedly have issues?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants