First, thank you for useful module.
Unfortunately I discovered that I cannot use it in its current state because of invalid design solution: blockqueue - block relationship stored in table blockqueue_block is based on block ID (bid). But bid is not stable parameter. It can be changed after adding/removing additional module to the system or changing modules weight. Stable parameters that can be used for block identification are (module, delta) because they are mostly hard-coded in code of module

CommentFileSizeAuthor
#4 blockqueue-6.x-1.8-module_delta_id.patch10.23 KBogorun

Comments

sethcohn’s picture

Priority: Normal » Critical

This is correct. You have to use 'tmd' aka Theme/Module/Delta, which the blocks table uses as the unique key for this exact reason. Essentially, your entire site block list can change on a rebuild, but the tmd shouldn't.

I need to use this module for a project, so I'm going to attempt to port it over from using bid to tmd.

sethcohn’s picture

Priority: Critical » Normal
Status: Active » Closed (works as designed)

Actually, looking it over, it's not as serious as I thought at my first glance.

Bid is unique enough, it won't change (the block list _order_ can, but not the bids), the original poster is incorrect. it's autoincremented primary key.

The one problem of handling modules that might go away and come back again (not reordering), if you run a _block_rehash while they are gone. The rehash will delete the blocks no longer in code, so they will lose their reserved bid #. Now if you bring the module back, the returning blocks will get a new bid, but your blockqueue will still have the old bid number, and so you'd have to add the blocks again...

A solution if this is a problem would be _also_ store the theme/module/delta, and if the bid becomes invalid, see if it's been given a new bid. Not sure how often this is the case though.

I think the original poster confused the block list (which is an array of the blocks in the current order, for a given theme, and is subject to module weighting, for example), with the bid values in the block table. This isn't the first time block bids have caused confusion among Drupalers: #14158: Remove {block} bid, replace with primary key In D6, bid is fine.

sethcohn’s picture

Priority: Normal » Critical
Status: Closed (works as designed) » Active

Well, I'm an idiot, because it turns out it _is_ a problem, for some items like view blocks. Still tracking down exactly when/why, but we're seeing blocks disappear from blockqueues when the nodes inside a view, which creates a block are edited. So this is a big problem to be fixed.

ogorun’s picture

StatusFileSize
new10.23 KB

Sorry that I didn't send patch solving the problem earlier. Please try this patch

ogorun’s picture

Status: Active » Needs review
sethcohn’s picture

Status: Needs review » Needs work

It's a good start... but has some issues still.
It's a major db change so it really needs a install script fix to update the existing tables, or a warning to uninstall and then reinstall the module entirely.
It still needs some overhaul in how it handles itself as a block. (it should be a full block, and not do it's own region/etc, doesn't allow region , doesn't set a subject/title correctly, etc).
It's got the same reuse of variables bug in theme_blockqueue_queue that there is another issue open for.
It shouldn't be doing that major set of db calls in a theming function, bad design and encourages worse design if someone wants to change it, they end up with a poor practice of extensive db code in a theme.

You did get rid of _block_rehash and I'm convinced that's the big problem here. _block_rehash is expensive, and causes all sort of problems. In the normal block.module, I've found you run a risk of corruption merely by having 2 users in the block admin page at the same time, because of the block_rehash call. This module's use of it all of the time, on every block display, and makes it far more likely for that block corruption to happen.

I say all of the above because I've actually rewritten much of the module now and also improved these and other things about it, now. I've added machine name blockqueue ids replacing #s, so potentially the use of ctools import/export is possible, allowing modules to define default blockqueues, for example.

I've emailed gogosan and gotten no reply so far about his status as maintainer, and if he's interested in taking patches, or a co-maintainer, or what.

I do see a nice bit or two you did (cleanup-wise) that I missed, which I'll fold into my version, and it'll get posted shortly.

ogorun’s picture

Thank you for feedback, @sethcohn. I'm waiting for your patch since you are in a further stage of work. Can you clarify this part of post:

"It still needs some overhaul in how it handles itself as a block. (it should be a full block, and not do it's own region/etc, doesn't allow region , doesn't set a subject/title correctly, etc)."

What is the problem you see here now?

sethcohn’s picture

Just put my version into production (which is one reason I'm behind on catching up with issues like this one), and so far, works great. None of the 'disappearing block problem' the old version suffered from.

The problem with blockqueue as written now, is that it attempts to handle some of the things blocks should let the block module do, for itself. It attempts to control region, but fails to provide a 'none' option. It attempts to provide a title, but fails to support no title. It doesn't theme itself as a block, so block control tools looking for proper theming hooks fails.

Additional, as I said, I ended up writing it to support machine names, to get rid of the #s, for use with features, import/exporting with ctools, etc. For future plans, I'd love to add some grid support (display queue as 2 columns, or left,right, etc) Better context support would be good too... You can now use a blockqueue with context, but imagine having blocks in a given queue showing or not, thanks to context.

All in all, it's a substantial rewrite. So far, still no answer from the maintainer, so I'm likely going to post it as a new module, despite the overlap, because I'm already at the point where it would be a 2.0 version due to major changes in the db, and all of the above as well. If I post it merely as a patch, it'll sit here, like the other issues that went unanswered, it seems.

divbox’s picture

+1 for this new module. thanks!

ogorun’s picture

Thank you. I'll be glad to this useful module development either in blockqueue or in another module. Please post in this issue when it is published.

ogorun’s picture

subscribing

divbox’s picture

sorry to be a pest, but any news/updates on the new BBQ module?

sethcohn’s picture

Sorry, was at Design4Drupal in Boston this weekend. I'll be posting something tomorrow, I hope.

sethcohn’s picture

The security issue puts an interesting spin on this... take over the module, or continue the fork.

Plus I need to track down the issue, and ensure it's not in my new version of the code. (I think I left it, as a to be fixed but aware of it thing)

Sorry for delay, I'm out of town for a few days, will decide when I get back the best path...

If someone else wants to take over blockqueue, speak up...

divbox’s picture

any news/update on this, or your updated bbq module?

sethcohn’s picture

I'll be posting things in the next few weeks, trying to get the free time to work on this...

sebastien m.’s picture

Version: 6.x-1.8 » 6.x-1.9
Issue summary: View changes
Status: Needs work » Closed (won't fix)

This module is no more maintained on it's 6.x branch.
The new release under Drupal 7.x is based on both "module" and "delta" data, which avoids any migration issue.
Thanks for notifying it.

So I'll close this issue because won't be fixed under 6.x branch.