Drupal 4.6 used to have per-content type visibility for blocks, but that was removed in Drupal 4.7 (Some people had gripes about that: http://drupal.org/node/67499, http://drupal.org/node/48569). However, for some settings cleanup around content type help messages (which are ought to become per-content type limited visible blocks for node editing), I think we should reintroduce this feature. Also, many people request this feature and it is blogged as a PHP snippet tip around many places on the internet: http://www.google.com/search?q=drupal+content+type+specific+visibility
I think the per-content type visibility settings were removed in the name of making the interface easier, but I don't consider instructing users to copy-paste PHP code from possibly untrusted sources and running their sites with them a better solution.
The attached patch loads up all blocks which have no content type limitations and only loads up blocks which were set to show up on specific node types when a node of the specific node types are shown, edited or any other subtab is shown under the node (path based limitation can already limit this further in Drupal).
The code works for me on testing with a simple article node and setting up a block limited to the article content type. It does show up when seeing the node or editing it, but it does not show up when seeing other nodes, or not seeing a node. An upgrade path would obviously be required, which would create this new table. Since no such feature existed for quite some time now, there would be no data to migrate. I'll add in the update function is this feature is validated to be worthwhile and accepted. Also, a CHANGELOG entry would be useful IMHO.
Marking migrating help texts for content types at #448784: Migrate content type help to custom blocks postponed on this one.
Comment | File | Size | Author |
---|---|---|---|
#32 | 453254-update.patch | 3.36 KB | agentrickard |
#29 | 453254-29-upgrade.patch | 1.49 KB | Pasqualle |
#22 | 453254-22-block_visibility_node_type.patch | 7.94 KB | Pasqualle |
#16 | 453254-16-block_visibility_node_type.patch | 7.94 KB | Pasqualle |
#14 | 453254-14-block_visibility_node_type.patch | 6.99 KB | Pasqualle |
Comments
Comment #1
Damien Tournoud CreditAttribution: Damien Tournoud commentedI would rather introduce some base context-like features into core. We already have per path and per role visibility settings, this patch suggest to introduce per content type, what's next? Per user id? Per day of the week? Per season? etc.
Putting everything into the block edit interface will simply not scale.
Comment #2
Gábor HojtsyI agree it is already confusing and with this patch it becomes more confusing. I believe however that we should be able to put on more user friendly interfaces on top of what we have now. Nothing tells people currently that all conditions should be met at the same time for example.
I would suggest that blocks would have a rule UI, where you can cook up the rules on when they show up much like you build up a filter for the content type admin listing. You add up and remove items and then see all the filtering criteria at once. I think it helps you understand the and/or relationships. Well, that would still need to support removing rules from the middle of the list, etc, but you get it.
Anyway, I agree that blocks visibility configuration is messy, and the existing options are already confusing (I'd guess that content type visibility was removed because it was confusing). Reintroducing this feature however is the only way I could fix #448784: Migrate content type help to custom blocks, so if we are not going to do this, then I need to mark that won't fix and live with custom setting of help texts that way. I am certainly not up to work on revolutionizing block visibility given how full my plate is already. :)
Comment #3
Pasquallesubscribe
Comment #4
Gábor HojtsyAlso, we'd obviously need tests for this if the new feature is approved.
Comment #5
catchNote there's #370172: refactor block visibility which tries to take the visibility checks out of block module's critical path (even they end up being block's implementations of its own hooks). No interface help though.
I don't think the fact that our existing block interface doesn't scale well is a good reason to avoid putting new features in - same as I don't think the user permissions page is a reason not to make our permissions more granular. However having done a site with lots of regions and blocks and no panels for the first time for a while I remembered how much of a pain it is to deal with large numbers of blocks - so we do need to fix that, I just don't think this patch necessarily has to be dependent on it.
Comment #6
akahn CreditAttribution: akahn commentedHi, Gábor. I'm getting the following error on my site after applying this patch. Am I doing something wrong?
PDOException: SELECT DISTINCT b.* FROM {block} b LEFT JOIN {block_role} r ON b.module = r.module AND b.delta = r.delta LEFT JOIN {block_content_type} c ON b.module = c.module AND b.delta = c.delta WHERE b.theme = ? AND b.status = 1 AND (r.rid IN (?) OR r.rid IS NULL) AND ( c.type IS NULL) ORDER BY b.region, b.weight, b.module - Array ( [0] => garland [1] => 2 ) SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal7.block_content_type' doesn't exist in _block_load_blocks() (line 581 of /Users/akahn/drupal-7/modules/block/block.module).
As someone who builds Drupal sites, this feature would be great to have. I think that just about any reasonably complex web site build with Drupal needs this feature, and I always do it with PHP, which is ok, but it leaves the editors/managers of the site out of the process of block visibility settings. More work for me. ;) A more context-like solution would be better, as Damien suggests, but in the mean time this will help.
Comment #7
Pasqualle@akahn: block_content_type is a new table, you need to reinstall Drupal.
Comment #8
catchgabor - could you run an explain on the modified query and post before/after results?
Comment #9
tstoecklerPatch reviewed and works fine.
Comment #10
Gábor HojtsyTurns out that to reproduce the functionality in #448784: Migrate content type help to custom blocks, we also need to match on paths when the node is created (node/add/$nodetype), when the admin selects the given type. The admin can still exclude such paths via the path admin interface. But the admin cannot add up that path type, since all conditions need to be met at the same time, and node/add/$nodetype does not have a menu_get_object() of type $node.
Comment #11
Pasquallethis patch is an incremental patch after the patch in #370172-30: refactor block visibility
1. fixed the problem in #10
2. fixed a missing db insert at block creation
3. renamed the table to block_node_type
note: please do not change the issue status to CNR as the testing bot can't apply this incremental patch.. You may change it to postponed (on #370172) if you agree that this is the right way to solve this feature..
Comment #12
catchOne minor thing:
Can use ->fetchCol() here I think.
Comment #13
Gábor HojtsyNow that #370172: refactor block visibility is committed, this patch can now build on Drupal 7 HEAD as it is, without depending on other patches.
Comment #14
Pasquallereroll
Comment #15
Gábor HojtsyPatch looks good on a visual inspection. The only concern I have is that we usually don't do these constructs:
We instead usually do
Comment #16
Pasquallefixed #15
Comment #18
lilou CreditAttribution: lilou commentedSetting to 'needs review' - testbot was broken.
Comment #19
tstoecklerMarked #496334: GUI for block visibility per content type as duplicate.
Comment #20
Dries CreditAttribution: Dries commentedThis looks ready to be committed. Marking as such.
Comment #22
Pasquallereroll. no code changed, same as #16.
Comment #24
Pasquallepatch #22 still applies cleanly..
Comment #25
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #26
catchNo upgrade path from D6.
Comment #27
Pasqualle@catch: what kind of upgrade do you expect for this? It is a new setting, which does not require a default value..
Comment #28
catchUmm, the new database table that was introduced and is queried on every page load?
Comment #29
PasqualleComment #30
catchThat's the one.
Comment #31
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD.
Should this live in block.install instead?
Comment #32
agentrickardYes. It should.
Comment #33
webchickThird time's a charm? ;) Committed to HEAD.
Comment #34
HedgeMage CreditAttribution: HedgeMage commentedOOPs! Did that in the wrong tab.
Comment #36
HedgeMage CreditAttribution: HedgeMage commentedOops... I commented in the wrong tab... didn't mean to do anything to this issue...returning to previous status of "fixed"
Comment #37
YesCT CreditAttribution: YesCT commentedI think it was an oops to move from fixed to rtbc. Moving back.
Comment #39
bgogoi CreditAttribution: bgogoi commentedhello! anyoen tested this patch? I need this urgently https://www.drupal.org/node/2639786