0
Fork 0
mirror of https://github.com/immich-app/immich.git synced 2025-03-25 02:41:37 -05:00

fix(mobile): 14983 Images upload to shared album with common name (#15127)

* Initial look at fixing issue where images are uploaded to the wrong album if a shared album conflicts with a local users album.

* Use owner instead of shared flag when fetching albums.

* Fix issue with refreshRemoteAlbums getting shared items twice and removed incorrect isShared comment.

Using `getAll(shared: true)` gets all shared albums the user can access (regardless of owner, despite the previous comment).

Using `getAll(shared: null)` gets all albums (incuding shared = true and shared = false). I presume the intent here was to get albums that were shared (and not mine), and not shared (ie: mine), but the logic is way off. It also just then combines them - so makes more sense to just get them in a single call.

* Fix formatting.

* Fixed tests.

* Revert "Fixed tests."

This reverts commit c38f5af5ac.

* Revert "Fix issue with refreshRemoteAlbums getting shared items twice and removed incorrect isShared comment."

This reverts commit 979ce90abf.

* Added comments to explain why filters behave the way they do for getAll() albums.

---------

Co-authored-by: Tom graham <tomg@questps.com.au>
Co-authored-by: Alex <alex.tran1502@gmail.com>
This commit is contained in:
Tom Graham 2025-01-17 14:24:09 +11:00 committed by GitHub
parent fd99bd05cf
commit efbc0cb192
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 48 additions and 8 deletions

View file

@ -13,6 +13,7 @@ abstract interface class IAlbumRepository implements IDatabaseRepository {
String name, { String name, {
bool? shared, bool? shared,
bool? remote, bool? remote,
bool? owner,
}); });
Future<List<Album>> getAll({ Future<List<Album>> getAll({

View file

@ -46,8 +46,18 @@ class AlbumNotifier extends StateNotifier<List<Album>> {
) => ) =>
_albumService.createAlbum(albumTitle, assets, []); _albumService.createAlbum(albumTitle, assets, []);
Future<Album?> getAlbumByName(String albumName, {bool remoteOnly = false}) => Future<Album?> getAlbumByName(
_albumService.getAlbumByName(albumName, remoteOnly); String albumName, {
bool? remote,
bool? shared,
bool? owner,
}) =>
_albumService.getAlbumByName(
albumName,
remote: remote,
shared: shared,
owner: owner,
);
/// Create an album on the server with the same name as the selected album for backup /// Create an album on the server with the same name as the selected album for backup
/// First this will check if the album already exists on the server with name /// First this will check if the album already exists on the server with name
@ -55,7 +65,7 @@ class AlbumNotifier extends StateNotifier<List<Album>> {
Future<void> createSyncAlbum( Future<void> createSyncAlbum(
String albumName, String albumName,
) async { ) async {
final album = await getAlbumByName(albumName, remoteOnly: true); final album = await getAlbumByName(albumName, remote: true, owner: true);
if (album != null) { if (album != null) {
return; return;
} }

View file

@ -34,11 +34,25 @@ class AlbumRepository extends DatabaseRepository implements IAlbumRepository {
Future<Album> create(Album album) => txn(() => db.albums.store(album)); Future<Album> create(Album album) => txn(() => db.albums.store(album));
@override @override
Future<Album?> getByName(String name, {bool? shared, bool? remote}) { Future<Album?> getByName(
String name, {
bool? shared,
bool? remote,
bool? owner,
}) {
var query = db.albums.filter().nameEqualTo(name); var query = db.albums.filter().nameEqualTo(name);
if (shared != null) { if (shared != null) {
query = query.sharedEqualTo(shared); query = query.sharedEqualTo(shared);
} }
if (owner == true) {
query = query.owner(
(q) => q.isarIdEqualTo(Store.get(StoreKey.currentUser).isarId),
);
} else if (owner == false) {
query = query.owner(
(q) => q.not().isarIdEqualTo(Store.get(StoreKey.currentUser).isarId),
);
}
if (remote == true) { if (remote == true) {
query = query.localIdIsNull(); query = query.localIdIsNull();
} else if (remote == false) { } else if (remote == false) {

View file

@ -170,7 +170,12 @@ class AlbumService {
try { try {
await _userService.refreshUsers(); await _userService.refreshUsers();
final (sharedAlbum, ownedAlbum) = await ( final (sharedAlbum, ownedAlbum) = await (
// Note: `shared: true` is required to get albums that don't belong to
// us due to unusual behaviour on the API but this will also return our
// own shared albums
_albumApiRepository.getAll(shared: true), _albumApiRepository.getAll(shared: true),
// Passing null (or nothing) for `shared` returns only albums that
// explicitly belong to us
_albumApiRepository.getAll(shared: null) _albumApiRepository.getAll(shared: null)
).wait; ).wait;
@ -212,7 +217,7 @@ class AlbumService {
for (int round = 0;; round++) { for (int round = 0;; round++) {
final proposedName = "$baseName${round == 0 ? "" : " ($round)"}"; final proposedName = "$baseName${round == 0 ? "" : " ($round)"}";
if (null == await _albumRepository.getByName(proposedName)) { if (null == await _albumRepository.getByName(proposedName, owner: true)) {
return proposedName; return proposedName;
} }
} }
@ -408,8 +413,18 @@ class AlbumService {
} }
} }
Future<Album?> getAlbumByName(String name, bool remoteOnly) => Future<Album?> getAlbumByName(
_albumRepository.getByName(name, remote: remoteOnly ? true : null); String name, {
bool? remote,
bool? shared,
bool? owner,
}) =>
_albumRepository.getByName(
name,
remote: remote,
shared: shared,
owner: owner,
);
/// ///
/// Add the uploaded asset to the selected albums /// Add the uploaded asset to the selected albums
@ -419,7 +434,7 @@ class AlbumService {
List<String> assetIds, List<String> assetIds,
) async { ) async {
for (final albumName in albumNames) { for (final albumName in albumNames) {
Album? album = await getAlbumByName(albumName, true); Album? album = await getAlbumByName(albumName, remote: true, owner: true);
album ??= await createAlbum(albumName, []); album ??= await createAlbum(albumName, []);
if (album != null && album.remoteId != null) { if (album != null && album.remoteId != null) {
await _albumApiRepository.addAssets(album.remoteId!, assetIds); await _albumApiRepository.addAssets(album.remoteId!, assetIds);