Bean Disable block

Thanks for the patch! FYI "Needs review" is the correct status when a patch is uploaded.

I reviewed patch #2

Nice idea.

Patch not working, i append reroll with few fixes:

1) Code format fixes
2) Append missed comments (for example block comments for functions and hooks)
3) Changed path to config page, because previous path can't be found anything in menu
4) Improvements on config form, because there are no any conditions, for example if we haven't beans yet
5) Append removing variable in hook_uninstall

In general, my vision of the settings is completely different. I see this setting in bean type form (when we create or modify bean type) and may be i see it in bean entity form (to override global bean type setting). Because current form in patch not usable, it's not sorted, without pager and when we will have too much blocks it will be not ok.
How you think guys? We can skip current solution or may be move settings to bean type and bean entity forms (although I do not want to overload bean entity).

Related ticket #2836128: Bean unnecessarily fully loads all bean entities on hook_block_info where this issue should be done as i explained above. I will process it.

Also now i understand that solution in #5 not fully work, because it's by default removed all existing blocks.
It work for new projects without content, so you can use it.
By the way i start work on solution with configuration in bean type and bean form. By default blocks will be enabled. In bean type we can disable blocks for any bean type and if blocks disabled for bean type - in each bean we can enable it as block.

According to #2836128: Bean unnecessarily fully loads all bean entities on hook_block_info it's major issue (impact to perfomance)

It's done, please review.

Remove one empty line in patch

Excellent work, everyone.

A few minor things:

• There are a few minor coding standards nitpicks, please do a little polish on this.
• I think the admin form setting to control the new option needs a warning message that explains disabling the option would cause blocks to "disappear" if they had been used via the blocks UI (or Context, or Panels..).
• Please don't use nested ternary operators :)

Otherwise I'm excited to get this committed.

Hi @DamienMcKenna,

Attached new patch and interdiff

Almost there - comments need to wrap at 80 chars.

I see only one place, fixed.

Additional nitpicks:

• The update hook doesn't need to indicate it's an implementation of hook_update_N().
• The description value in $form['block'] shouldn't be split up across multiple lines, it can be left as one long line, and it's missing a period at the end of the sentence.

Looking through the patch itself, I wonder if we really need the option of controlling it on a per-bean basis? Shouldn't it be enough to control it per bean type? Does anyone actually need that level of flexibility?

it can be left as one long line

I did it, but i think we can split to more readability.

and it's missing a period at the end of the sentence.

Sorry, about what period are you talking?

Looking through the patch itself, I wonder if we really need the option of controlling it on a per-bean basis? Shouldn't it be enough to control it per bean type? Does anyone actually need that level of flexibility?

I think, it's case when we have bean type with many entities and only one - two of them should be used as block. In one project on my work we have about 10 bean types and each of them have already > 1000 beans. Sometimes some block inserted on homepage thru block UI, other blocks not needed in block UI.

Almost there :)

• You fixed the minor things in the settings $form['block'] but missed the same items in the bean_form().
• The docblock on BeanTypePluginInterface->getBlockStatus() needs a correct punctuation and indenting.
• I think Bean->getBlockStatus() needs the same docblock as in BeanTypePluginInterface().
• It is best not to use the words "I" or "we" in comments, e.g. instead of "We hide field" just say "Hide the field".

Good point about the bean usage, that can get rather hairy on some sites because it was used in place of ECK.

The docblock on BeanTypePluginInterface->getBlockStatus() needs a correct punctuation and indenting.

I can append only one dot at the first line, but if we look to other places in same file - it's missing everywhere.

I think Bean->getBlockStatus() needs the same docblock as in BeanTypePluginInterface().

I copied it, but with difference. I think they should be different, because in one place it's status of all bean type, but in other place it's status only for bean entity

That's fine about the coding standards, I created #2940809: Clean up the codebase to do a general cleanup of the codebase.

Patch in #22 still applies.

This patch seems to successfully allow you to turn off blocks per bean, but it doesn't change the behavior of caching by bean.module.

Being able to turn off block per bean, plus separation of concerns for block caching as per #2577095: Move Block table integration to a submodule would go a long way towards solving a number of scale issues, and make further scalability changes maintainable.