Block visibility by role

I agree that this functionality is needed in one form or another. I will leave others who need this functionality to try it out and comment on it.

Perhaps it is best that the access by role thing should be unified, instead of having one for nodes and another for blocks, and each with its own set of tables.

You are doing just GREAT for someone who has submitted their first patch to Drupal. You provided an overview for developers, another for users, security risks, description of files included, performance impact, ...etc. That is way more than 99% of other new contributors do.

So, good work and keep it up. Looking forward to more contributions.

I like the functionality of this patch (it's certainly been requested a lot), but the code needs work:

- Code style! All code committed to core needs to follow the drupal coding style conventions. Check the handbook and/or look at existing code. Note bracket placement, quote usage, sql formatting and spacing between operators and quotes. Also, we do not use anything like "// flexer begin" "// flexer end". It only clutters the code. Please avoid spaces at the end of lines. You can easily get rid of them with a regexp find and replace " +$" to "". Try to use consistent variable naming with the rest of drupal (e.g. $result instead of $rs). We prefer using db_fetch_object to db_fetch_array. This is not consistent everywhere, but the best advice is to try and do the same as what the surrounding code does.

- Please make a single, large patch next time with all the changes in it. It is annoying to have to ungzip the patch first. You should add the tables to the database.??sql files. And you should provide an update path in update.php which creates the tables and converts legacy content (in your case there is nothing to convert).

- The help texts seem a bit long winded. You should only add information that actually helps users fill out the form. For example:

The node type selector says:
"Selecting one or more content types will cause this block to only be shown on pages of the selected types."
The role selector says:
"Selecting one or more roles will cause this block to only be shown if the current user belongs to one of the selected roles. If no role is selected, the block is showed to all. Selecting all roles or selecting none, has the same effect."

Users are never going to read all that. The short version says everything you say, just with some implicity.

- Try to avoid redundant comments: say what a piece of code does with a clear, simple sentence. You have:
" // Test if the user resides in at least one role that can access (view) the block
// Match role if necessary "
and a bit lower:
"// Block view depends on user roles"
This sentence is useless because it is already explained. The part about "match role if necessary" does not say anything useful: blocks are role-dependant, so of course the role will be matched.
You should only comment specific pieces of code when needed: for example when there are subtle nuances to pay attention to, or if your code solves a particular problem that's not immediately obvious.

The feature is really nice, and indeed oftenrequested, thanks for looking into this.
I modified the patch a little, to make it more drupalish:
We do not use "and" but &&
you used some ifs that were unneeded: it makes little sense to have aw hile inside an if, with the exact same parameters. I removed that.

I'm not sure about the unset, we usually don't do that, but i left it in for the time being.

I'm not sure about the inital declaration of variables, it is uneecessary in PHP, buand we usually don't do that in Drupal. But i left it in for the time being .

Bèr

Hi,

although i have been searching quite a lot the last couple of days for this functionality, i only found this patch after i wrote my own :-).

Also for me, this is my first contribution, i attach it, so that you guys can discuss it. I took a different approach to tackle the problem. I don't need an extra table in the database:

from a admin point of view:
on the permissions page you can check which roles may have access to the blocks. After creating new blocks you have to visit the permissions page to set correct permissions for the new blocks, otherwise they won't show up.

from a developper point of view:
i create permissions with the same name as the block's title, and add an extra check ("allowed") before displaying the block. This way, no extra tables are needed, no changes are needed upon creation of new blocks. Downside: you have to enable a block and need to check the permissions, which is done in a different place.

I leave it in your hands now :-)

pieter

I made this one a few months ago, just updated View blocks by roles. Look at drumm comments, maybe usefull.
Maybe we would work together ? As far as I see, I don't have any more ideas for improvements, feel free to submit feedback.

Re-activating this... setting status to 'needs work' and setting priority to critical, as it was identified as a top-5 issue for administration usability by Kieran. Djun

Note: tostinni added a pointer to http://drupal.org/node/21353 in duplicate issue http://drupal.org/node/29304 - possibly helpful for 4.7 port.

Marking this as a feature request against CVS.

