immich icon indicating copy to clipboard operation
immich copied to clipboard

feat: add option to include external assets in storage template settings

Open ikbenignace opened this issue 5 months ago • 1 comments

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: false to default storage template configuration in server/src/config.ts
  • Validation: Added @ValidateBoolean() includeExternalAssets!: boolean; to SystemConfigStorageTemplateDto in server/src/dtos/system-config.dto.ts
  • Service Logic: Updated moveAsset method in server/src/services/storage-template.service.ts to 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 SettingSwitch control to storage template settings in web/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)
  • Type Safety: Fixed TypeScript compilation issues by using proper StorageAsset type 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 StorageAsset type

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.

ikbenignace avatar Jun 10 '25 21:06 ikbenignace

Label error. Requires exactly 1 of: changelog:.*. Found: . A maintainer will add the required label.

github-actions[bot] avatar Jun 10 '25 21:06 github-actions[bot]

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.

zackpollard avatar Jul 17 '25 14:07 zackpollard