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): Does not assign lat/lon if they are at 0,0 #2991 #3669

Merged
merged 4 commits into from
Aug 15, 2023

Conversation

sellnat77
Copy link
Contributor

@sellnat77 sellnat77 commented Aug 13, 2023

Description

  • Allows null input to parseLatitude and parseLongitude
    • Returns null
  • If the latitude AND longitude are both 0, their exif coordinates are not set

Fixes #2991
Closes #3665

How Has This Been Tested?

  • Edit picture metadata to have lat,lon (0,0) -> open map with default settings -> verify null island is empty

Screenshots (if appropriate):

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if applicable

@vercel
Copy link

vercel bot commented Aug 13, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
immich ⬜️ Ignored (Inspect) Visit Preview Aug 15, 2023 1:14am

@sellnat77 sellnat77 marked this pull request as ready for review August 13, 2023 22:57
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.

Can we add a migration to fix existing data? Is that something you would be able to do? We can SQL update the data and set 0s to null on the next release.

@sellnat77
Copy link
Contributor Author

I can try! Any hints on where to look for a previous example?

@jrasm91
Copy link
Contributor

jrasm91 commented Aug 14, 2023

The src/infra/migrations folder

@jrasm91
Copy link
Contributor

jrasm91 commented Aug 14, 2023

Here's the docs for them as well:

https://immich.app/docs/developer/database-migrations

@sellnat77
Copy link
Contributor Author

Added migration file, not sure i did it right? specifically the down portion

When i transferred the migration to the dir the 0,0 entries were properly nulled out
if the down migration were to be ran, all the null entries would be placed on null island

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.

Looks great. We don't need a down for this migration 😁

(You can leave it empty)

@alextran1502 alextran1502 merged commit f1b8a7a into immich-app:main Aug 15, 2023
@sellnat77 sellnat77 deleted the 2991-nullIslandServer branch August 15, 2023 02:23
@JackBailey
Copy link

For anyone with this issue with imported files, here's how you perform the migration manually for docker installs:

docker exec -it immich-postgres psql --password --username immich-user immich-database

Enter your password, then run the following:

UPDATE "exif" SET latitude = NULL, longitude = NULL WHERE latitude = 0 AND longitude = 0;

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

Successfully merging this pull request may close these issues.

[BUG] Map shows some pictures on Null Island
5 participants