Warning on term deletion

My patch fixes this issue, and the missing deletion of IMG.

I also cleanup the deletion function.

@thePanz: Some module maintainers get very upset when someone marks their own patch as RTBC. It's not a big deal for me.

I will get to this shortly.

Sorry NancyDru, I saw some maintainers following that way.. so I followed them :)

What I mean setting my patch to RTBC is that I used it on a production site without bugs or errors... sorry for my misunderstanding..

Regards

ps: do you think that migrating from custom path to Drupal {files} could improve TI? I mean: we store the imagename in DB, why can't we rely on {files} and save only (TID, FID) in our table? :)

I have at least twice started on converting to tid/fid and have gotten thrown off each time. It is not a trivial change (think about the update code), but I think it is the right thing to do. In the past it did make the external image impossible, but I think now that I just copy it to the site, it may work now.

If you are interested in issue status meanings, take a look at http://drupal.org/node/156119. You can set your own issue to RTBC if it is trivial, but this one is not.

I get Error deleting term image; it seems that this image doesn't exists. when there is an image.

$full_path = $taxonomy_image_path . $old_path; but there is no "$old_path".

Sorry, I've got some other working project right now.. I'll give you feedback (or an updated patch) in next days..

[EDIT] I've put some edits on _delete() function. My idea is to delete Drupal messages from it and let the code that calls _delete() to act on true/false results. So external modules can handle TaxonomyImage better. What do you think?

Cheers,
thePanz

I nearly rewrote the function to be more straightforward. I also added a setting to show/not-show all the messages.

Hi NancyDru, I ported my patch with your latest commit in -dev snapshot.
I essentially removed the _verbose_ setting, and cleaned the _delete() function: it returns TRUE or FALSE as a result so caller function can (if needed) provide an error message.

My intention is to provide _delete($tid) and _add($tid, $filename) like an API (for the letter I noticed a patch that goes in that direction).

What do you think? Sould we port all this "features" into a 2.x version cause it's a major module change (I also think about moving from custom table with filenames to drupal {files} FID)?

I don't have a problem with returning TRUE or FALSE.

I like the verbose setting, and here's why: The most important part of the function is to break the term-image association and clear the cache. If, for some reason, the files table or the disk cannot be deleted, it is not the end of the world; there would be junk left, but the image no longer displays. I can live with that, but it would be nice if there was a way to find out where the function failed. But I suspect most people probably don't care as long as the image goes away.

Also, I cleaned up some of the comments and your new patch puts them back.

Yes, I would like to get this to only saving the fid, which is normalizing the database. I just need to get back to it. The hardest part of doing this will be the hook_update_N code. That would justify a version change.

More justification for the fid use idea: #437814: Images won't load and site "crashes" after upgrade from 1.5 to 1.6