Hi there,

I've a couple of ctools plugins (panel panes) that 1.- show the reply form associated with a chosen entity type & field (that can be chosen in the settings of the plugin), and 2.- the replies list posted against that entity.

Would you be willing to accept a patch to include this panels support? I'd just need to remove a couple of specific bits of the client project in which I'm using them, and then can upload the patch.

Cheers!

CommentFileSizeAuthor
#21 2249375-panels_support_21.patch5.87 KBslv_
PASSED: [[SimpleTest]]: [MySQL] 146 pass(es). View
#12 2249375-panels_support_12.patch5.96 KBslv_
PASSED: [[SimpleTest]]: [MySQL] 146 pass(es). View
#10 2249375-panels_support_9.patch5.7 KBslv_
PASSED: [[SimpleTest]]: [MySQL] 115 pass(es). View
#6 2249375-panels_support-6.patch5.72 KBslv_
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2249375-panels_support-6.patch. Unable to apply patch. See the log in the details link for more information. View
#5 2249375-panels_support-5.patch1.73 KBslv_
PASSED: [[SimpleTest]]: [MySQL] 115 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

pameeela’s picture

Hi slv_, patches are welcome if you're still willing!

pameeela’s picture

Status: Active » Postponed
gangaloo’s picture

@Pameeela.

I'm interested in using Reply as the method to do Profile2 comments.
From what I've read, this is not an issue and both modules play nice.

The final question comes to Panels. All i see when I add the the field and form in panels, is "Entity Info".
I disable Panels, and the content shows without any issues.

I'm wondering if it works now with Panels?
Or if this is still in the to-do list?

I didn't see any notes on it besides this post.

Thanks in advance,

slv_’s picture

I'll upload a patch for the 7.x-1.x branch in a few hours.

For the 7.x-2.x, it should be easy to adapt, but I haven't used that release yet, and a simple update in my local installation was making it crashing, so that'll have to wait a bit longer.

slv_’s picture

Status: Postponed » Needs review
FileSize
1.73 KB
PASSED: [[SimpleTest]]: [MySQL] 115 pass(es). View

Right, here it is.

The patch attached:

1.- It adds some notes to the README explaining the panels support provided.
2.- Implements the hook_ctools_plugin_directory() to let ctools know where the 'content_types' plugins are.
3.- Adds a 'reply_add_form' plugin that can be used in panels UI to display the reply form that should be attached to the entity displayed on the page. For that to happen, the plugin requires that a context is passed to it. The context is selected from the settings form of the plugin, and the user is responsible to ensure that the context is an actual entity object. This has to be pretty much like that to make it generic.

Hope this helps. Would be great if you could get that merged @pameeela =). Applying it to 7.x-2.x branch should be reasonably easy, but I ran out of time for it.

slv_’s picture

FileSize
5.72 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 2249375-panels_support-6.patch. Unable to apply patch. See the log in the details link for more information. View

Previous patch didn't include the actual plugin, as git diff won't pick up new files by default. Re-attached complete patch =).

Status: Needs review » Needs work

The last submitted patch, 6: 2249375-panels_support-6.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: 2249375-panels_support-6.patch, failed testing.

slv_’s picture

Status: Needs work » Needs review
FileSize
5.7 KB
PASSED: [[SimpleTest]]: [MySQL] 115 pass(es). View

Ok, trying again....

larowlan’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev

Thanks, this looks great, just some minor requested changes

