fix(server): Live Photo migration bug when album is in template

[NikhilAlapati]

Description

Fix live photo migration bug where if album is in template the live video will not get moved to the album. With this it will be moved with or without an album. This is due to the metadata of the still photo is not being used for the motion part in the migration.

I have previously made this PR #24688 which aimed to fix live photo video parts not getting moved by the storage template properly, that PR fixes the issue only partially. This PR improves upon #24688 by using the same metadata for the Live Video as the Still Photo which will enable Live photo template migration to all storage templates

Fixes # #17668

How Has This Been Tested?

• Unit tests added

Basic Upload Test Web

1. Uploaded a few files with the following storage template: {{album}}/{{y}}/{{MM}}/{{filename}}
2. Added photos to different albums
3. Ran storage migration job after adding photos to album
4. Ensure that all assets are inside albums and no strays in the library or upload folder

Comprehensive Full Library Upload Manual E2E Sanity Check

1. Uploaded my entire personal library via mobile(IOS) with template: {{album}}/{{y}}/{{MM}}/{{filename}}
2. Synced albums in mobile after upload
3. Ran storage template migration job

Screenshots (if appropriate)

Screenshots for Basic Web Upload test

Image shows all assets are uploaded to their albums and organized correctly

Uploads folder empty as expected after migration

Screenshots for Comprehensive Full Library Test

Screenshot shows all assets properly arranged into their respepective albums, no strays without albums

Screenshot shows upload folder empty

Screenshot shows albums correctly visible in web

Checklist:

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation if applicable
• I have no unrelated changes in the PR.
• I have confirmed that any new dependencies are strictly necessary.
• I have written tests for new code (if applicable)
• I have followed naming conventions/patterns in the surrounding code
• All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
• All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

Please describe to which degree, if any, an LLM was used in creating this pull request.

LLM used for the following

1. Understand codebase(i.e. current database schema around live photos and assets) and implementations
2. Assisting in creating/updating tests
   ...

[SimJoSt]

If you need some beta testers, let me know.

[NikhilAlapati]

If you need some beta testers, let me know.

Thanks! feel free to pull this commit I can rebase so it contains the latest versions. I’ve been using this as a mirror seems to work well so far lmk if you face any issues

[NikhilAlapati]

If you need some beta testers, let me know.

Rebased it, should be upto-date with the latest patches, lmk if you face any issues, thanks!

[NikhilAlapati]

