Rework block.module for the new data api
zoo33 - August 31, 2008 - 13:34
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | block.module |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | duplicate |
| Issue tags: | DBTNG Conversion |
Description
Here is an uncomplete patch from the code sprint! Uncomplete as I have to get som air and then go to the airport, and I need some help on a few parts. I did test everything that I changed though, and I think everything is running as it should.
All parts that are either not ported yet or have other problems are marked with TODO:.
– In block_user() and _block_load_blocks() there are two dynamic queries with joins that I can't convert without either some more documentation or some more help.
– block_admin_display_form_submit() isn't working.
I'll try to finish the rest of the trivial stuff on the train. So long!
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| block-module-db-rewrite.patch | 9.46 KB | Idle | Failed: Failed to apply patch. | View details | Re-test |
#1
OK, block_admin_display_form_submit() wasn't working simply because I had left out execute().
More thougths:
Crell says we should the default object fetch mode as much as possible. There are at least two places in this patch where I override that and use arrays, since it seemed like a pretty big job to change every occurrence of the retrieved variable from arrays to objects ($box and $old_block). So I wanted to check first to see if people think we should actually do that.
Also:
Since there are still few examples to look at I'm not sure how to format this code: chained method calls, wrapping long statements etc. I seem to remember Crell showing something similar to this:<?phpdb_update('blocks')->fields(array(
'status' => $block['status'],
'weight' => $block['weight'],
'region' => $block['region'],
))->condition('module', $block['module'])->condition('delta', $block['delta'])->condition('theme', $block['theme'])->execute();
?>
...or even:
<?phpdb_update('blocks')->fields(array(
'status' => $block['status'],
'weight' => $block['weight'],
'region' => $block['region'],
))->condition(
'module',
$block['module']
)->condition(
'delta',
$block['delta']
)->condition(
'theme',
$block['theme']
)->execute();
?>
The fields array works good this way I think, but I don't particularly like the condition() clutter in any of these two examples. Thoughts?I'll be back later with an updated patch.
#2
Ah, so this it probably what we should do:
<?phpdb_update('blocks')->fields(array(
'status' => $block['status'],
'weight' => $block['weight'],
'region' => $block['region'],
))->condition('module', $block['module'])
->condition('delta', $block['delta'])
->condition('theme', $block['theme'])
->execute();
?>
Wasn't aware of that possibility. Looks a lot better.
#3
Here comes a new patch. All tests pass!
It's been pretty easy to port this moule once I got the hang of it.
Just a couple of questions (as above):
– is db_last_insert_id() used in the new API?
– are there any query results that should be converted to objects (instead of using PDO::FETCH_ASSOC)?
– coding style OK?
#4
Oh, and of course, there are still two complicated dynamic queries marked TODO.
#5
The last submitted patch failed testing.
#6
Greetings from Drupalcamp Copenhagen!
Here is a new patch which is updated to reflect recent changes in HEAD, such as changed table names.
I also had to add a couple of (int) casts as PDO was complaining about wrong data types.
The TODO items are still there.
#7
The last submitted patch failed testing.
#8
#337212: DBTNG: Block module
#9
Add tag. I would like to see a tag on the key queries for loading blocks so that modules can alter the query. That means using db_select() instead of db_query().
#10
Closing as duplicate of #337212-10: DBTNG: Block module. That one also contains a block_load tag for loading query...