I really get a feeling that somehow we are missing a «vision». This frigging interface for blocks has changed three times in not even one release!

I am not saying that we should hold back this issue/patch because of this, but really, this is leading no-where.

We need to catch the bigger picture, directly after this patch lands!
* What do we want the blocks visibility to do
* Who do we allow to alter the visibility
* What kinds of visibility do we want (on/off in my profile, for certain pages, for certain users etc)
* What existing interfaces are there where we can borrow ideas (mail filters in eudora/outlook/evolution, eg)

IMO this is a case where small patches Just Don't Get It Done. we need a plan first.

And I would really love Dries speaking up on this.

Because it is annoying to see the amount of effort put into this, and then see it change all the time, within one release cycle.

Here is a patch for flexer's feature request, following the given specification.

There is no additional table; rather, a field 'roles' is added to the blocks table. This contains the allowed roles as a comma separated string. An empty string corresponds to 'visible to anonymous and authenticated' i.e., to all. The administer >> blocks >> configure page requires an additional DB query to load all user roles. Otherwise, no additional DB queries are made.

The code adds a fieldset/multi-select list; deals with getting and setting the values (including exploding list to array and vice-versa), and does a check for visibility. In addition, I've expanded the documentation to explain the changes I've made (as well as other changes introduced since the text was last edited) and cleaned up and simplified some of the other text.

The patch file updates database/database.mysql, database/database.pgsql, database/updates.inc, modules/blocks.module.

The user module is not modified; if a role is deleted; the role will be deleted from the 'roles' field the next time the configuration page for the block is saved. In the meantime, since the role is no longer in the list of a user's roles, no visibility is granted by the existence of the role in the block's roles field; the role will not show up in the list of available roles for the block either.

No PHP evaluation is required, so this is faster than the PHP code approach (though I like the additional flexibility allowed by the PHP visibility block)

In addition, the interface will probably be more intuitive for most users.

Bèr, I don't think that this is 'yet another interface'. I'm basically just finishing the original feature request, using the 4.7 API. This feature request, in turn, extends the request to 'hide blocks for anonymous users'. I think that this approach is complementary to the PHP visibility approach, not a replacement.

However, I do agree with you that taking a step back and looking at the bigger picture/making a plan is a very good idea.

Djun

Please use a separate table instead of cramming the roles into comma separated values. This would break if someone had a role name containing a comma.

I'm concerned about the DB overhead for a site with a lot of blocks. How about we store a comma separated list of role ids rather than role names; this has the advantage that people can rename their roles and still preserve the block behaviour.

A properly crafted JOIN has less overhead then grepping (with PHP) for comma seperated values.
If you have to abuse a column to store structured data, your database model is plain wrong. If you store CSV data in there, or if you structurally store data serialised, then you *really* have to rethink how you are using your database.
Any database archietect will tell you this. Even the database-for-dummies will tell you that.

In other words: A big -1 for selfinvented database architectures. A +1 for the way it is meant to be: a separate table.

And yes: I am aware that we do this in other places, in core, too, but you might have noticed that these areas become less and less, in core. And you might also know that where we did/do this (taxonomy, filters) we had quite some problems with them.

OK, a new table it is, then.

Hi puregin,
Sorry, but I miss the main goal here...
I made the same feature a long time ago as I mentioned it up in this thread.
Then I drop the updates of my patch because a more powerfull (only with a harder UI) has been pushed into core.

So I don't understand what's the goal here ?

Would we make this going into core and getting 2 ways to do the same things ? I know it's a very requested feature, but CVS version already have the possibility to do this, so why add this again ?
Why not built a strong documentation to provide some example to cover all situations with a few lines of php ?

Hi tostinni, I'm aware of the earlier work on this. I've tried to explain the reasons for this in comment #15.

The short answer is 'usability'. The more general solution requires the user to have permission to write PHP, which may not be suitable for many sites. Also, it requires the user (block admin) to write PHP, or at least copy and paste from somewhere, which is not nearly as easy to use as a multi-select list.

Dries talked in Vancouver last week about the Drupal Roadmap. I hope he'll post his slides soon, but one of the most important ideas was understanding the positioning of Drupal in the space of usability vs. functionality. He presented this as a scatterplot of competing systems, with usability on the y-axis (ease of use increasing with y), and functionality on the x-axis. Drupal is a leader in functionality, but needs to improve usability.

The interface that you and flexer (and many others over the years) have proposed, and which I am reviving here, adds moderate functionality while remaining easy to use.

The more general solution is great if, for example, I want a block that appears only on the last Tuesday of a month, or on Feb. 29, or if it's some user's birthday, or.... But it's way too much trouble for most users to understand, based on Kieran's administration usage survey. In other words, this is not just my personal opinion :)

Usability needs that overall «vision» too. Just take thunderbird, kmail, evolution, GMX, gmail or hotmail as an example for "filtering". And tell your mom (your survey clients) to "make all emails that are Foo and BAR, but not boo" go into directory Baz. Most of them will do it right. (I have to note that you will find out that hardly anyone can find the place where to do so, but once given them the interface, it is easy).

We. Drupal. Must look into this. And not come up with those endless "hmm i would like users to be able to show block at Foo only" patches. Simply because these patches are the easiest to make, maintain and to get in, is not a reason to let them go in. We do that far too often.

Puregin: I tried to stress, that this idea should not hold back this very patch. I was only rambling about the endless changes in this particular UI. I am not saying that this is Yet another Interface. I am saying that this is the third big *change* to this particular interface in not even one release.

That fact indicates that this particular area needs not some small pathces. But a goo usability team to rehual the *concept* of blocks, or just the concept of that UI. Again: not for now.

OK, here is a patch which uses a new table {blocks_role} to normalize the many-to-many relation engendered by associating blocks to roles.

To test:

*1. Apply the patch to CVS HEAD
*2. Run update.php to create the new table
 3. Go to administer >> access control >> [roles] and create a new role 'block test role'
 4. Add yourself to the new role: go to administer >> users, select the edit link next to your 
    user account and check the box beside 'block test role'.
 5. Go to administer >> blocks >> [add block], enter a description, title, and test 
    content; save the block
*6. Go to administer >> blocks, click on the configure link next to the new block.  
    Select combinations of roles for which the block should be visible in the 
    'role-specific visibility settings' box, save.
*7. Check to make sure that the block is visible or not, as specified in your 
    role-specific visibility settings.
*8. Read the help text in the role-specific visibility box, on the 
    administer >> blocks page, and at "admin/help/block".   
    Make sure that the instructions are clear, correct, and useful.

Starred items (*) involve behaviour/content specific to this patch.

When reviewing code:

• Please pay attention to the table definitions (database/database.*), especially the PostgreSQL definition, and to update.inc (I've not written many updates, especially with the new system)

Thanks, Djun

Bèr, I think we agree. I'm certainly willing to work on a vision for the block administration interface, or usability more generally.

I understand your point about the many changes to the UI. I think we've seen one neat (general, powerful) solution in the form of the PHP visibility block - I hope that this selection-based interface will complement it, not replace it. The other interfaces seem to be riffs on the same theme.

I blogged a bit about 'efficiency vs. effectiveness' in the Drupal community at http://www.puregin.org/node/1195.

Hi

I rewrote the block module to include the role based settings. its more consistent with the drupal code . Also dosent require a user module patch.
I am attaching the whole module here.

Reagards
Karthikeyan

Please don't hijack issues.

Your 'patch' is a pretty messed up tarball, full of backup files and cruft - please supply a real patch.

Your suggested replacement for block module appears to use the 4.6 form API. Actually, now that I'm looking at it, it kinda looks like you just took flexer's patch, applied, it, and uploaded the works. Hmm.

I suggest if you really want a 4.6 version of this functionality, that you wait until it hits core (you could help this by testing and reviewing) and then backport it to 4.6. No point in having a forked 4.6 block module floating around.

Djun

Hi all
I apologise for the wrong tar file , in a hurry i uploaded the wrong file. well the patch written is similiar to one written by dww at http://drupal.org/node/49154.

Useful patch.

IMO, the user interface should use checkboxes, not a multi-select selection field.

Also, change $rolename to $name or $role_name.

Is the roles-column still needed? The patch against database.mysql and database.pgsql might need updating.

Can't we do a more intelligent join against users_roles to avoid the while-loop-comparison stuff? Just wondering.

Please keep working on this. It is an important patch! Thanks.

is this a bug? Then, please, lets not press this into 4.7. We have enough code to tend to, and enough features that are MUCH more important to others.

Let us PLEASE focus on the outstanding bugs and get 4.7 out of the door so that we THEN can include features like this one.

Bèr

Ber: accoring to the user survey, this was the number one feature request.

Sorry:
According to me, the #1 Drupal developers request is to get 4.7 out of the door.

My comment is about the fact that we do not dare to say "NO, 4.7 COMES FIRST".

Its a lot of politics, but to me it appears that if I spoke to the right people on the right moment a feature suddenly becomes so critical that the whole release will have to wait again. 4.7 is FAR too late. People are waiting for it, people have busyness models running of the "idea" that 4.7 will be out somewhere soon. Etc.

Bèr

Where did I say that getting 4.7 out is less important? My point was that comment #29 has no value whatsoever. Worse, it uses false facts/information to push your politcal message through people's gullet. Let's use the issues for technical discussions.

Nice work puregin.

Attached is an updated and slightly modified version of puregin's patch.

Changes based on previous comments:

* uses checkboxes rather than multiple select to indicate roles for block
* extraneous roles column deleted

Other tweaks:

* Changed table name from blocks_role to blocks_roles (for parallelism with users_roles)
* we needed to have a three-field rather than two-field primary key on the table to maintain uniqueness
* moved block role fetching into separate function, block_get_roles(), as it's needed in more than one place
* simplified some of the code (e.g., using array_intersect() rather than iterating through array), and removed some that seemed unneeded. (Note that some of the code in this patch is unrelated cleanup of text and formatting. I've left that in - it's useful - but it makes reviewing the code a bit more work.)

This works on my basic testing and I think is getting close to ready, but could use some more testing and review.

Looks like I generated that patch before my last tweak (eliminating a few unneeded lines). Here's a fresh one.

Not that I want to block this issue (I really really do not) but...

wouldn't it be better to simply add an extra field to the blocks config table, and that field is an imploded array of roles? The only reason to have a roles table would be in case we wanted to do searching...but I can't think of a reason we need to do that. That would then not add any extra database access, but instead add an explode and an array_intersect against $user->roles.

Thoughts?

As I mentioned before in this thread..

13:02 < drumm> merlinofchaos: unnormalized data in the database usually finds a way to be annoying. for blocks we wouldn't be able to doa proper query for all the displayed blocks
13:03 < drumm> merlinofchaos: please don't promote adding more of this serialized stuff we have been trying to remove
13:08 < merlinofchaos> drumm: So comment to that effect. =)

Nice to see my drupal first patch getting a second breathe.

I was working on updating puregin patch, but nedjo beat me.

Btw, I have a little enhancement to suggest (as mentioned Dries ;) ).

Currently in nedjo patch, there's a loop on the list of blocks and then this line :
$block_roles = block_get_roles($block->module, $block->delta); retrieve the roles for the current block, so one query is made with each block.

I think it would be better to restrict this list from the very first query
$result = db_query("SELECT * FROM {blocks} WHERE theme = '%s' AND status = 1 ORDER BY region, weight, module", $theme_key);

So I made a little query to solve this, but it has been a litte more complicate than I thought because it was decided that with no records in blocks_roles, we would display the blocks.
When I first made the patch, Drumm and I agreed on the fact that with no roles selected, blocks would be invisible. Also the patch were made to update blocks_roles with current blocks and roles in order to set everything visible as before applying the patch.

So the query is :

SELECT b.* 
FROM 
  blocks b 
  LEFT OUTER JOIN blocks_role r 
    ON r.delta = b.delta
    AND r.module = b.module
WHERE b.theme = 'bluemarine'
  AND b.status =1
  AND (r.rid IN (1,2) 
       OR 
       NOT EXISTS (
         SELECT NULL 
         FROM blocks b2
         WHERE b2.delta = b.delta
           AND b2.module = b.module
         )
      )
ORDER BY b.region, b.weight, b.module

--the code to build the IN list is implode(',', array_flip($user->roles))

Note the NOT EXISTS, here is the problem.
I added the NOT EXISTS in order to get the block visiblie if no role is selected but I think it's not a good idea...
In fact I had to switch to MySQL 4.1 because my old 3.23.58 doesn't support the NOT EXISTS clause (it's suppose to work fin according to documentation but it doesn't).

Also I had to change the primary key to be on the 3 columns of blocks_roles if not you can't insert various roles for one given block.

Any thoughts ?

  AND (r.rid IN (1,2)

 AND ((r.rid IN(1,2) OR r.rid IS NULL)

upsss...
Yes you're right merlinofchaos, it's way too easy ;)
I considered this but discarded it without giving it a try ;)

Tostinni, this sounds like an important improvement. Are you going to reroll the patch?

I'll reroll a patch during the day.

Here we go.
I just updated the query as described earlier, removed the block_get_roles function, simplify the block_list and updated the update.inc

Nice tostinni.

I've gone over the patch again and made a few more (last?) refinements.

1. We needed to test for role support when presenting the user-configurable blocks, so users can configure only blocks they have access to. This is done with a join modelled after tostinni's.
2. Pruned down some more unneeded code, e.g., static variable for user roles.
3. Changed description of role checkboxes in block config (wording was still for selects).
4. I was finding the default role settings in block configuration confusing (defaulted to anonymous and authenticated user checked, but if not submitted the real default was visible to all roles). So I've deleted the defaults and the default will be none checked.

I've run this leaner version through its paces and it seems to be working well.

One more pair of eyes so we can mark this ready commit?

I see one of the cleaned up lines uses ucfirst() instead of the drupal version of that.

A little remark on my own : don't use DISTINCTROW clause, it's not very common and won't work in PostgreSQL.

Could we align the page-visibility code with this code, so they are more conform/consistent/similar?

That DISTINCTROW might not work on PostgreSQL though, as hinted by tostinni.

New version with DISTINCTROW gone, using drupal_ucfirst(), and a few more minor cleanups, e.g., removed double spaces after sentences, dropped the formatting of query parameters one per line (more readable, yes, but inconsistent with what we do elsewhere).

Dries, can you point to the page-visibility code you have in mind, or explain more?

what's the story with this issue? i know dries asked if we could "align" the "page visibility" code with this, but apparently no one knows what he's talking about. ;) i think this is a crucially important feature for drupal, and i'd hate to see us not add it to 4.7 because of this. if the goal is to get 4.7.0 RC out before dries gets back from his honeymoon, we either need someone to find the "page visibility" code in question and "align" this patch with it, or we need someone to make the executive decision that this question from dries isn't worthy of delaying such an important usability feature over...

in an effort to help get this committed to CVS, i've rerolled nedjo's latest with a few updates:

• we're now up to system_update_179()
• i changed the text on the block configuration form to match the other settings on that page ("Show block for specific roles:" instead of "Block visibility by role:").

otherwise, i tested carefully, and reviewed nedjo's reroll of puregin's reroll of tostinni's patch (i hope i got that chain of credit right, apologies if i missed something/someone). bottom line: this feature is highly requested, and the attached patch works. dries even replied in this thread (#32) that he thought this issue was no less important than getting 4.7 out...

i don't quite have the courage/gall to set this to RTBC, but i think it is...

please, let's get this committed before 4.7.0 RC.

thanks!
-derek

I am in a bit difficult situation here. :p

Yesterday, I told somebody that I'd only accept bug fixes. OTOH, Dries thinks this is important. :p

Did somebody test the performance impact of this patch?

the update function does not make the table use utf-8
the table is defined as myisam, this is deprecated.

I could not find a performance impact when not using the new feature.

updated patch for the utf8 charset thing... i'm not sure how to do that in postgres, but i don't see "utf8" anywhere in database/database.pgsql, so i'm not going to worry about it. ;)

re: performance, i don't know of a good way to test that (i'm doing most of my development on a local site on my laptop, these days). is there a node somewhere that explains best practices for performance testing?

thanks,
-derek

This is a very tasty feature... The patch definitely applies, but I haven't been able to test it yet.

Great feature. Please don't add features to 4.7. We need a 4.7 release.

[cross posting from development@drupal.org]

argh. ;) dries himself thought this was worthy of getting in 4.7.0. kieran has talked about this feature as the "#1 requested usability feature from admins" (right?). there have been *dozens* of forum posts about it. the code is ready. it's been reviewed and tested many times. for apparently no good reason, it sat here for a month, unloved. :( i'm all for getting 4.7.0 out the door, but spending 10 minutes now to finally commit the patch can save hours (days?) of needless support and admin time hacking around this problem for the entire lifespan of 4.7. i'm also all for speeding up the devel/release cycle, but i'm sure it'll be months before 6.8.0 is out, and at least a year before most drupal sites are running 4.8, probably longer. i think it'd be a shame to force all our users to suffer through another release without this crucial feature. i too, have other features i'd love to see, but they aren't as universally requested as this one, so i'm willing to wait on those. also, since this *does* alter the db schema (it adds a new table), if we don't get it committed before the RC, there's no hope for it before 6.8. i'm sure many of us would be disappointed in that, from dries on down the chain. ;)

steven, can you comment? i think dries left you in charge as the ultimate arbitrator of last resort. ;)

thanks,
-derek

Sorry, I'm with Robert on this. Any additional feature has the potential to break stuff, and we do _not_ need more broken stuff at this point.

Wondering if it would be at all possible to move this to a contrib module for the 4.7 release cycle with some combination of hook_form_alter and hook_init? If so, then definitely postpone. If not, then I guess Steven's our man. ;)

killes asked:

Did somebody test the performance impact of this patch?

in the absence of any better suggestions on how to do this, i used the script from node/43478#comment-80933 and ran some page load-time tests on my test site (my laptop):

• 1.25 GHz PowerPC G4
• Mac OS 10.3.9
• Apache 1.3.33
• PHP 4.4.1
• MySQL 4.1.18

completely fresh cvs HEAD checkout, dropped my DB, recreated as a fresh install, etc.
created uid 1, enabled default blocks (who's online, recent comments)

ran 100 iterations of the page load script from both firefox 1.0.6 and safari 1.3.2 (v312.6):

• firefox: 1.0611093235016
• safari: 1.0211480021477

then, i applied block_roles_5.patch, ran update 179, but didn't select any role-specific visibility settings for any blocks:

• firefox: 1.0999955773354
• safari: 1.0158399748802

finally, i added some role-specific visibility settings to conditionally hide a few blocks, and ran again:

• firefox: 1.218269135952
• safari: 1.0654229974747

the firefox results are exactly what i'd have expected... basically no change at all between the clean HEAD install, and the patched version with the feature disabled, but a roughly 0.1 second increase with the feature enabled. the safari results still showed an increase, but only about 1/2 as much as firefox. so, i tried again (another 100 iterations from each browser, feature enabled):

• firefox: 1.2236393690109
• safari: 1.0669119405746

pretty much the same, apparently not a fluke at all. ;)

of course, this is all 1 possible platform and AMP (no 'L' *grin*) configuration, but the results are pretty clear:

• basically no cost of applying this patch if you don't use the functionality
• slight cost (~4%) for enabling the feature on safari
• higher cost (~13%) for enabling it on firefox

obviously, there are a ton of other variables that can effect this, your timings can (and will) vary, and 100 iterations each isn't exactly the best sample size (but it's what i had time for right now). however, i think that this is entirely reasonable (you only get a performance hit if you use the functionality) and to be expected given the patch...

hope this helps folks decide to apply it. ;)

thanks,
-derek

I can't understand how a backend feature would be browser dependant. There is no Javascript or anything client specific.

A benchmark using ab would rule out any browser specific stuff.

In any case, the patch is harmless if you do not enable the functionality.

i think the differences are all basically explained by: safari loads pages faster than firefox (in every case, safari beat ff), at least for the particular versions i'm using. the results were consistent independent of the browser: clean vs. disabled == same, enabled == slight cost. i'm a little confused why ff seemed to be more impacted, but i'm no browser-peculiarities expert. i certainly agree, this patch is harmless if you don't enable the feature, and only introduces a minor hit if you use it. i'd be curious to see comparisons against having to define custom php blocks that duplicated all the access checking and invoked the core block hooks themselves (which is the pain-in-the-ass alternative for site admins if we don't commit this). but, i don't really want to spend more time doing benchmarking if this feature is going to be postponed...

-1 for 4.7.

Don't think this will apply since there is a system_update_179 already.

There are a few good code style and help text improvements hidden in here. I think it would be a good idea to file these separately so they can get in quickly and this patch will be easier to review.

i'll work on this, but probably not until thursday. if anyone has time today (wednesday), please assign this issue to yourself and indicate you're planning to do the work.

thanks!
-derek

("block visibility" is less likely to confuse people than "block access", IMHO)

at drumm's suggestion, i moved all the help text improvements to a new issue/patch: #62262. the old patch didn't apply anymore for various reasons, so i had to do it all manually. while i was at it, i did another round of edits, attempted to make it more consistent with the other help pages in a few places, etc. also, since it's just improving the docs, i made a 4.7 and HEAD version of the patch, since that could be applied in DRUPAL-4-7, not just HEAD.

the other code-style changes in the old patch were already cleaned up by unconed in revisision 1.204: "#56813: Simplify admin/block code, and fix sorting of blocks in the listing"

so, the only remaining goodness from block_roles_5.patch is the actual feature in the title of this issue. ;) i'm mostly done re-implementing the feature against the current HEAD, but it's getting late and i need sleep. just wanted to update this issue that the help stuff has been moved out, and that i'm making progress on the rest of it.

The multi-select option for enabling blocks by role obviously didn't make it into 4.7 final, so what is the status of the patch and it's implementation in future versions of Drupal. I came across the thread too late, obviously, since this feature is one I always find myself running into. I know how to implement it using php snippets, but from a usability perspective it is a huge issue to include it as a gui type interface.

What's next? What's the condition of the patch?

read my posts directly above. i'm actively working on a) getting this code in shape to be commited to HEAD (for 4.8 and beyond) and b) writing a 4.7 contrib module to allow this functionality in 4.7.x.

since dries specifically mentioned this issue in his telecast today, i figured i should dust it off and get it working with HEAD again. here's an updated patch, applies cleanly to HEAD, heavily tested locally. i went through the old patches with a fine-toothed comb to make sure i grabbed everything worth saving. this patch gets the dww seal of approval. ;) please review and test. let's get this into core before it gets stale again.

thanks!
-derek

Reviewed and tested. Great job.

{fix}

hehe, good one. sorry about that. +1.

btw, thanks a lot to everybody on this thread for a great new feature. :D

Would someone please be kind enough to supply a pre-patched version of the block module (and instructions) so that those of us who cannot patch could benefit from this development?

Such a kind thought was spared to us in 4.6, which is when I started using block_by_role. Now that I come to upgrade to 4.7 I am finding this is the only thing that stops me switching my site over.

I believe this was something that was promised to appear in the off-the-shelf Drupal by 4.7, but never made it. Now that it is a finished project, all work completed, could you please put it into a form we can use?

Hopefully.....
Nigel

I believe this was something that was promised to appear in the off-the-shelf Drupal by 4.7, but never made it. Now that it is a finished project, all work completed, could you please put it into a form we can use?

there are no such promises in drupal development. ;) either the patch gets committed or it doesn't.

the feature is in a form you can use, namely, install 5.0-beta-2. however, i realize that it's premature to recommend that since a) it's beta and b) not all of the contribs are ported.

so, feeling kind (and probably stupid), i decided to provide a patch/tarball for backporting this feature to 4.7.x sites in http://drupal.org/node/103341 since it only took about 10 minutes. read there. use at your own risk, and DO NOT expect any additional help/support for this.

good luck,
-derek

p.s. setting the version/category/etc of this issue back to how it should remain for posterity... this was a feature request that went into the 5.x series.