Skip to content

Commit

Permalink
Focus PR back to original scope
Browse files Browse the repository at this point in the history
Getting validation errors to display nicely on blocks is out of scope -
we're just making it so if there _are_ validation errors, block content
is not lost.
  • Loading branch information
GuySartorelli committed May 15, 2023
1 parent fc19dc9 commit 7b3e94c
Show file tree
Hide file tree
Showing 12 changed files with 110 additions and 222 deletions.
2 changes: 1 addition & 1 deletion client/dist/js/bundle.js

Large diffs are not rendered by default.

38 changes: 18 additions & 20 deletions client/src/legacy/ElementEditor/entwine.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import { destroy } from 'redux-form';

/**
* Reset the Apollo and Redux stores holding data relating to elemental inline edit forms
*
* @param {Array} invalidFieldNames Field names that failed server-side validation
*/
const resetStores = () => {
// After page level saves we need to reload all the blocks from the server. We can remove
Expand Down Expand Up @@ -63,32 +61,32 @@ jQuery.entwine('ss', ($) => {

onunmatch() {
// Reset the store if the user navigates to a different part of the CMS
// Ensure there is not a form submission in progress, as this handler will
// also be triggered from a form submission
if (this.attr('data-submitting-element-editor') !== '1') {
// or after submission if there are no validation errors
if (!$('.cms-edit-form').data('hasValidationErrors')) {
resetStores();
}
ReactDOM.unmountComponentAtNode(this[0]);
},

'from .cms-container': {
onsubmitform() {
this.attr('data-submitting-element-editor', '1');
}
},

'from .cms-edit-form': {
onaftersubmitform(event, data) {
this.attr('data-submitting-element-editor', null);
const validationResultPjax = JSON.parse(data.xhr.responseText).ValidationResult;
const validationResult = JSON.parse(validationResultPjax.replace(/<\/?script[^>]*?>/g, ''));

// Reset redux store if form is succesfully submitted so apollo to refetches element data
// Do not rest if there are any validation errors from either the ElementalAreaField or a
// regular page field because we want redux to hydrate the form, rather than then refetching
// which will return a value from the database. Instead the user should still
// see any modfied value they just entered, whether valid or invalid
if (validationResult.isValid) {
if (validationResultPjax) {
const validationResult = JSON.parse(validationResultPjax.replace(/<\/?script[^>]*?>/g, ''));

// Reset redux store if form is succesfully submitted so apollo to refetches element data
// Do not reset if there are any validation errors because we want redux to hydrate the
// form, rather than then refetching which will return a value from the database.
// Instead the user should still see any modfied value they just entered.
if (validationResult.isValid) {
$('.cms-edit-form').data('hasValidationErrors', false);
resetStores();
} else {
$('.cms-edit-form').data('hasValidationErrors', true);
}
} else {
// No validation result returned
// assume everything is valid and reset stores so data is refetched.
resetStores();
}
}
Expand Down
10 changes: 0 additions & 10 deletions src/Extensions/ElementalAreasExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,12 @@
use DNADesign\Elemental\Forms\ElementalAreaField;
use DNADesign\Elemental\Models\BaseElement;
use DNADesign\Elemental\Models\ElementalArea;
use DNADesign\Elemental\Validators\ElementalAreasValidator;
use SilverStripe\CMS\Model\RedirectorPage;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\CMS\Model\VirtualPage;
use SilverStripe\Core\ClassInfo;
use SilverStripe\Core\Config\Config;
use SilverStripe\Core\Extensible;
use SilverStripe\Forms\CompositeValidator;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\LiteralField;
use SilverStripe\ORM\DataExtension;
Expand Down Expand Up @@ -246,14 +244,6 @@ public function onBeforeWrite()
}
}

/**
* @param CompositeValidator $compositeValidator
*/
public function updateCMSCompositeValidator(CompositeValidator $compositeValidator): void
{
$compositeValidator->addValidator(ElementalAreasValidator::create());
}

/**
* @return boolean
*/
Expand Down
17 changes: 1 addition & 16 deletions src/Forms/EditFormFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@

namespace DNADesign\Elemental\Forms;

use DNADesign\Elemental\Models\BaseElement;
use SilverStripe\Control\RequestHandler;
use SilverStripe\Core\Config\Configurable;
use SilverStripe\Forms\DefaultFormFactory;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\HiddenField;
use SilverStripe\Forms\HTMLEditor\HTMLEditorField;

class EditFormFactory extends DefaultFormFactory
Expand Down Expand Up @@ -37,8 +35,8 @@ public function getForm(RequestHandler $controller = null, $name = self::DEFAULT
// Namespace all fields - do this after getting getFormFields so they still get populated
$formFields = $form->Fields();
$this->namespaceFields($formFields, $context);
$this->addClassNameField($formFields, $context['Record']);
$form->setFields($formFields);

return $form;
}

Expand Down Expand Up @@ -72,17 +70,4 @@ protected function namespaceFields(FieldList $fields, array $context)
$field->setName($namespacedName);
}
}

/**
* @param FieldList $formFields
* @param BaseElement $record
*/
private function addClassNameField(FieldList $formFields, BaseElement $record)
{
$fieldName = sprintf(self::FIELD_NAMESPACE_TEMPLATE, $record->ID, 'ClassName');
$formFields->addFieldsToTab(
'Root.Main',
new HiddenField($fieldName, 'ClassName', get_class($record))
);
}
}
132 changes: 0 additions & 132 deletions src/Validators/ElementalAreasValidator.php

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,3 @@
<div>
<% if $Message %><p class="alert $AlertType" role="alert" id="message-$ID">$Message</p><% end_if %>
<div $AttributesHTML data-schema="$SchemaData.JSON">
<%-- Field is rendered by React components --%>
</div>
<div $AttributesHTML data-schema="$SchemaData.JSON">
<%-- Field is rendered by React components --%>
</div>

20 changes: 14 additions & 6 deletions tests/Behat/Context/FeatureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -240,19 +240,27 @@ public function stepIFillInForForBlock($value, $name, $blockNumber)
public function theFieldForBlockShouldContain($field, $blockNumber, $negate, $content)
{
$block = $this->getSpecificBlock($blockNumber);
$field = $this->findFieldInBlock($block, $field);
$isTinyMCE = $field->getAttribute('data-editor') === 'tinyMCE';
$fieldElem = $this->findFieldInBlock($block, $field);
$isTinyMCE = $fieldElem->getAttribute('data-editor') === 'tinyMCE';

if ($isTinyMCE) {
$this->cmsContext->theHtmlFieldShouldContain(
$field->getAttribute('name'),
$fieldElem->getAttribute('name'),
$negate,
$content
);
} elseif ($negate) {
$this->assertFieldNotContains($field, $content);
return;
}

$actual = (string) $fieldElem->getValue();
$regex = '/^' . preg_quote($content, '/') . '$/ui';

if ($negate) {
$message = sprintf('The field "%s" value is "%s", but "%s" expected.', $field, $actual, $content);
Assert::isTrue((bool) preg_match($regex, $actual), $message);
} else {
$this->assertFieldContains($field, $content);
$message = sprintf('The field "%s" value is "%s", but it should not be.', $field, $actual);
Assert::isFalse((bool) preg_match($regex, $actual), $message);
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/Behat/Context/FixtureContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public function thePageHasABrokenElement(string $type, string $pageTitle, string
*/
public function contentBlocksAreNotInLineEditable()
{
$contentBlockClass = TestElementContent::class;
$contentBlockClass = ElementContent::class;
$config = <<<YAML
---
Name: testonly-content-blocks-not-inline-editable
Expand Down
11 changes: 3 additions & 8 deletions tests/Behat/features/edit-block-element.feature
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,6 @@ Feature: Edit elements in the CMS
And I should see "Alice's Block"
And I should see "Bob's Block"

@sboyd
Scenario: I retain invalid values client-side
When I click on block 1
Then I should see "Alice's Block"
# Then I change Title
# and i save the page

Scenario: I can edit a non in-line editable block
Given content blocks are not in-line editable
And I go to "/admin/pages"
Expand Down Expand Up @@ -77,7 +70,8 @@ Feature: Edit elements in the CMS
And I wait 1 second
Then I should see a "Saved 'Charlie's Block' successfully" notice
When I click on the caret button for block 2
Then I should see "New sample content"
Then the "Content" field for block 2 should contain "New sample content"
And the "Title" field for block 2 should contain "Charlie's Block"

@unsavedChanges
Scenario: I can edit inline-editable blocks and save the page as a whole
Expand All @@ -97,5 +91,6 @@ Feature: Edit elements in the CMS
When I wait 1 second
And I click on block 2
Then the "Content" field for block 2 should contain "<p>Alternate HTML within element 2</p>"
And the "Title" field for block 2 should contain "Alice's Much Improved Block"


Loading

0 comments on commit 7b3e94c

Please sign in to comment.