@SimJoSt I tried your template out of curiosity
{{y}}/{{#if album}}{{album-startDate-y}}-{{album-startDate-MM}}-{{album-startDate-dd}} = {{album-endDate-MM}}-{{album-endDate-dd}} - {{album}}/{{else}}{{MM}}/{{/if}}{{y}}-{{MM}}-{{dd}}_{{hh}}.{{mm}}.{{ss}}_{{filename}}

seems to work well! here is the tree of my personal library, doesn't seem to have any live video stragglers as expected

C:.
├───2017
│   └───2017-11-27 = 01-18 - Videos
├───2019
│   └───2017-11-27 = 02-13 - Recents
├───2020
│   ├───2017-11-27 = 01-18 - Videos
│   └───2020-11-24 = 08-19 - Screenshots
├───2021
│   ├───2017-11-27 = 01-18 - Videos
│   └───2017-11-27 = 02-13 - Recents
├───2022
│   ├───2017-11-27 = 01-18 - Videos
│   └───2017-11-27 = 02-13 - Recents
├───2023
│   ├───2017-11-27 = 01-18 - Videos
│   └───2017-11-27 = 02-13 - Recents
├───2024
│   ├───2017-11-27 = 01-18 - Videos
│   ├───2017-11-27 = 02-13 - Recents
│   └───2020-11-24 = 08-19 - Screenshots
├───2025
│   ├───2017-11-27 = 01-18 - Videos
│   ├───2017-11-27 = 02-13 - Recents
│   └───2020-11-24 = 08-19 - Screenshots
└───2026
    ├───2017-11-27 = 01-18 - Videos
    └───2017-11-27 = 02-13 - Recents

[SimJoSt]

@NikhilAlapati thank you for checking 🙏
The main issue I had was not on upload or when adding them to albums later, but when I changed the storage label of a user.
When I changed name1 to name2, the name1 directory persisted with the live photo video files.
When I changed it a second time, there were other files left behind as well. Not exactly the same ones or the same amount.

My main concern is to fix the current situation and consolidate all files under the same storage label again.
Would it be possible to test for that scenario as well?

[NikhilAlapati]

@SimJoSt yea that shouldn't be a problem now. that was happening due to the prod version doesn't fully transfer the metadata(still photo -> motion video). I changed my test storage label to admin_changed_label and the directory structure looks identical as before with the user folder name changed with your template. the old label is no longer there as expected

└───admin_changed_label
    ├───2017
    │   └───2017-11-27 = 01-18 - Videos
    ├───2019
    │   └───2017-11-27 = 02-13 - Recents
    ├───2020
    │   ├───2017-11-27 = 01-18 - Videos
    │   └───2020-11-24 = 08-19 - Screenshots
    ├───2021
    │   ├───2017-11-27 = 01-18 - Videos
    │   └───2017-11-27 = 02-13 - Recents
    ├───2022
    │   ├───2017-11-27 = 01-18 - Videos
    │   └───2017-11-27 = 02-13 - Recents
    ├───2023
    │   ├───2017-11-27 = 01-18 - Videos
    │   └───2017-11-27 = 02-13 - Recents
    ├───2024
    │   ├───2017-11-27 = 01-18 - Videos
    │   ├───2017-11-27 = 02-13 - Recents
    │   └───2020-11-24 = 08-19 - Screenshots
    ├───2025
    │   ├───2017-11-27 = 01-18 - Videos
    │   ├───2017-11-27 = 02-13 - Recents
    │   └───2020-11-24 = 08-19 - Screenshots
    └───2026
        ├───2017-11-27 = 01-18 - Videos
        └───2017-11-27 = 02-13 - Recents```

[SimJoSt]

Nice, then it should be enough to run the storage migration job again to cleanup the file structure.
Looking forward to the merge and release.

[danieldietzler]

We already have logic for this here https://github.com/immich-app/immich/blob/main/server/src/services/storage-template.service.ts#L159-L167 and here https://github.com/immich-app/immich/blob/main/server/src/services/storage-template.service.ts#L192-L199. Why does that not apply here?

[NikhilAlapati]

We already have logic for this here https://github.com/immich-app/immich/blob/main/server/src/services/storage-template.service.ts#L159-L167 and here https://github.com/immich-app/immich/blob/main/server/src/services/storage-template.service.ts#L192-L199. Why does that not apply here?

@danieldietzler Those implementations don't work fully for all templates due to Metadata not being attributed to the Live Photo Video from the Still Photo. when handleMigrationSingle runs the result depends on if the motion video gets processed first vs the still photo getting processed first. if the still photo gets processed first then it will migrate the motion video, but when the motion video gets processed by this function later it will be moved back the video to the wrong location since the metadata is not properly attributed

Same deal for handleMigration if a still photo gets processed first then the live will be moved correctly however, in future iteration when the live video gets processed it will not be templated properly

This issue in the original implementation is fixed by this https://github.com/immich-app/immich/pull/25329/changes#diff-160ca0c92818fe07b15afe84633d23c96c44bf66c97c54bf6d0d25cdaafebadfR279 which uses the still photo(if it's Live) to get the template path, which makes sure they both will be in the same location regardless of the template used

[danieldietzler]

Is there any reason to migrate assets with visibility = 'hidden'? I feel like we should never queue a migration for the individual motion part in the first place

[NikhilAlapati]

Is there any reason to migrate assets with visibility = 'hidden'? I feel like we should never queue a migration for the individual motion part in the first place

@danieldietzler That's a good point, if we don't queue visibility=hidden and let the current logic migrate the still+live that could simplify the logic. I'll give that a try when I have the chance and see if that works

[NikhilAlapati]

@danieldietzler Updated the implementation, much simpler now. Removed the SQL to find still photo from the live photo, removed the complex checks in handleMigration and handleMigrationSingle, and updated unit tests
Reran my testing and I'm getting the same results with successful migration

Manual Testing Details

template used: {{#if album}}{{album}}{{else}}Other{{/if}}/{{y}}/{{MM}}/{{filename}}
Result: no "Other" album with only live photos

and live photo and still photo in the same folder

complex template: {{y}}/{{#if album}}{{album-startDate-y}}-{{album-startDate-MM}}-{{album-startDate-dd}} = {{album-endDate-MM}}-{{album-endDate-dd}} - {{album}}/{{else}}{{MM}}/{{/if}}{{y}}-{{MM}}-{{dd}}_{{hh}}.{{mm}}.{{ss}}_{{filename}}

Result: migrated correctly with live and still photos in the same folder

└───admin_changed_label
    ├───2017
    │   └───2017-11-27 = 01-18 - Videos
    ├───2019
    │   └───2017-11-27 = 02-13 - Recents
    ├───2020
    │   ├───2017-11-27 = 01-18 - Videos
    │   └───2020-11-24 = 08-19 - Screenshots
    ├───2021
    │   ├───2017-11-27 = 01-18 - Videos
    │   └───2017-11-27 = 02-13 - Recents
    ├───2022
    │   ├───2017-11-27 = 01-18 - Videos
    │   └───2017-11-27 = 02-13 - Recents
    ├───2023
    │   ├───2017-11-27 = 01-18 - Videos
    │   └───2017-11-27 = 02-13 - Recents
    ├───2024
    │   ├───2017-11-27 = 01-18 - Videos
    │   ├───2017-11-27 = 02-13 - Recents
    │   └───2020-11-24 = 08-19 - Screenshots
    ├───2025
    │   ├───2017-11-27 = 01-18 - Videos
    │   ├───2017-11-27 = 02-13 - Recents
    │   └───2020-11-24 = 08-19 - Screenshots
    └───2026
        ├───2017-11-27 = 01-18 - Videos
        ├───2017-11-27 = 02-13 - Recents
        └───2026-02-18 = 02-18 - Test Album

[danieldietzler]

Added this query so I can get the live photo asset for migration since the other storage template queries skip over live photos(hidden assets)

I am so confused. You don't ever need to get the motion part of a live photo, that's the whole point of this exercise. You also don't seem to be using that query anyways, so I really don't understand that comment

[NikhilAlapati]

Added this query so I can get the live photo asset for migration since the other storage template queries skip over live photos(hidden assets)

I am so confused. You don't ever need to get the motion part of a live photo, that's the whole point of this exercise. You also don't seem to be using that query anyways, so I really don't understand that comment

Hey, I use it here inside handleMigration() and handleMigrationSingle() here the purpose of this query is to get the live asset(When the still is being processed) so it can be passed to moveAsset for migration. You are right we don't want process the live video directly, but we still move it when the still photo is being processed.

[danieldietzler]

Edit: my bad I was lost

[NikhilAlapati]

Edit: my bad I was lost

No worries, it's easy to forget the details when you review dozens of PRs per day

[danieldietzler]

Right, this is very similar to getForStorageTemplateJob however. I would prefer if you just extend getForStorageTemplateJob to take an { withHidden: boolean } as a second argument in that function. Then, in the query, you'd $if(withHidden, (qb) => qb.where(...))

[NikhilAlapati]

Right, this is very similar to getForStorageTemplateJob however. I would prefer if you just extend getForStorageTemplateJob to take an { withHidden: boolean } as a second argument in that function. Then, in the query, you'd $if(withHidden, (qb) => qb.where(...))

Refactored

[JageshP]

So I am really new to github - this issue has been driving me crazy and it seems there is a fix?

So the next steps it to contact the devs to get it implemented? How long is that process normally with open source projects? I apologize for my ignorance.

Thank you!

[danieldietzler]

So I am really new to github - this issue has been driving me crazy and it seems there is a fix?

So the next steps it to contact the devs to get it implemented? How long is that process normally with open source projects? I apologize for my ignorance.

Thank you!

Hey @JagguRaja, so what you commented on is a "Pull Request" (PR). That's essentially proposing a code change to be merged into the project. As such, Nikhil opened a PR to fix this issue. As you can see in the history if you scroll up, we already went through a couple of rounds of reviews. Once we're happy with the state of the PR, we'll merge it and it will be available in the next release :)

[JageshP]

So I am really new to github - this issue has been driving me crazy and it seems there is a fix?

So the next steps it to contact the devs to get it implemented? How long is that process normally with open source projects? I apologize for my ignorance.

Thank you!

Hey @JagguRaja, so what you commented on is a "Pull Request" (PR). That's essentially proposing a code change to be merged into the project. As such, Nikhil opened a PR to fix this issue. As you can see in the history if you scroll up, we already went through a couple of rounds of reviews. Once we're happy with the state of the PR, we'll merge it and it will be available in the next release :)

Thank you for being so patient and answering that with so much detail.

Pull requests make a lot of sense now.

Thank you again.

[JageshP]

Can we incorporate an extra fancy braacket into the code so that users don't need to add an extra "{}" around the album name?

#4917

For example, if I use "{{album}}/{{y}}-{{MMM}}-{{d}}-{{filename}}" and the album has a special character such as "&" (for example "Hike & Waterfall"), Immich names the folder "Hike & Waterfall"

To get around this, we need to add another set of brackets so it becomes "{{{album}}}/{{y}}-{{MMM}}-{{d}}-{{filename}}"

This names the folder "Hike & Waterfall" as the user intended.

Just bringing this up here because I have faced this issue and it seems you are fixing an issue in a similar area so maybe this would be easy to implement while you make the change for live photos.

Thanks for the work you are doing on this issue!

[danieldietzler]

Just bringing this up here because I have faced this issue and it seems you are fixing an issue in a similar area so maybe this would be easy to implement while you make the change for live photos.

PRs should always be as focused as possible, thus the feature you're asking for should definitely not be implemented here. Feel free to open a feature request for it though if there isn't already one covering that

[danieldietzler]

Finally got around to testing this; seems to work well. Thanks!