silverstripe / silverstripe-userforms

UserForms module provides a visual form builder for the Silverstripe CMS. No coding required to build forms such as contact pages.
BSD 3-Clause "New" or "Revised" License
134 stars 226 forks source link

Form not showing when Elemental is installed #790

Closed silbinarywolf closed 5 years ago

silbinarywolf commented 6 years ago

Please note the following steps were taken on a project with which I have very little familiarity.

The problem (pre-investigation) I have the UserDefinedForm module and Elemental installed. My UserDefinedForm.ss template looks like the following:

$ElementalArea
$Form

However, the $Form is refusing to render and I initially had no ideas on why that wasn't working.

The problem (post-investigation) So after digging into the code of UserDefinedFormController, I deduced that the index action is blanking out the "Form" property, because $Content defaults to having "$UserDefinedForm" when you create the page.

EDIT: If you don't know, the $Content field is completely removed / hidden in the CMS when you have Elemental installed. This made discovery of this problem tough!

The proposed solution Add a config that disables the blanking out of the "Form". Modules like Elemental or Blocks could enable this OOTB.

ie.

public function index(HTTPRequest $request = null)
{
    // Add this vvvv
    if ($this->config()->disable_form_content_interpolation) {
        return [];
    }
    if ($this->Content && $form = $this->Form()) {
        $hasLocation = stristr($this->Content, '$UserDefinedForm');
        if ($hasLocation) {
            /** @see Requirements_Backend::escapeReplacement */
            $formEscapedForRegex = addcslashes($form->forTemplate(), '\\$');
            $content = preg_replace(
                '/(<p[^>]*>)?\\$UserDefinedForm(<\\/p>)?/i',
                $formEscapedForRegex,
                $this->Content
            );
            return [
                'Content' => DBField::create_field('HTMLText', $content),
                'Form' => ''
            ];
        }
    }

    return [
        'Content' => DBField::create_field('HTMLText', $this->Content),
        'Form' => $this->Form()
    ];
}

Why not use Elemental UserForms?

robbieaverill commented 6 years ago

Sounds reasonable =)

tardinha commented 6 years ago

Agreed. This is a big issue on our sites!

stephenmcm commented 6 years ago

Bump. @robbieaverill this and it looks like a couple of other open issues are fixed in 5.2.x-dev it might be worth doing a 5.2.1 tag? Not sure what sort of release cycle userforms is currently following, but I never like doing -dev unless I'm testing stuff.

ScopeyNZ commented 6 years ago

Hmmmn. It probably would've been released by now if it wasn't marked as a minor change. Looks to me like it's just a bugfix, no breaking changes... I will cherry-pick it back into 5.2 and tag a release over the next few days.

dhensby commented 6 years ago

@ScopeyNZ we don't define "bug fixes" based on whether they are breaking changes; breaking changes have to go into new major releases.

If it's a new feature (which this is) then it goes into the next minor release, if it's a bug fix, then it goes into the patch release.

ScopeyNZ commented 6 years ago

Sorry I assumed it was a bug fix from reading the context not the label. It seems a bit broken to me. I won't cherry-pick then, but I think there's a good case for treating this as a patch bug fix.

stephenmcm commented 6 years ago

Yeah fair this should be a minor version bump. I smashed out that reply without putting much thought into versioning. Happy to wait on a proper release.

stephenmcm commented 6 years ago

Is there any ETA on a minor release? There's a handful of commits ready to go into one now by the look of it.

robbieaverill commented 6 years ago

@stephenmcm most likely in the next week or two

tardinha commented 5 years ago

@robbieaverill did this ever get fixed?

NightJar commented 5 years ago

@tardinha Yes, in 5.2.1 it sounds like. Thanks for the bump.

NightJar commented 5 years ago

Cancel that sorry @tardinha - looking at the change logs for the releases, looks like it finally made it out in 5.3.0

tardinha commented 5 years ago

@NightJar do you know if this relies on any particular version of elemental?

We have 5.3.0 already running with SS 4.2.4 with elemental 3.x, and forms aren't showing.

NightJar commented 5 years ago

Oh, no sorry, I just read back on this comment thread. It seems I was too hasty to close :)

tardinha commented 5 years ago

For anyone following along at home...

SilverStripe\UserForms\Control\UserDefinedFormController:
  disable_form_content_shortcode: true
NightJar commented 5 years ago

The important part of the above snippet (thanks @tardinha ) is that there also needs to be an After header to ensure the setting is not then overwritten by the userforms included configuration:

app/_config/testforms.yml:

---
Name: testforms
After: userformsconfig
---
SilverStripe\UserForms\Control\UserDefinedFormController:
  disable_form_content_shortcode: true

This fix is however limited to project wide configuration, and is not configurable per subclass for reasons I was unable to uncover (in my testing, it is entirely possible I had misconfigured something). But the fix in userforms 5.3.0 does work.