Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/dev 212 errors summary #12125

Merged
merged 49 commits into from
Aug 9, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
92c9662
errors summary for entries and categories
i-just Oct 12, 2022
5991ef3
mutisite - validate for all sites when publishing
i-just Oct 12, 2022
2393ef1
errors summary class value update (consistency)
i-just Oct 12, 2022
1b73427
errors summary in slideouts (heading to be done)
i-just Oct 12, 2022
d9dcfa5
tweaks; error summary heading translatable
i-just Oct 12, 2022
fba6d41
Merge branch '4.3' into feature/dev-212-errors-summary
brandonkelly Oct 17, 2022
2de8b27
adjustments to cross site validation
i-just Oct 17, 2022
673db9f
amends to cross site validation logic
i-just Oct 19, 2022
e49a245
basic styling for errors summary
i-just Nov 8, 2022
28c0ed5
Update src/web/assets/cp/src/css/_cp.scss
i-just Nov 9, 2022
13e45a9
Update src/web/assets/cp/src/css/_cp.scss
i-just Nov 9, 2022
2c31290
Update src/web/assets/cp/src/css/_cp.scss
i-just Nov 9, 2022
7b5ebd0
Update src/web/assets/cp/src/css/_cp.scss
i-just Nov 9, 2022
8a14e67
Merge branch 'develop' into feature/dev-212-errors-summary
i-just Nov 9, 2022
2f9b9d2
errors summary accessibility improvements WIP
i-just Nov 9, 2022
2006c68
#12290 fixes & slideout errors summary accessibility
i-just Nov 10, 2022
344413c
bug fixes
i-just Nov 10, 2022
8352178
bug fix
i-just Nov 11, 2022
596e77d
Merge branch 'develop' into feature/dev-212-errors-summary
i-just Nov 23, 2022
022041e
Merge branch '4.4' into feature/dev-212-errors-summary
i-just Nov 23, 2022
d4c563e
href attr for error summary links
i-just Nov 24, 2022
8ac8cb7
amend anchoring errors in summary
i-just Nov 29, 2022
09ecb9a
FieldTest > testBlocks update after adding tabindex
i-just Nov 29, 2022
32d7d24
Update src/services/Elements.php
i-just Dec 1, 2022
b5a28bb
also target fields without "fields-" in data-attribute
i-just Dec 1, 2022
5b2833c
wrap attr name in span in error summary msgs
i-just Jan 11, 2023
bb32024
adjust for native fields; css fixes
i-just Jan 12, 2023
0d2d71b
don't show related element's errors
i-just Jan 12, 2023
3f41ec1
adjustment for custom field labels
i-just Jan 13, 2023
828820b
different approach to wrapping attr name
i-just Jan 16, 2023
f2fac0e
Update src/web/assets/cp/src/css/_main.scss
i-just Jan 17, 2023
393c156
recompiled css
i-just Jan 17, 2023
ebb13dd
change to related element validation message
i-just Jan 25, 2023
3e2d301
remove unused function
i-just Mar 15, 2023
c5bf440
Merge branch 'develop' into feature/dev-212-errors-summary
i-just Mar 15, 2023
b469896
user element test updated
i-just Mar 15, 2023
6b2b677
Merge branch 'develop' into feature/dev-212-errors-summary
i-just Mar 30, 2023
e8a2175
adjust for new tab ids
i-just Mar 30, 2023
e18a4b2
slideout tab dropdown menu - errors indicator
i-just Mar 30, 2023
e549669
compiled assets
i-just Mar 30, 2023
64f8d0d
Merge branch '4.5' into feature/dev-212-errors-summary
brandonkelly Aug 9, 2023
acd274d
Merge branch '4.5' into feature/dev-212-errors-summary
brandonkelly Aug 9, 2023
ba3041a
Fix JS error
brandonkelly Aug 9, 2023
1c32605
Cleanup + remove error summary from slideouts
brandonkelly Aug 9, 2023
b1914fd
const
brandonkelly Aug 9, 2023
f3361e7
Remove getNativeFieldByAttribute() - not used
brandonkelly Aug 9, 2023
7053d78
`@since` tags
brandonkelly Aug 9, 2023
8869a1e
errorsSummary → errorSummary
brandonkelly Aug 9, 2023
1c2368b
Release notes
brandonkelly Aug 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions src/controllers/ElementsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,7 @@ public function actionEdit(?ElementInterface $element, ?int $elementId = null):
$isDraft
))
->notice($element->isProvisionalDraft ? fn() => $this->_draftNotice() : null)
->errorsSummary(fn() => $this->_errorsSummary($element))
->prepareScreen(
fn(Response $response, string $containerId) => $this->_prepareEditor(
$element,
Expand Down Expand Up @@ -802,6 +803,42 @@ private function _editorContent(
return $html;
}

private function _errorsSummary(
ElementInterface $element,
): string {
$html = '';

if ($element->hasErrors()) {
$errorsList = [];
foreach ($element->getErrors() as $errors) {
foreach ($errors as $error) {
$errorsList[] = Html::tag('li', Craft::t('app', $error));
}
}

if (!empty($errorsList)) {
$heading = Craft::t('app', 'Found {num, number} {num, plural, =1{error} other{errors}}:', [
'num' => count($errorsList),
]);

$html = Html::beginTag('div', [
'class' => ['errors-summary'],
]) .
Html::beginTag('h2') .
$heading .
Html::endTag('h2') .
Html::beginTag('ul', [
'class' => ['errors'],
]) .
implode('', $errorsList) .
Html::endTag('ul') .
Html::endTag('div');
}
}

return $html;
}

private function _editorSidebar(
ElementInterface $element,
bool $mergedCanonicalChanges,
Expand Down Expand Up @@ -888,7 +925,7 @@ public function actionSave(): ?Response
}

try {
$success = $elementsService->saveElement($element);
$success = $elementsService->saveElement($element, crossSiteValidate: Craft::$app->getIsMultiSite());
} catch (UnsupportedSiteException $e) {
$element->addError('siteId', $e->getMessage());
$success = false;
Expand Down Expand Up @@ -1280,7 +1317,7 @@ public function actionApplyDraft(): ?Response
$element->setScenario(Element::SCENARIO_LIVE);
}

if (!$elementsService->saveElement($element)) {
if (!$elementsService->saveElement($element, crossSiteValidate: Craft::$app->getIsMultiSite())) {
return $this->_asAppyDraftFailure($element);
}

Expand Down
83 changes: 72 additions & 11 deletions src/services/Elements.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
use craft\helpers\ElementHelper;
use craft\helpers\Queue;
use craft\helpers\StringHelper;
use craft\helpers\UrlHelper;
use craft\i18n\Translation;
use craft\queue\jobs\FindAndReplace;
use craft\queue\jobs\UpdateElementSlugsAndUris;
Expand Down Expand Up @@ -1026,7 +1027,7 @@ public function getEnabledSiteIdsForElement(int $elementId): array
* @throws Exception if the $element doesn’t have any supported sites
* @throws Throwable if reasons
*/
public function saveElement(ElementInterface $element, bool $runValidation = true, bool $propagate = true, ?bool $updateSearchIndex = null): bool
public function saveElement(ElementInterface $element, bool $runValidation = true, bool $propagate = true, ?bool $updateSearchIndex = null, ?bool $crossSiteValidate = false): bool
{
// Force propagation for new elements
$propagate = !$element->id || $propagate;
Expand All @@ -1035,7 +1036,7 @@ public function saveElement(ElementInterface $element, bool $runValidation = tru
$duplicateOf = $element->duplicateOf;
$element->duplicateOf = null;

$success = $this->_saveElementInternal($element, $runValidation, $propagate, $updateSearchIndex);
$success = $this->_saveElementInternal($element, $runValidation, $propagate, $updateSearchIndex, crossSiteValidate: $crossSiteValidate);
$element->duplicateOf = $duplicateOf;
return $success;
}
Expand Down Expand Up @@ -2760,6 +2761,7 @@ private function _saveElementInternal(
?bool $updateSearchIndex = null,
?array $supportedSites = null,
bool $forceTouch = false,
bool $crossSiteValidate = false,
): bool {
/** @var ElementInterface|DraftBehavior|RevisionBehavior $element */
$isNewElement = !$element->id;
Expand Down Expand Up @@ -3020,7 +3022,9 @@ private function _saveElementInternal(
// Skip the initial site
if ($siteId != $element->siteId) {
$siteElement = $siteElements[$siteId] ?? false;
$this->_propagateElement($element, $supportedSites, $siteId, $siteElement);
if (!$this->_propagateElement($element, $supportedSites, $siteId, $siteElement, crossSiteValidate: $crossSiteValidate)) {
return false;
}
}
}
}
Expand Down Expand Up @@ -3153,14 +3157,17 @@ private function _saveElementInternal(
* @param array $supportedSites The element’s supported site info, indexed by site ID
* @param int $siteId The site ID being propagated to
* @param ElementInterface|false|null $siteElement The element loaded for the propagated site
* @param bool $crossSiteValidate Whether we should "live" validate the element across all sites
* @retrun bool
* @throws Exception if the element couldn't be propagated
*/
private function _propagateElement(
ElementInterface $element,
array $supportedSites,
int $siteId,
ElementInterface|false|null $siteElement = null,
) {
bool $crossSiteValidate = false,
): bool {
// Make sure the element actually supports the site it's being saved in
if (!isset($supportedSites[$siteId])) {
throw new UnsupportedSiteException($element, $siteId, 'Attempting to propagate an element to an unsupported site.');
Expand Down Expand Up @@ -3241,17 +3248,71 @@ private function _propagateElement(

// Save it
$siteElement->setScenario(Element::SCENARIO_ESSENTIALS);

// for Entries and Categories: validate element against "live" scenario across all sites,
// if element is to be set to live/pending/enabled
if ($crossSiteValidate && ($siteElement instanceof Entry || $siteElement instanceof Category)) {
i-just marked this conversation as resolved.
Show resolved Hide resolved
$siteElementStatus = $siteElement->getStatus();
if (($siteElement instanceof Entry &&
($siteElementStatus === Entry::STATUS_LIVE || $siteElementStatus === Entry::STATUS_PENDING)
) ||
($siteElement instanceof Category && $siteElementStatus === Category::STATUS_ENABLED)
) {
$siteElement->setScenario(Element::SCENARIO_LIVE);
}
i-just marked this conversation as resolved.
Show resolved Hide resolved
}

$siteElement->propagating = true;

if ($this->_saveElementInternal($siteElement, true, false, null, $supportedSites) === false) {
// Log the errors
$error = 'Couldn’t propagate element to other site due to validation errors:';
foreach ($siteElement->getFirstErrors() as $attributeError) {
$error .= "\n- " . $attributeError;
if ($this->_saveElementInternal($siteElement, true, false, null, $supportedSites, crossSiteValidate: $crossSiteValidate) === false) {
// if the element we're trying to save has validation errors, notify original element about them
if ($siteElement->hasErrors()) {
return $this->_crossSiteValidationErrors($siteElement, $element);
} else {
// Log the errors
$error = 'Couldn’t propagate element to other site due to validation errors:';
foreach ($siteElement->getFirstErrors() as $attributeError) {
$error .= "\n- " . $attributeError;
}
Craft::error($error);
throw new Exception('Couldn’t propagate element to other site.');
}
Craft::error($error);
throw new Exception('Couldn’t propagate element to other site.');
}

return true;
}

/**
* @param ElementInterface $siteElement
* @param ElementInterface $element
* @return bool
* @throws Throwable
*/
private function _crossSiteValidationErrors(
ElementInterface $siteElement,
ElementInterface $element,
): bool {
// get site we're propagating to
$propagateToSite = Craft::$app->getSites()->getSiteById($siteElement->siteId);
$user = Craft::$app->getUser()->getIdentity();
$message = Craft::t('app', 'Validation errors for site {siteName}', [
'siteName' => $propagateToSite->name,
]);

// check user can edit this element for the site that throws validation error on propagation
if ($user &&
Craft::$app->getIsMultiSite() &&
$user->can("editSite:{$propagateToSite->uid}") &&
$siteElement->canSave($user)
) {
$queryParams = ArrayHelper::without(Craft::$app->getRequest()->getQueryParams(), 'site');
$url = UrlHelper::url($siteElement->getCpEditUrl(), $queryParams + ['prevalidate' => 1]);
$message .= " <a href=\"$url\">(" . Craft::t('app', 'View') . ")</a>";
}

$element->addError('global', $message);

return false;
}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/templates/_layouts/cp.twig
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
{% set footer = (footer ?? block('footer') ?? '')|trim %}
{% set crumbs = crumbs ?? null %}
{% set tabs = (tabs ?? [])|length > 1 ? tabs : null %}
{% set errorsSummary = errorsSummary ?? null %}

{% set mainContentClasses = [
sidebar ? 'has-sidebar',
Expand Down Expand Up @@ -247,6 +248,9 @@
<h2 id="content-heading"></h2>
{% endif %}
{% block main %}
{% if errorsSummary is not empty %}
{{ errorsSummary is defined ? errorsSummary|raw }}
{% endif %}
<div id="content" class="content-pane">
{% if contentNotice or tabs %}
<header id="content-header" class="pane-header">
Expand Down
1 change: 1 addition & 0 deletions src/translations/en/app.php
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,7 @@
'Forgot your password?' => 'Forgot your password?',
'Format' => 'Format',
'Formatting Locale' => 'Formatting Locale',
'Found {num, number} {num, plural, =1{error} other{errors}}:' => 'Found {num, number} {num, plural, =1{error} other{errors}}:',
'Free' => 'Free',
'From {date}' => 'From {date}',
'From' => 'From',
Expand Down
1 change: 1 addition & 0 deletions src/translations/pl/app.php

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 33 additions & 0 deletions src/web/CpScreenResponseBehavior.php
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,13 @@ class CpScreenResponseBehavior extends Behavior
*/
public $notice = null;

/**
* @var string|callable|null The errors summary HTML (DEV-212).
* @see errorsSummary()
* @see errorsSummaryTemplate()
*/
public $errorsSummary = null;

/**
* Sets a callable that will be called before other properties are added to the screen.
*
Expand Down Expand Up @@ -594,4 +601,30 @@ public function noticeTemplate(string $template, array $variables = []): Respons
fn() => Craft::$app->getView()->renderTemplate($template, $variables, View::TEMPLATE_MODE_CP)
);
}

/**
* Sets the errors summary HTML.
*
* @param callable|string|null $value
* @return Response
*/
public function errorsSummary(callable|string|null $value): Response
{
$this->errorsSummary = $value;
return $this->owner;
}

/**
* Sets a template that should be used to render the errors summary HTML.
*
* @param string $template
* @param array $variables
* @return Response
*/
public function errorsSummaryTemplate(string $template, array $variables = []): Response
{
return $this->errorsSummary(
fn() => Craft::$app->getView()->renderTemplate($template, $variables, View::TEMPLATE_MODE_CP)
);
}
}
5 changes: 5 additions & 0 deletions src/web/CpScreenResponseFormatter.php
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ private function _formatJson(\yii\web\Request $request, YiiResponse $response, C

$sidebar = $behavior->sidebar ? $view->namespaceInputs($behavior->sidebar, $namespace) : null;

$errorsSummary = $behavior->errorsSummary ? $view->namespaceInputs($behavior->errorsSummary, $namespace) : null;

$response->data = [
'editUrl' => $behavior->editUrl,
'namespace' => $namespace,
Expand All @@ -92,6 +94,7 @@ private function _formatJson(\yii\web\Request $request, YiiResponse $response, C
'action' => $behavior->action,
'content' => $content,
'sidebar' => $sidebar,
'errorsSummary' => $errorsSummary,
'headHtml' => $view->getHeadHtml(),
'bodyHtml' => $view->getBodyHtml(),
'deltaNames' => $view->getDeltaNames(),
Expand All @@ -115,6 +118,7 @@ private function _formatTemplate(YiiResponse $response, CpScreenResponseBehavior
$notice = is_callable($behavior->notice) ? call_user_func($behavior->notice) : $behavior->notice;
$content = is_callable($behavior->content) ? call_user_func($behavior->content) : ($behavior->content ?? '');
$sidebar = is_callable($behavior->sidebar) ? call_user_func($behavior->sidebar) : $behavior->sidebar;
$errorsSummary = is_callable($behavior->errorsSummary) ? call_user_func($behavior->errorsSummary) : $behavior->errorsSummary;

if ($behavior->action) {
$content .= Html::actionInput($behavior->action, [
Expand Down Expand Up @@ -154,6 +158,7 @@ private function _formatTemplate(YiiResponse $response, CpScreenResponseBehavior
'contentNotice' => $notice,
'content' => $content,
'details' => $sidebar,
'errorsSummary' => $errorsSummary,
],
'templateMode' => View::TEMPLATE_MODE_CP,
]);
Expand Down
1 change: 1 addition & 0 deletions src/web/assets/cp/CpAsset.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ private function _registerTranslations(View $view): void
'Folder deleted.',
'Folder renamed.',
'Format',
'Found {num, number} {num, plural, =1{error} other{errors}}:',
'From {date}',
'From',
'Give your tab a name.',
Expand Down
2 changes: 1 addition & 1 deletion src/web/assets/cp/dist/cp.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/web/assets/cp/dist/cp.js.map

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions src/web/assets/cp/src/js/CpScreenSlideout.js
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,7 @@ Craft.CpScreenSlideout = Craft.Slideout.extend(
Craft.cp.displayError(data.message);
if (data.errors) {
this.showErrors(data.errors);
this.showErrorsSummary(data.errors);
}
},

Expand All @@ -539,6 +540,23 @@ Craft.CpScreenSlideout = Craft.Slideout.extend(
this.fieldsWithErrors.forEach(($field) => {
Craft.ui.clearErrorsFromField($field);
});
Craft.ui.clearErrorsSummary(this.$body);
},

/**
* @param {string[]} errors
*/
showErrorsSummary: function (errors) {
var allErrors = [];
Object.values(errors).forEach((fieldErrors) => {
for (var i = 0; i < fieldErrors.length; i++) {
allErrors.push(fieldErrors[i]);
}
});

if (allErrors.length > 0) {
Craft.ui.showErrorsSummary(this.$body, allErrors);
}
},

isDirty: function () {
Expand Down
Loading