Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deleting file fields makes file fields in the panel seem empty, while they are not #6371

Open
trych opened this issue Mar 26, 2024 · 5 comments

Comments

@trych
Copy link

trych commented Mar 26, 2024

Description

I ran into an issue with file fields in a live deployed site that seemed very counter intuitive to me.

I had this snippet throw an error, although I safeguarded it with an if statement:

  if ($page->musicmp3()->isNotEmpty()) {
    $audioMusic[] = $page->musicmp3()->toFile()->url();
  }

This threw an error on the audioMusic line. I expected it to not even be able to reach that line if there was no file in the field. Upon further investigation I found out that the content file had a file reference there that was apparently linking to a file that no longer existed, as it must have been deleted earlier.

Now, this makes for some confusing behaviour, as my supposed safeguard isNotEmpty() failed to save me here. It's also confusing that the field in the panel shows me it's empty, when apparently it is not.

Expected behavior
There's two solutions to this issue:
A) remove corresponding file references in the content file when files are deleted
B) show a "File Not Found" symbol or something like this in the file field that holds a file that no longer exists

I think isNotEmpty() should be consistent with what the users see in the panel.

I haven't tested it, but the same probably goes for file fields and user fields, which is where I realize that Option A is probably really complicated as probably all the website's content files would need to be searched for corresponding content fields.

Having said all that, I would also appreciate a short hint, how I could improve my safeguard here and quickly test if the file reference in the file field actually points at an existing file without throwing an error.

@distantnative
Copy link
Member

I think the easy workaround would be

if ($file = $page->musicmp3()->toFile()) {
    $audioMusic[] = $fille->url();
}

@trych
Copy link
Author

trych commented Mar 26, 2024

Thanks for the super quick workaround, this solves my immediate problem. 🙏

However, I think the issue itself remains, the fields should not appear empty in the panel when they still hold content. 🙂

@gbdesign2023
Copy link

@trych Is it possible that no file template has been assigned to the file? Is the file still in the content folder? Please upload another file and check the .txt file to see if a template has been assigned.

@distantnative
Copy link
Member

@gbdesign2023 ehat he's saying is that the file itself was deleted but the files field still holds the reference to that non-existing file. He's right that the a panel won't show it, but tests like Empty/NotEmpty will say it has content.

@rasteiner
Copy link
Contributor

I guess the question is whether a "File not found" symbol or graphic (like a crossed out filename or something) is really better than simply hiding the link to the non existing file.
I mean would it make the UX really better?
If I think of an example where a page has a pages field with min: 1, multiple: false, then something deletes the linked file from the server and therefore the user needs to relink a new file: would it really be better to have it show a non existing file first?

That also means the files field would need to store the name of the file (not only the uuid), otherwise, at most it could only display a random identifier which isn't really useful to a user. But storing the filename makes everything more complicated: when someone renames a file, kirby now has to update every reference to it in the whole website.

My 2 cts: I'm fine with having the reference missing in the panel as this reflects the results of a ->toFiles(), which is what the docs tell us to use

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants