Expose Fieldable Panels Panes as blocks

Wondering why we can't just re-use the existing "Make this entity reusable" field? That would make sense to me. I want this item to be able to be placed in Panels and/or Blocks.

#3: fieldable_panels_panes-render_as_block-1910934-3.patch queued for re-testing.

Update issue description to reflect latest patch implementation

The patch in #3 adds a dependency to Entity.

Here's a quick-and-dirty updated patch to #3 that just adds a dependency for Entity. Should probably evaluate whether this really warrants adding Entity as a dependency.

I disagree with #2 about reusing the "Make this entity reusable" field. You may want to have many reusable panes without having to create lots of blocks. That is the current behaviour.

IMO It makes sense to keep that as a separate field like in patch #1. If it is not accepted the functionality could be moved to another contrib module (and in that case, reusing that checkbox like in patch #7 could be ok).

I think it'd be useful to keep this simple and allow all "reusable" panes are available as blocks. However, there needs to be a global setting to enable this, because I wouldn't want a site to suddenly have hundreds or thousands of records flooding the block system.

That sounds good to me

Attached is the patch from #6 modified as follows and rolled against latest dev:

1. Defined 'fieldable_panels_panes_expose_as_blocks' variable to store global setting requested by #8
2. Created 'Settings' page at admin/structure/fieldable-panels-panes/settings
3. Modified 'fieldable_panels_panes_block_info' function to check if 'fieldable_panels_panes_expose_as_blocks' var is TRUE before making fieldable entity panes available as blocks
4. Modified 'fieldable_panels_panes_block_view' function to check if 'fieldable_panels_panes_expose_as_blocks' var is TRUE before printing fieldable entity panes as blocks
5. Modified 'fieldable_panels_panes_block_view' function to check if specified entity is reusable before printing entity
   
   (It's possible an entity could be added to a region as a block and later have its reusable status changed to FALSE.)

The dependency to entity is because these functions are used:

entity_view()
entity_load_single()
entity_label()

But they can easily be replaced with these instead:

fieldable_panels_panes_view()
fieldable_panels_panes_load()
fieldable_panels_panes_entity_label_callback()

Revised patch attached.

1. Removes entity dependency
2. Replaces entity_view() with fieldable_panels_panes_view()
3. Replaces entity_load_single() with fieldable_panels_panes_load()
4. Replaces entity_label() with fieldable_panels_panes_entity_label_callback()

When 'fieldable_panels_panes_expose_as_blocks' variable is set to FALSE, the patch from #10 was causing the following error:

Warning: Invalid argument supplied for foreach() in _block_rehash() (line 389 of /var/www/example.com/modules/block/block.module).

This patch corrects this issue by moving '$blocks = array();' and 'return $blocks;' outside the 'if' logic in fieldable_panels_panes_block_info().

FYI entity_label() is in core and *should* be used

@Dave Reid - I'll update the patch to use entity_label() in the context of this issue. Do you want to create a separate issue to replace the fieldable_panels_panes_entity_label_callback() function elsewhere?

Revised patch attached.

Per @Dave Reid, replaces fieldable_panels_panes_entity_label_callback() with entity_label().

@Chris Burge - I didn't realize "entity_label()" is in core - it's sometimes quite confusing what part of entity functionality is in core and what isn't. Actually I don't see "fieldable_panels_panes_entity_label_callback" being called directly anywhere, so no need for another issue.

Updated patch to prepend block title with "FPP(%bundle): ". Without this change, a user would have no idea which blocks are provided by FPP. On sites with more than a handful of reusable FPPs, this would get messy very quickly. Views prepends its block titles with "Views: ".

Has anyone tested this patch or the patch from #15 yet?

Tested it, and it is working.

One whitespace error found

$ git apply fieldable_panels_panes-render_as_block-1910934-17.patch
fieldable_panels_panes-render_as_block-1910934-17.patch:82: trailing whitespace.
  );
warning: 1 line adds whitespace errors.

As for the possibility of lots of unwanted blocks making blocks administration cumbersome, the "Make field entity panes available as blocks" configuration could be made per-bundle, as suggested here: https://www.drupal.org/node/1860118#comment-6818858

Thoughts?

Edit:

Make field entity panes available as blocks

This sounds a bit strange to me. Shouldn't it be "Make fieldable panels panes available as blocks" ?

Whitespace noted.
I agree with changing the language from "Make field entity panes available as blocks" to "Make fieldable panels panes available as blocks".
I can see that exposing FPPs as block on a per-bundle basis would be very useful. For example, a site builder may designate a single FPP bundle to be exposed as blocks. I'll work to add that functionality this coming week.

Here is an updated patch:

• Fixes whitespace
• Changes settings checkbox title to 'Make fieldable panels panes available as blocks'
• Provides ability to specify which FPP bundles to expose as blocks
• • If FPPs are exposed but no bundle is selected, then all FPPs entities are exposed
• Adds 'fieldable_panels_panes_exposed_bundles' function to return array of exposed bundles
• Adds uninstall code to remove variables on uninstall

Looks great. There are some code-style errors and nitpicks. Coder can easily detect all of them.

But existing code does not exactly comply with all Drupal standards either, so this can be committed without modification.

This is a reasonable approach, IMHO.

A small nitpick - I don't see the benefit of the "master" setting - just go with the per bundle setting. Also, as MiroslavBanov pointed out, there are a few coding standards violations.

@DamienMcKenna, you state that you "don't see the benefit of the 'master' setting". Do you see a benefit of an "all" setting, which would expose all FPP bundles and override any per-bundle settings?

@Chris Burge: eh. I'm second-guessing myself. Stick with the global option, but just clean up the patch and it should be GTG.

BTW did you double-check that the blocks don't show twice on the Add Content popup?

Attached patch is cleaned up with Coder.

Regarding blocks appearing twice, I ran the following test:

1. Create FPP named "Test FPP" and expose as a block
2. Create a block named "Test Block"
3. In a Panels edit page, click on "Add Content" for a region
4. Select "Custom blocks" and observe "Test Block"
5. Select "Miscellaneous'" and observe "Test FPP"

Neither "Test Block" nor "Test FPP" appears more than once in the "Add Content" pop up.

With coder, I still see the following, after applying the patch.

fieldable_panels_panes.module

  409 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
      |         | question marks

...

  746 | WARNING | There must be no blank line following an inline comment
  746 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
      |         | question marks
  753 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
      |         | question marks
  757 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
      |         | question marks
  761 | WARNING | Line exceeds 80 characters; contains 83 characters
  761 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
      |         | question marks
  782 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
      |         | question marks
  792 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
      |         | question marks
  803 | ERROR   | Function comment short description must end with a full stop
  806 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
      |         | question marks
  819 | WARNING | There must be no blank line following an inline comment
  822 | ERROR   | Function comment short description must end with a full stop

includes/admin.inc:

  10 | ERROR   | Function doc comment must end on the line before the function
     |         | definition
  13 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  36 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  39 | ERROR   | Concat operator must be surrounded by spaces
  41 | WARNING | Only string literals should be passed to t() where possible
  42 | ERROR   | Concat operator must be surrounded by spaces
  47 | ERROR   | Inline comments must end in full-stops, exclamation marks, or
     |         | question marks
  50 | ERROR   | Array indentation error, expected 6 spaces but found 5
  54 | ERROR   | Line indented incorrectly; expected 2 spaces, found 1

This is an old version of coder, I think is not even very strict.

Interdiff between #20 and #27.

Looks good.

Going to hold off on this until the next release.

Chris: Would you mind adding a test or two to confirm the functionality works correctly? Thanks.

I've committed this, with the addition of an update script to clear the menu cache. Oh, and the settings page now also shows the FPP bundle's machine name in addition to its label.

Thanks everyone!

We'll add tests separately, see #2691577: (ongoing) Write more tests.