Also, new features go in 2.x so can this be rolled against that branch?

  1. +++ b/README.txt
    @@ -39,3 +39,9 @@ Q) Is there an easy way to return the reply count for a particular entity the re
    \ No newline at end of file
    

    Can we get a new line here?

  2. +++ b/plugins/content_types/reply_add_form.inc
    @@ -0,0 +1,124 @@
    +function reply_add_form_content_type_admin_info($subtype, $conf) {
    +}
    

    If this is unused, can we remove it?

  3. +++ b/plugins/content_types/reply_add_form.inc
    @@ -0,0 +1,124 @@
    +  foreach ($entity_fields as $field) {
    +    $field_info = field_info_field($field['field_name']);
    +    if ($field_info['type'] == 'reply') {
    +      $options[$field['field_name']] = $field['label'] . ' (' . $field['field_name'] .')';
    +    }
    +  }
    

    Using field_info_field_map() might be more performant here. We can use array_filter to quickly reduce the return to those of type reply.

  4. +++ b/plugins/content_types/reply_add_form.inc
    @@ -0,0 +1,124 @@
    +  foreach ($form_state['values'] as $k => $v) {
    

    Can we use array_filter() here and drop the loop?

slv_’s picture

FileSize
5.96 KB
PASSED: [[SimpleTest]]: [MySQL] 146 pass(es). View

Thanks for the review @larowlan, all those changes make sense to me.

New patch attached. The array_filter() callback for the 'reply' field types is right at the end: "_reply_add_form_filter_reply_fields"

I've adapted the plugin for 7.x-2.x branch, too.

slv_’s picture

Status: Needs review » Active
slv_’s picture

Status: Active » Needs review
larowlan’s picture

Can we use a callable instead of a new function?
I forget what version of php Drupal 7 supports.

slv_’s picture

Sorry I might have explained myself badly. I added that function, but as a callback for array_filter(), rather than being called directly.

ctools does it in other parts (e.g export.inc):

  return $for_export ? array_filter($export_tables, '_ctools_export_filter_export_tables') : $export_tables;

Not sure I understand what would be what you're suggesting. Thought this would be the cleaner way, to avoid using a lambda function there.

larowlan’s picture

Yeah sorry I said callable but meant closure, which is same thing as a lambda function.
If Drupal 7 supports closures, I'd prefer that as it keeps the function logic with the array_filter call.
But it might not be supported in the minimum version of PHP for Drupal 7.

slv_’s picture

Yup, just checked https://www.drupal.org/requirements, and minimum is 5.2.5.

Drupal 7: PHP 5.2.5 or higher (5.4 or higher recommended).

Anonymous (lambda) functions were introduced in PHP 5.3 version (http://php.net/manual/en/functions.anonymous.php).

larowlan’s picture

Issue tags: +Needs tests

Ok, next step is a test.
Since taking over we've been slowly adding test coverage.
Do you have a panels/page manager page you're using this in?
If so can you provide the export of it?
We can put it in a module called reply_test and then enable that in our test.

Our test (reply.test) uses a field named foobar and a node entity (bundle name is ponies).
So with your export we can change the field name and other configuration to suit the existing test and then add a

$this->drupalGet('path/to/your/page/manager/page');
$this->assertFieldByName($reply_field);
$this->drupalPost(NULL, $reply_values, t('Submit'));

To ensure that no-one breaks the behavior.

Thanks again for all your work

slv_’s picture

Right. So just the file with the exportable is what you're after. Did I get that right?

I'll amend what I've in place to remove other unrelated panes and upload it, then.

slv_’s picture

FileSize
5.87 KB
PASSED: [[SimpleTest]]: [MySQL] 146 pass(es). View

Right. New patch attached and export of panels page.

When working with nodes, I just noticed that because I was calling field_info_instancies with the bundle type set to the same as the entity type, it wouldn't work with "node" entities. Since it's better to avoid not passing the bundle, I just refactored that bit slightly, and removed that call directly. Now the loop on the "select field" screen is more efficient and only goes iterates over the actual "reply" field types. Drawback is that the labels are not available, but I don't think that's a problem.

Export:

$page = new stdClass();
$page->disabled = FALSE; /* Edit this to true to make a default page disabled initially */
$page->api_version = 1;
$page->name = 'test_reply_page';
$page->task = 'page';
$page->admin_title = 'Test reply page';
$page->admin_description = '';
$page->path = 'test-reply/%node';
$page->access = array();
$page->menu = array();
$page->arguments = array(
  'node' => array(
    'id' => 1,
    'identifier' => 'Node: ID',
    'name' => 'entity_id:node',
    'settings' => array(),
  ),
);
$page->conf = array(
  'admin_paths' => FALSE,
);
$page->default_handlers = array();
$handler = new stdClass();
$handler->disabled = FALSE; /* Edit this to true to make a default handler disabled initially */
$handler->api_version = 1;
$handler->name = 'page_test_reply_page_panel_context';
$handler->task = 'page';
$handler->subtask = 'test_reply_page';
$handler->handler = 'panel_context';
$handler->weight = 0;
$handler->conf = array(
  'title' => 'Panel',
  'no_blocks' => 0,
  'pipeline' => 'standard',
  'body_classes_to_remove' => '',
  'body_classes_to_add' => '',
  'css_id' => '',
  'css' => '',
  'contexts' => array(),
  'relationships' => array(),
  'access' => array(
    'logic' => 'and',
    'plugins' => array(),
  ),
);
$display = new panels_display();
$display->layout = 'onecol';
$display->layout_settings = array();
$display->panel_settings = array(
  'style_settings' => array(
    'default' => NULL,
    'middle' => NULL,
  ),
);
$display->cache = array();
$display->title = '';
$display->uuid = 'b8464d2e-3885-499a-ad76-bfe20a73faeb';
$display->content = array();
$display->panels = array();
  $pane = new stdClass();
  $pane->pid = 'new-86ec40f8-a82b-4615-ac11-675c06fb0aab';
  $pane->panel = 'middle';
  $pane->type = 'reply_add_form';
  $pane->subtype = 'reply_add_form';
  $pane->shown = TRUE;
  $pane->access = array();
  $pane->configuration = array(
    'next' => 'Continue',
    'cancel' => 'Cancel',
    'context' => array(
      0 => 'argument_entity_id:node_1',
    ),
    'reply_entity_type' => 'node',
    'form_build_id' => 'form-q7Nmakdi-coGRhjc9wOU4F7ETMoEeOqKfgDlqKmR6zE',
    'form_token' => 'CnzyqFrX7pLemUlpBByODGMxxK_hWpK-96j2R8f7CiU',
    'form_id' => 'reply_add_form_select_reply_field',
    'op' => 'Finish',
    'override_title' => 0,
    'override_title_text' => '',
    'previous' => 'Back',
    'return' => 'Finish',
    'reply_field' => 'field_replies_2',
  );
  $pane->cache = array();
  $pane->style = array(
    'settings' => NULL,
  );
  $pane->css = array();
  $pane->extras = array();
  $pane->position = 0;
  $pane->locks = array();
  $pane->uuid = '86ec40f8-a82b-4615-ac11-675c06fb0aab';
  $display->content['new-86ec40f8-a82b-4615-ac11-675c06fb0aab'] = $pane;
  $display->panels['middle'][0] = 'new-86ec40f8-a82b-4615-ac11-675c06fb0aab';
$display->hide_title = PANELS_TITLE_FIXED;
$display->title_pane = '0';
$handler->conf['display'] = $display;
$page->default_handlers[$handler->name] = $handler;
jibran’s picture

Can someone try #2509830: Reply Field is not populated by default? It fixed the panel integration for me.

alexverb’s picture

jibrans' patch works for me too. The patch from this issue itself didn't change anything for me.

paranojik’s picture

Yes #2509830: Reply Field is not populated by default solves it. This issue might be redundant...