Package Uninstall and Data Cleanup

Permalink
Hi,

I have a question about how to handle certain data when uninstalling a package. I am in the process of writing a new package for a shipping type for the eCommerce add-on. Once installed, it is possible for the user to use this new shipping type in some of the core eCommerce components (for example the Free Shipping Discount). If I have set up a discount, but then choose to uninstall my shipping type package, the discount still references a shipping type that now no longer exists.

I know I can manually delete any discounts as part of the package uninstall, but I am wondering whether this is the best option. If it is, should my uninstall provide an alert to say that discounts have been deleted? Or, should my uninstall disable the discount and alert the user that this has been done and they should update the discount to choose a new shipping type? Or, do I do nothing (not sure this is the best option)?

I guess what I am asking is what is the best way to do this and still have a good user experience?

Would be interested in your thoughts.

James

jheanly
 
jordanlev replied on at Permalink Reply
jordanlev
This is a good question, and I don't know if there's a great way to address it currently, because C5 doesn't give you a way to output your own uninstall message or anything like that. I usually err on the side of leaving the data intact just in case someone accidentally uninstalled an addon, the data is still there in MySQL for emergency exporting. But in your case, leaving the data intact is causing problems because now there are orphan records.

I think ideally you would prevent the user from uninstalling the shipping type if any orders exist using it, but again there is no way currently to do this kind of prevention. So... I personally would just leave the data alone. If someone uninstalls the shipping type after orders exist using it, they're kind of screwed (from a data integrity point of view) one way or another, so why bother with extra effort?

Hopefully someone on the core team can chime in on this as I'd be very interested to hear their answer.
jheanly replied on at Permalink Reply
jheanly
Thanks for the response.

Is there anyway to pop up an alert during the execution of my package controllers uninstall() method? At least then I could highlight to the user that I have deleted discounts (for example) as they were using the shipping type that has now been uninstalled

James
jordanlev replied on at Permalink Reply
jordanlev
None that I know of. Maybe someone else knows different?
Mnkras replied on at Permalink Reply
Mnkras
Yea, There really needs to be a better way to do this,

you _can_ stop the uninstall if records exist that use it though, just throw an exception after you do a check, I think I will add some package events,

on_before_package_install
on_package_install
on_before_package_update
on_package_update
on_before_package_uninstall
on_package_uninstall

Im sticking in the before's because I forsee the code being like this (pseudo code):

Events::fire('on_before_package_install', $pkg);
$pkg = $pkg->install();
Events::fire('on_package_install', $pkg);


thoughts?
jheanly replied on at Permalink Reply 1 Attachment
jheanly
I have just had another thought. For my purposes (ie informing the user that I am going to delete some discounts) can I override the uninstall process at the point where it currently tells you what data is going to be removed as part of the uninstall (see attached screen shot)?

BTW, I like your idea of being able to intercept the process at various points.

James
Mnkras replied on at Permalink Reply
Mnkras
Look at the getPackageItems() function in the package model,

(btw if you don't call parent::uninstall() then it won't preform any c5 uninstall methods)

That method populates $items, below is the code that you see in the dashboard.

foreach($items as $k => $itemArray) { 
   if (count($itemArray) == 0) {
      continue;
   }
   ?>
   <h3><?=$text->unhandle($k)?></h3>
   <? foreach($itemArray as $item) { ?>
      <?=$pkg->getItemName($item)?><br/>         
   <? } ?>
   <br/>
<? } ?>
jheanly replied on at Permalink Reply
jheanly
This was a nice idea, and it gets me most of the way there, but not quite. The getItemName() method in Package has a defined list of types of items that it can process. If I wanted to include a custom item in the list of things that are going to be uninstalled, there is no way to show what the item is actually called (ie relates to).

It might be nice if there was a way to plug into this uninstall page, even if it was just to show some plain text warning. In my case it would be good if I could show a message along the lines of...

"Warning: you are about to uninstall a shipping type that is still in use by enabled Free Shipping Discounts. Please modify these discounts and change the Shipping Method"
jordanlev replied on at Permalink Reply
jordanlev
I'd make it so any of these events can return FALSE to halt the uninstallation.
Also, in addition to just having events, there should be some way to provide messages or confirmations to users -- but this is probably a lot more work than just adding some event firings :)
Mnkras replied on at Permalink Reply
Mnkras
Returning messages is easy with events,

$ret = Events::fire('on_package_uninstall', $pkg);
if($ret) {
    throw new Exception $ret;
}

basic example
jordanlev replied on at Permalink Reply
jordanlev
I guess I meant interactive messages, or things that provide confirmations or allow user to continue or cancel (like the OP is requesting) -- not just a fatal error message.
Mnkras replied on at Permalink Reply
Mnkras
ah, yea, that would be nice, actually... you could possibly return an element... but that is horribly hacky...
JohntheFish replied on at Permalink Reply
JohntheFish
Giving any package complete control of whether it uninstalls or not runs the risk of a faulty (or malign) package becoming stuck in a system so that the user cannot uninstall it. So whatever happens there would need to be an option coming from C5 to forcibly uninstall, no matter what the actual package is saying.
jordanlev replied on at Permalink Reply
jordanlev
This is a good point. I think I was a little confused yesterday and was actually thinking about how all of this stuff should be in the *install* methods, not uninstall.