Forum OpenACS Development: Re: Calling ad_script_abort from within ad_returnredirect

Collapse
Posted by russ m on
I'd have thought the reason for doing this is to make it harder for the programmer to mess up. Adding an optional -abort switch and requiring the programmer to remember to use it is no different to requiring the programmer to remember to use an ad_script_abort. (The -abort switch would have to be optional otherwise you're back to the problem of fixing every call to ad_returnredirect in the current sources).

Personally I like the idea of -no_abort since it makes the most common behaviour the default, but I'm not putting my hand up to crawl through every use of ad_returnredirect to see if it's required or not... :)

Not only does it occur a number of times in the toolkit, it is propably relied upon in a lot of custom code as well. Imagine how hard a change like this would make the life of those who are upgrading their existing sites with a new version of oacs. It would produce a failure of their custom code that would hardly be noticed immediately, and make poor sysadmins search for the cause in e.g. their mailing setups in case of an ns_sendmail call after ad_returnredirect. You wouldn't even be able to catch it with an automated testing script unless you can somehow check for successfully delivered mails with it.

I think since that it seems that OpenACS is actually being used we should take more care for those people who want to upgrade their code base from time to time, or at least not make their life unnecessarily hard.

A more acceptable solution would be to introduce a new command like ad_redirectandabort (think of a better name) and replace the occurences of ad_returnredirect in the toolkit with it, and maybe eventually deprecating ad_returnredirect, IMHO.