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

Avatar image deletion don't update avatar field in yaml file. #2805

Closed
mhlsf opened this issue Dec 9, 2019 · 6 comments
Closed

Avatar image deletion don't update avatar field in yaml file. #2805

mhlsf opened this issue Dec 9, 2019 · 6 comments

Comments

@mhlsf
Copy link

mhlsf commented Dec 9, 2019

Environment :

  • Linux Fedora
  • PHP 7.3.12
  • Grav 1.6.19
  • Admin Panel 1.9.12

Also tested on window Wamp

Step to reproduce the bug :

  1. Upload an image using the filepicker on the admin profile page (admin/user/youraccount)
  2. Save the profile with the save button to the top right
  3. Delete the image using the bin icon on the image
  4. Save again

What should happen :

There should be no image on the filepicker zone

What happen :

There is a blank image with the same information as the previously upload image

What the issue :

When we delete a profile image, Admin plugin call taskRemoveFileFromBlueprint() method in adminbasecontroller.php that correctly delete the image from the user/accounts/avatar folder but don't update the avatar field in the yaml file.

I would like to provide a fix but I don't know if the yaml file should be updated in taskRemoveFileFromBlueprint() or should be done trough taskSaveUser()

I suspect it should be done in the taskSaveUser() but the data filter seem to remove the avatar field when it is empty (see Grav/Common/Data/Validation.php)

@mahagr mahagr self-assigned this Dec 10, 2019
@mahagr
Copy link
Member

mahagr commented Dec 13, 2019

Avatar cannot be deleted because of deleting it will not trigger AJAX request to delete the file. Additionally data[avatar] contains information from the old image.

@mhlsf
Copy link
Author

mhlsf commented Dec 13, 2019

Actually on deletion from admin the AJAX call is send and the file is well deleted, this is just the blueprint that isn't updated.

My guess would be to toggle $keepEmptyValues from filter function of Grav\Common\Data.

Editing method taskSaveUser() in admincontroller, line 704 as follow :

$data->filter();

to

$data->filter(False, True);

It seem to fix the issue, I don't know if there is other repercussion.

@mahagr
Copy link
Member

mahagr commented Jan 28, 2020

Looks like I've already fixed this in 6a9724d#diff-a387dc90668050191ec659f1e145f5e0R34

@mahagr
Copy link
Member

mahagr commented Jan 28, 2020

@rhukster Do we need to backport this one or is it ok to have it fixed only in Grav 1.7?

@mahagr mahagr transferred this issue from getgrav/grav-plugin-admin Jan 28, 2020
@mahagr mahagr added the 1.6 label Jan 29, 2020
@mahagr
Copy link
Member

mahagr commented Jan 29, 2020

Cherry picked the fix to Grav 1.6 as well.

@mhlsf
Copy link
Author

mhlsf commented Jan 29, 2020

This fix does work, thank you !

@mhlsf mhlsf closed this as completed Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants