I only wanted users to have a limited set of block types they could duplicate so I created a hook in multiblock that would allow me to add more than just multiblock to the blocked list.

The modified code is on 126 - 132

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

NancyDru’s picture

Status: Needs review » Needs work

A) This is not a patch; it is a complete module based on old code (not the current -dev).

B) I see what you want and this needs to be made more generic. Something like "Add feature to exclude modules from the 'Add Instance' list". Given that there is currently no admin interface, this may be more than Andrew wants in his module.

NancyDru’s picture

Assigned: Daemon_Byte » NancyDru
Status: Needs work » Needs review
FileSize
4.83 KB

Try the attached patch to the current -dev release. In order to limit the instance list, you must have "administer site configuration" permission; I did that to limit who can limit the list.

@Andrew: If this looks okay, I'll back port it to 5.x and commit them both.

NancyDru’s picture

FileSize
6.43 KB
6.66 KB

While I was working on back porting this to 5.x, it dawned on me that an additional feature would be simple to add: With these two versions of patch, modules can now block themselves by adding another case to their hook_block (as this patch does to Multiblock itself). If a module wants to block MB, it can implement op='mb_blocked' and return the module name as a string. For example:

function multiblock_block($op = 'list', $delta = 0, $edit = array()) {
  switch ($op) {
    case 'mb_blocked':
      return 'multiblock';
...

With this in place, admins will not be able to add instances of blocks from the blocked module.

The original patch (included in these) adds a new tab to the administer>>blocks page so that the admin can manually list modules that are not to be included in the "add instance" list.

NancyDru’s picture

Status: Needs review » Fixed

Committed. Documentation updated.

andrewlevine’s picture

Status: Fixed » Active

Hey, sorry I didn't chime in earlier. I like this feature but I think it is better for 1.3. Two reasons: it is a fairly complicated change and I would like to test it more, but more importantly I think it makes more sense to specify blocks to prohibit instead of modules. For instance, I may want to prohibit the navigation block without prohibiting the who's online block.

So my vote is to revert this and then release 1.2. What do you think?

NancyDru’s picture

It's your module. I do think the ability for a module to block itself is important though. But I can see limiting specific blocks - maybe in addition to what I already did.

andrewlevine’s picture

It's just as much your module as mine, but if you don't mind I'd rather put this feature off until the next version. I'm fine with getting out a quick release right after, but I want to get this release out tonight and I want to think about this feature more before putting it in.

andrewlevine’s picture

So I really like your hook_block idea in comment #3, but I'm wondering if we implement that if there really is a common use-case for the UI page that does the same thing.

The hook_block case makes sense to me because it can be safely ignored and it gives a module developer/site integrator the power to decide whether a block can be safely cloned. Views pre 2.2 for example may have prevented cloning because it was incompatible. We may also want to add a hook so integrators can exclude a module without hacking it.

If we implemented this, the UI page would only be useful in the case where an administrator wanted to restrict blocks from other sub-administrators and did not know how or want to code. This seems like it could be a rare use-case to add code and confusion for.

Of course I'm likely overlooking something so please tell me if you disagree.

NancyDru’s picture

Well, I actually have 3 modules that already provide either multiple-instance, so they should not be clonable, and thus are candidates for putting that extra op in. As a matter of fact, I did add it to one of them just to test with. Another type of module that shouldn't be cloned is Taxonomy List (one of mine) that shows everything for a vocabulary, so it does not make sense to clone it.

Preventing sub-admins may be a rare case, but is the case that spawned this patch.

If you back this out for now, you probably should also go comment out the section from the doc page too.

I'm okay with whatever you decide. I agree that getting a new release out is important. I almost did it myself.

andrewlevine’s picture

Version: 6.x-1.1 » 6.x-1.x-dev
Priority: Minor » Normal

Nancy, to be clear, what I was saying in #8 is that my preference is to leave the mb_blocked op and to remove the admin/build/block/blocked_modules page interface.

You use this feature more than I do so I'll leave it up to you. Let me know what you think should stay in and I'll modify/test the patch for you so you'll have more time to work on one of your other modules.

NancyDru’s picture

Status: Active » Postponed (maintainer needs more info)

It would be nice if Daemon_Byte would weigh in on this since he is the one who asked for this feature. If I understand his request correctly, the UI is what he asked for, but maybe more specific blocks rather than by module.

  • NancyDru committed 4730840 on 8.x-1.x
    #359688 by NancyDru for Daemon_Byte - Add feature to block instances...
  • andrewlevine committed 9af439c on 8.x-1.x
    Reverting patch from #359688 to defer until next release
    
    
intrafusion’s picture

Issue summary: View changes
Status: Postponed (maintainer needs more info) » Closed (won't fix)