DBTNG: Trigger module

Certain aspects still need to be rewritten.
Elements like this

$result = db_query("SELECT aa.aid, a.type FROM {trigger_assignments} aa LEFT JOIN {actions} a ON aa.aid = a.aid WHERE aa.hook = '%s' AND aa.op = '%s' ORDER BY weight", $hook, $op);

should be rewritten to use the new replacement syntax.

In addition, db_query return an object that can be iterated over, so this

while ($action = $result->fetchObject()) {

can be rewritten as a foreach that iterates over the $result object.

Re-roll of the trigger module patch with modifications for db_query calls.

1 step closer every time, that's what I say. Sooner or later, I'll just do it right the first time.
I reformatted the queries to break across lines, but I still wasn't 100% sure how to format 1 aspect:

    $aid_exists = db_query("SELECT aid FROM {trigger_assignments} WHERE hook = :hook AND op = :op AND aid = :aid", array(
      ':hook' => $form_values['hook'],
      ':op' => $form_values['operation'],
      ':aid' => $aid,
    ))
    ->fetchField();

Would you like the array indented more and the fetchField in its current location, or is this format the preferred format?

The straight select queries/db_query() should just be on one line, no need to split the array for those. Otherwise looks good.

There are two possible ways to indent, I'd prefer B) because we actually do chaining, even if it's only a single method and because the statement is not finished.

A) This is used if the statement is finished after the closing array, for example in hook_menu()

    $aid_exists = db_query("SELECT aid FROM {trigger_assignments} WHERE hook = :hook AND op = :op AND aid = :aid", array(
      ':hook' => $form_values['hook'],
      ':op' => $form_values['operation'],
      ':aid' => $aid,
    ))
    ->fetchField();

B) This is used in chaining, when there are multiple chaining methods

    $aid_exists = db_query("SELECT aid FROM {trigger_assignments} WHERE hook = :hook AND op = :op AND aid = :aid", array(
        ':hook' => $form_values['hook'],
        ':op' => $form_values['operation'],
        ':aid' => $aid,
      ))
      ->fetchField();

Ok, we pretty much agreed on the following coding style:

$aid_exists = db_query("SELECT aid FROM {trigger_assignments} WHERE hook = :hook AND op = :op AND aid = :aid", array(
  ':hook' => $form_values['hook'],
  ':op' => $form_values['operation'],
  ':aid' => $aid,
))->fetchField();

So the following needs to be updated, the rest of the patch looks good.

+    ))
+    ->fetchField();

@Crell - http://drupal.org/node/456824#comment-1596042

Seems I copied the wrong example, I meant A. What I wanted to point out is that fetchField() should be on the same line. Updated the above comment.

Here reroll after #246096: Cron triggers are not executed

With modifications from #12

My preference is ))->fetchField().

Crell, I don't remember having settled on that. I don't feel that strong about it though, and it certainly shouldn't hold up this patch.

Reroll with ))->

-  while ($action = db_fetch_object($result)) {
+  foreach ($result as $action) {
     $actions[$action->aid] = $action->description;
   }
   return $actions;

You can replace that with:

 return $result->fetchAllKeyed();

   $aids = array();
-  $result = db_query("SELECT aa.aid, a.type FROM {trigger_assignments} aa LEFT JOIN {actions} a ON aa.aid = a.aid WHERE aa.hook = '%s' AND aa.op = '%s' ORDER BY weight", $hook, $op);
-  while ($action = db_fetch_object($result)) {
+  $result = db_query("SELECT aa.aid, a.type FROM {trigger_assignments} aa LEFT JOIN {actions} a ON aa.aid = a.aid WHERE aa.hook = :hook AND aa.op = :op ORDER BY weight", array(
+    ':hook' => $hook,
+    ':op' => $op,
+  ));
+  foreach ($result as $action) {
     $aids[$action->aid]['type'] = $action->type;
   }

And that one probably with: (untested)

return db_query("SELECT aa.aid, a.type FROM {trigger_assignments} aa LEFT JOIN {actions} a ON aa.aid = a.aid WHERE aa.hook = :hook AND aa.op = :op ORDER BY weight", array(
  ':hook' => $hook,
  ':op' => $op,
))->fetchAllAssoc('aid');

First is chandged

Second is not working - aids holds keyed subarray with only 1 key - 'type' but if fetchassoc then we get whole record in sub array

Looks good now.

We agreed that we could probably simplify the array because the sub-array contains only a single key -> value pair, but we can do that in another issue.

Committed to CVS HEAD.

So here follow-up #476972: Optimizations of action_info array