deckerst / aves

Aves is a gallery and metadata explorer app, built for Android with Flutter.
BSD 3-Clause "New" or "Revised" License
2.77k stars 106 forks source link

Existing albums can be recreated #1272

Closed An-anonymous-coder closed 1 week ago

An-anonymous-coder commented 3 weeks ago

Describe the bug When creating a new album, it is possible to create an album with the same name as an existing album. This doesn't do anything but make it seem like that existing album was just created. There is important additional context about this issue.

To Reproduce Steps to reproduce the behavior:

  1. Open Aves
  2. Go to the "Albums" tab
  3. Tap the three dots on the top right
  4. Tap "Create album"
  5. Enter the name of an already existing album (e.g. "Sunsets") It will say "Directory already exists", but it will still allow you to press "CREATE" to create the album.
  6. Tap "CREATE" A banner will appear at the bottom of the screen that says "Done!"
  7. Tap "SHOW" on the banner that says "Done!" The already existing album will now have the "NEW" tag displayed on the corner

Expected behavior The app should not allow you to create an already existing album

System information and logs:

Package: deckers.thibault.aves
Installer: dev.imranr.obtainium
Aves version: 1.11.17-izzy, build 13602
Flutter: stable 3.24.4
Android version: 15, API 35
Android build: 2024102400
Device: Google Pixel 8
Support: dynamic colors=true, geocoder=false, HDR=true
Mobile services: not available
Connectivity: none
System locales: en_US
Storage volumes: /storage/emulated/0/
Storage grants: /storage/emulated/0/Movies/, /storage/emulated/0/Pictures/, /storage/emulated/0/DCIM/
Error reporting: false

I can provide logs if requested

Additional context

This bug is due to the fix of https://github.com/deckerst/aves/issues/1248 which would create "phantom" albums and wouldn't let you create an album with the same name as any of those. The phantom albums are still present in this update, because when I type the name of one of the previously created phantom albums, it will display the "Directory already exists" message. New phantom albums cannot be created in this update.

deckerst commented 3 weeks ago

when I type the name of one of the previously created phantom albums, it will display the "Directory already exists" message.

The message is still there for information, but you should be able to create the album anyway (the button is enabled).

An-anonymous-coder commented 3 weeks ago

The phantom albums remaining from the previous update is still a bug.

deckerst commented 3 weeks ago

"Phantom album" is not a concept. Please be more precise for me to understand.

When you do whatever you do:

An-anonymous-coder commented 3 weeks ago

Steps to reproduce:

  1. Install Aves v1.11.16
  2. Open Aves
  3. Go to "Albums"
  4. Tap the three dots
  5. Select "Create album"
  6. Name the album anything. I named mine "Test" for clarity.
  7. Tap "CREATE"
  8. Force close Aves Bug https://github.com/deckerst/aves/issues/1248 is reproduced, meaning there is an album that the app detects as created, but does not show up in the app.
  9. Update Aves to v1.11.17
  10. Open Aves
  11. Go to the "Albums" tab
  12. Tap the three dots on the top right
  13. Tap "Create album"
  14. Enter the name you had before. Mine was "Test" It will say "Directory already exists", despite the album not showing up.

It is still possible to create the album.

do you expect to see a folder on storage?

No

is there a folder on storage?

Yes

do you expect to see an album in Aves?

No

is there an album in Aves?

No

I understand why this happened, it says "Directory already exists" because folders were created. The fix is to simply delete the empty folders. I expected Aves to do this automatically after an update, because I assumed Aves kept its own internal index of which directories are supposed to be albums. Do you think it would cause issues if Aves automatically deleted empty directories?

An-anonymous-coder commented 3 weeks ago

I think there is a misunderstanding that occurred from https://github.com/deckerst/aves/issues/1248 and its fix. Here is how I would expect the behavior of Aves to work:

When creating an album, if the album already exists, the "CREATE" button should be grayed out with the message "Album already exists".

If the directory (NOT the album) already exists, the "Directory already exists" message should appear, but you should be able to convert that directory into an album (i.e. create an album with that name)

I expected that, upon updating, the missing albums created from https://github.com/deckerst/aves/issues/1248 would be erased. I thought the bug was being reproduced because Aves kept an index of created album names, and albums created by reproducing https://github.com/deckerst/aves/issues/1248 were not being deleted from that index. I now understand that what is really going on is that empty directories were being made from reproducing https://github.com/deckerst/aves/issues/1248, which was causing the bug to occur.

I'm sorry for the confusion.

deckerst commented 1 week ago

I appreciate your suggestions to clarify "Album already exists" v. "Directory already exists", and will likely implement something like this (i may switch the "CREATE" button to "SHOW" when the album exist, not sure yet).

As for the internals, it doesn't exactly work as you think. Albums are an emergent property of your collection. There is not a list of "Aves albums". Rather, albums are derived from the directories of media files in your collection. That's why Aves generally does not show empty albums. The exception to this is allowing users to create a new empty album, to which they can move media files. The new empty album is only visible in that session, as it will disappear if no media is moved to it. On album creation, the directory is not created yet. The directory is only created when necessary, when files are moved to it.

Deleting empty directories is a delicate matter, because Aves is not a general purpose file manager. It's a gallery app, so it has a limited view of what's on storage.

In any case, thanks for following up and clarifying your initial issue. It's appreciated.