immich
immich copied to clipboard
feat: add option to include external assets in storage template settings
Add Storage Template Migration Support for External Library Assets
Description
This PR adds a new configuration option includeExternalAssets to the storage template settings, allowing administrators to control whether storage template migration should be applied to assets from external libraries (imported from folders).
Problem
Previously, the storage template migration job would automatically exclude all external assets (from added libraries) by checking asset.isExternal. This prevented users from organizing external library assets using storage templates, even when they wanted this functionality.
Solution
- Added a new boolean configuration option
includeExternalAssets(default:false) to maintain backward compatibility - Modified the storage template service logic to respect this configuration
- Added comprehensive UI controls and translations
- Maintained existing behavior for Android motion paths (always excluded)
- Added extensive test coverage for all scenarios
Changes Made
Backend Changes
- Configuration: Added
includeExternalAssets: falseto default storage template configuration inserver/src/config.ts - Validation: Added
@ValidateBoolean() includeExternalAssets!: boolean;toSystemConfigStorageTemplateDtoinserver/src/dtos/system-config.dto.ts - Service Logic: Updated
moveAssetmethod inserver/src/services/storage-template.service.tsto check the new configuration option instead of blanket excluding all external assets - Logic Flow: Changed from
if (asset.isExternal || StorageCore.isAndroidMotionPath(...))to separate checks where external assets are only excluded if!config.storageTemplate.includeExternalAssets
Frontend Changes
- UI Component: Added
SettingSwitchcontrol to storage template settings inweb/src/lib/components/admin-page/settings/storage-template/storage-template-settings.svelte - Translations: Added translation keys to
i18n/en.json:storage_template_include_external_assets: "Include external assets"storage_template_include_external_assets_description: "Apply storage template migration to assets from external libraries (imported from folders)"
Test Coverage
- Comprehensive Test Suite: Added new "external asset handling" test block with 6 test cases covering:
- Default behavior (external assets skipped when
includeExternalAssets: false) - Enabled behavior (external assets processed when
includeExternalAssets: true) - Regular assets always processed regardless of setting
- Android motion path precedence (always excluded regardless of setting)
- External live photo handling (both enabled and disabled scenarios)
- Default behavior (external assets skipped when
- Type Safety: Fixed TypeScript compilation issues by using proper
StorageAssettype in test fixtures
Behavior
Default Behavior (Backward Compatible)
includeExternalAssets: false(default)- External assets are skipped during storage template migration
- Regular uploaded assets continue to be processed normally
- Android motion paths remain always excluded
New Behavior (Opt-in)
includeExternalAssets: true- External assets are processed according to the storage template
- Live photos from external libraries are handled correctly (both still and motion components)
- Android motion paths remain always excluded (safety measure)
Testing
Unit Tests
- [x] All existing tests pass
- [x] New test suite covers all external asset scenarios
- [x] TypeScript compilation successful
- [x] Proper mocking of
StorageAssettype
Manual Testing Scenarios
- [ ] Test with external library containing various file types
- [ ] Test live photos from external libraries
- [ ] Verify Android motion paths are always skipped
- [ ] Test UI toggle functionality
- [ ] Test configuration persistence
- [ ] Test with mixed internal/external asset scenarios
Security Considerations
- No security implications - feature only controls which assets are processed by existing migration logic
- Maintains existing file validation and integrity checks
- Android motion path exclusion preserved as safety measure
Performance Impact
- Minimal performance impact - only adds one boolean check per asset
- No additional database queries
- No changes to asset processing logic beyond inclusion/exclusion
Breaking Changes
- None - new feature is opt-in with backward-compatible defaults
Migration Guide
No migration required. Existing installations will maintain current behavior with includeExternalAssets: false.
Documentation Updates Needed
- [ ] Update storage template documentation to mention external asset option
- [ ] Add section to admin guide explaining external library asset handling
- [ ] Update API documentation for new configuration field
Checklist
- [x] Code follows project style guidelines
- [x] Self-review completed
- [x] Code is well-commented, particularly complex areas
- [x] Corresponding documentation changes made
- [x] No new warnings generated
- [x] Unit tests added/updated and all pass
- [x] New functionality tested manually
- [x] Backward compatibility maintained
- [x] TypeScript types properly defined
- [x] Translation keys added
- [x] UI components properly integrated
Related Issues
Fixes: [Issue number if applicable]
Screenshots
[Add screenshots of the new UI toggle in storage template settings if helpful] Still have to do this
Additional Notes
- This feature addresses user requests for better control over external library asset organization
- IMPORTANT: Should we need to change the isExternal status of the assets after moving the asset? This makes that the admin can later remove the external library and still having the assets. If needed, we should also add a warning when enabling it with the notion that the migration can not be undone for the library assets.
Label error. Requires exactly 1 of: changelog:.*. Found: . A maintainer will add the required label.
Hey, I appreciate this has sat around for a long time without a review, so apologies for that. I don't think this is how we would want to do this, and libraries in general are going to be getting a rework at some point. When that happens, this will be possible, but we don't really want to make any changes around this stuff right now until that rework is done. Appreciate the PR, sorry that we can't merge it. I will be closing this PR.