Allow hook_auc_update() to prevent DB update
rsm08 - April 25, 2009 - 18:27
| Project: | Ubercart Auction |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | minor |
| Assigned: | Unassigned |
| Status: | active |
Jump to:
Description
Hi Garrett
In uc_auction.module hook_nodeapi 'update' op,
I suggest that you change this line:
db_query('UPDATE {uc_auction} SET ' . implode(', ', $qparts) . ' WHERE nid = %d', array($record['nid']));to this:
if count($record) {
db_query('UPDATE {uc_auction} SET ' . implode(', ', $qparts) . ' WHERE nid = %d', array($record['nid']));
} This will allow us to prevent a db call by resetting the $record array in hook_auc_update, if we don't want the update to take place.
#1
Oops, forgot the ( and ) in the if statement!
#2
For consistency, I'd rather make it check to see if your hook function returned FALSE instead.
However, I remember having a good reason for not allowing these hooks to abort an auction update. I can't quite recall what it was, though.
#3
Regardless of the reason, I think that as much modification as possible should be allowed for programmers using hooks. Because if you're tampering with modules using their hooks, you should be responsible enough to account for the consequenses of your changes.
My use for it was simple. Since I'm making an ebay-like site, I want to disallow updating of the auction once a bid has been placed. Using your hook the way I described will allow me to do this very easily.
BTW you deserve credit for exposing as many hooks as you've done in UCA. A lot of Ubercart modules has almost no hooks and rely on those nasty CA's instead.
#4
I can't recall if this was the reason in particular, but this did occur to me - if your module cancels the update, where does that leave other modules which have also hooked into hook_auc_update() and are expecting the update to have gone through?
Hmm. Maybe there needs to be two hooks - one that fires before the update and can cancel it, and one that fires after a successful update has for sure been committed to the database… When I can look at UC Auction again, I'll consider the feasibility of that. It doesn't seem impossible.
I don't really understand Conditional Actions. Or Drupal's standard Triggers/Actions stuff either. It just seems to be a bunch of stuff which invariably does almost, but not quite, exactly what you need it to…
#5
Hey Garrett - did you see my other issue about your hooks? It dropped a little bit down the list. Right now your hooks aren't working at all due to the use of reference variables. Please see my other post about this!
The way I see it, it's my site, so I'll decide if I want to rob the other modules of this hook. I don't have to kill the array, but I'd surely like to have the option to do so. My point is just, that when dealing with hooks, people who use them should be expected to be able to manage the consequenses of their changes. The more flexibility - the less chance that actual hacks into the module are neccesary.
Exactly how I feel too. That stuff and Views is something I'm never gonna get into, and I try to walk in a big circle around it!
#6
I saw the other post, but I can't do anything about it now. I'm crunching on other projects and can't exactly set things aside to focus on UC Auction much now.
That's all well and good for you, but I've got to consider other users - users who install two modules that conflict and then show up in my issue queues asking why.
#7
Ok just afraid you hadn't seen it. I'll post a suggestion for a solution in it, so you can just look at it when you have time.
That's a good point. But on the other hand, every hook in Drupal can screw things up for other modules if the right (or wrong) code is put in it, so I think we should ultimately rely on module developers using hooks responsibly.
And if a contributed module has code that screws things up for other modules without good reason, I'm sure it'll come up in that module's issue queue very quickly and not here!