Steps to reproduce:

  1. Go to a node with a body field and the IPE available to your user
  2. Click "Customize this page"
  3. Click the settings icon on the body field
  4. Change the content in the WYSIWYG, and notice the changes appear in the preview
  5. Click "Continue", and notice the change is still in the preview
  6. Click "Finish"
  7. Notice that in the IPE, it's not showing the changes!
  8. Click "Save"
  9. Notice that the changes aren't on the page!

I'm not sure at what point we broke FAPE, because we have no tests for it. :-/ This feels like it might be related to #2443499: User profile: Unable to chage image field style formatter options because they don't get saved or #2398347: Panopoly Magic abuses FPP revisions but I haven't actually dug into it yet.

CommentFileSizeAuthor
#11 panopoly_test-fape-2459163-11.patch5.8 KBdsnopek
#8 editing_body_of_node_in-2459163-8.patch1.05 KBmglaman
#6 editing_body_of_node_in-2459163-6.patch844 bytesmglaman

Comments

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

The last working version was from Panopoly 1.18!

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Priority: Normal » Critical
Issue tags: +sprint, +Needs tests

Because this is a relatively recent regression, I'm marking this as Critical. Hopefully, we'll be able to get to it at the sprint!

bwood’s picture

bwood commented
Assigned: Unassigned » bwood
bwood’s picture

bwood commented
Assigned: bwood » Unassigned
mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented 
+++ b/panopoly_magic.module
@@ -1076,7 +1081,7 @@ function panopoly_magic_form_ctools_entity_field_content_type_formatter_styles_a
+  if ((empty($form_state['triggering_element']['#next']) || !empty($form_state['field']['body'])) && !empty($form_state['entity']) && !empty($form_state['display_cache'])) {

This logic breaks the FAPE from #2443499: User profile: Unable to chage image field style formatter options because they don't get saved

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Active » Needs review
StatusFileSize
new editing_body_of_node_in-2459163-6.patch844 bytes

Patch

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs review » Needs work

Patch breaks test. Logic needs revisions.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented
Status: Needs work » Needs review
StatusFileSize
new editing_body_of_node_in-2459163-8.patch1.05 KB

Talked with dsnopek, moved the display cache check inside of the body/entity check to provide fix for the image formatter issue.

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented

Trying this patch with the existing tests on Travis:

https://travis-ci.org/panopoly/panopoly/builds/63330570

This still needs Behat tests for the issue itself. @bwood worked on this a bit at the sprint, but unfortunately didn't post his work-in-progress patch and now he's out on vacation. :-/

I'd prefer to not just start over, so I may make an exception to the "panopoly_magic changes must have tests" rule and just commit it. Or if I have the time, maybe I'll just write a new test. Not sure...

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
StatusFileSize
new panopoly_test-fape-2459163-11.patch5.8 KB

I decided to take a stab at writing the test! Sorry, @bwood :-/

Anyway, attached is a patch which adds a test that fails without @mglaman's patch and passes with it.

EDIT: Here's a build on Travis-CI with these patches: https://travis-ci.org/panopoly/panopoly/builds/63347199

  • dsnopek committed d3795a7 on 7.x-1.x 
    Update Panopoly Magic and Test for Issue #2459163 by mglaman, dsnopek:...
dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Status: Needs review » Fixed

Automated tests came back successful! Committed. :-)

Status: Fixed » Closed (fixed)

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

dsnopek’s picture

dsnopek
he/him
Primary language English
Location USA
commented
Parent issue: » #2489306: [meta] Identify everything panopoly_magic is doing and create issues to implement tests for it

Post-humously adding this to #2489306: [meta] Identify everything panopoly_magic is doing and create issues to implement tests for it