CRM-21334: Fire hooks on Contact Image Deletion

[mickadoo]

Overview

When deleting a contact image from the profile overview page the contact image is removed and hooks are fired.

Before

No hooks are fired when the contact image is deleted.

After

postUpdate and postSave hooks are fired.

---

• CRM-21334: Fire hooks on contact image deletion

[colemanw]

This is an improvement for sure. It doesn't go all the way to calling pre and post hooks though. Honestly this function probably shouldn't even exist, as we can just use the api from the 1 place where it's called.

[totten]

Riffing on the point that the function deleteContactImage shouldn't exist...

• It doesn't quite do what it says -- it clears the reference to the image rather than deleting the image. (In fairness, the actual behavior is more practical than the putative behavior, so that's just a superficial smell.)
• If you grep universe, the only call to deleteContactImage is from within the same file (CRM/Contact/BAO/Contact.php).

[colemanw]

Well, contact.image_url is a funny field. It's not a proper file reference, it just stores a url string and the file can be anywhere.

[mickadoo]

I agree this isn't the cleanest fix, my first impulse was to do an api call from inside the function.. but then I didn't see any other api calls from inside the BAO and figured that API calls probably don't belong at this level. I tried using create, but gave up after a few errors in favor of this approach where at least someone listening for changes can hook into image deletion.

I'm very open to any suggested change, totally agree the hook firing should be consistent with any other field update. If you think it would be better I could change the page callback to point to a new class and call the API from there.

If we want to go the path of removing the image file just let me know and I can check it out too.

[eileenmcnaughton]

I'm going to merge this. I think it's a minor improvement. I think calling the api would be a better fix (yes we DO do that from the BAO & it's more tested) and that removing the function would be a better fix. But it seems to be not worth holding up further for one of those