A patch was made to add dragabble classes for Paragraph items on the back end. This was merged up and closed and after closing, a patch was added to fix an issue with 'Undefined index: #bundle', which was never merged in, I assume due to the issue being closed.

On top of that, that patch fails in PHP 7.1.0, so this patch includes both fixes, and in a new issue that hopefully will get noticed and merged.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ChrisDarke created an issue. See original summary.

ChrisDarke’s picture

Here is the patch that encapsulates the fixes required. Testing on PHP7.1 should result in no issues or warnings with multiple paragraph instances on the edit form.

ChrisDarke’s picture

Status: Active » Needs review
oadaeh’s picture

Assigned: ChrisDarke » Unassigned
Status: Needs review » Reviewed & tested by the community

The patch looks good and fixes the problem (and the tests pass).

miro_dietiker’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/paragraphs.module
@@ -1204,7 +1204,7 @@ function theme_paragraphs_field_multiple_value_form($variables) {
+        'class' => !empty($item['#bundle']) ? array('draggable', drupal_html_class('paragraphs_item_type_' . $item['#bundle'])) : array(),

I would prefer to initialize a $class variable, set it conditionally and use it here. The ternary operator seems to be a bit too long to remain readable.

dhirendra.mishra’s picture

Assigned: Unassigned » dhirendra.mishra

let me do that.

dhirendra.mishra’s picture

Assigned: dhirendra.mishra » Unassigned
Status: Needs work » Needs review
FileSize
872 bytes
609 bytes

Here is the patch.

sickness29’s picture

Re-roll of the patch above for 1.0-rc5 release.

Status: Needs review » Needs work

The last submitted patch, 8: paragraphs-add-bundle-php7-rc5-3010369-8.patch, failed testing. View results

sickness29’s picture

jacob.embree’s picture

Status: Needs work » Needs review

sickness29, generally, it isn't necessary to post patches against a tag. Development happens on a dev branch. If you need to use the patch on a site you can probably download the latest dev and apply the issue's patch against that. If, for some reason, you find it necessary to post a patch against a tag rather than the latest dev then your patch should be marked "Do not test" before you upload it so that the test bot doesn't test it against the latest dev, find that it doesn't apply, and incorrectly mark the issue "Needs work".
Can you review #7 to make sure it works and report if it includes the suggestion recommended in #5?

iStryker’s picture

We are getting the following error. Notice: Undefined index: #bundle in theme_paragraphs_field_multiple_value_form() (line 1176 of /var/www/.../modules/contrib/paragraphs/paragraphs.module).

I believe this way with PHP 5.x for us. It is when we upload multiple values (images). I like patch in #2542066: Add paragraphs bundle class to row in backend, but it should be can be cleaned up.

jacob.embree’s picture

iStryker, I assume you get the notice without applying the patch. Have you tried the patch yet?

Status: Needs review » Needs work

The last submitted patch, 14: paragraphs-add-bundle-class-php7-3010369-14.patch, failed testing. View results

jacob.embree’s picture

#7 is still the preferred patch because #14 uses short array syntax which wasn't introduced until PHP 5.4.
#7 still needs to be reviewed. Once it has been reviewed the status of this issue can be changed to "Reviewed and tested by the community".

jacob.embree’s picture

The followups in #2542066: Add paragraphs bundle class to row in backend should have been in a separate issue. We now have this issue to serve that purpose. #5 in this issue has requested that the class be stored in a variable before insertion into the array. #7 does that. #17 does not. Therefore, #7 is still the preferred patch. We still need a review of #7.

iStryker’s picture

Status: Needs review » Reviewed & tested by the community

Ok #1 and #17 is the same patch. I still think 1 line is better. It is common practice with form elements to have a one-liner if statement. Happens all the time with default values. Maintainer wants to create a variable, so let him create an extra variable. #7 is good. Marking RBTC!

joegraduate’s picture

Issue tags: +PHP 7.0 (duplicate), +PHP 7.1
firewaller’s picture

#7 works for me.

DamienMcKenna’s picture

DamienMcKenna’s picture

DamienMcKenna’s picture

miro_dietiker’s picture

@jstoller committed and still RTBC?

EDIT: I also see that this applies to multiple other 7.x issues. Please check their status.

jacob.embree’s picture

@miro_dietiker, it looks fixed to me. What other issues have bearing here?

miro_dietiker’s picture

@jacob.embree The issue status is still RTBC and not updated to Fixed.

jacob.embree’s picture

Status: Reviewed & tested by the community » Fixed

Yeah, that looks like an oversight, so I'll go ahead correct the status.

jstoller’s picture

Sorry for the confusion. I had to run out the door right after pushing my commits last night. I'm updating issue statuses now.

miro_dietiker’s picture

@jstoller Perfect, thank you. Also @jacob.embree.

dhirendra.mishra’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.