Skip to content

Commit

Permalink
Better draft validation & error reporting
Browse files Browse the repository at this point in the history
Fixes #4508
  • Loading branch information
brandonkelly committed Jul 10, 2019
1 parent bb60cf9 commit ee12dfe
Show file tree
Hide file tree
Showing 13 changed files with 335 additions and 192 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG-v3.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@

## Unreleased

### Changed
- If a draft can’t be saved, an alert icon is now shown in the Control Panel header, which can be clicked on to reveal more information.

### Fixed
- Fixed a bug where Feed widget items weren’t getting hyperlinked.
- Fixed a bug where the `app/migrate` controller wasn’t applying new `project.yaml` changes if there were no pending migrations.
- Fixed a SQL error that could occur when saving an entry draft. ([#4508](https://github.com/craftcms/cms/issues/4508))

## 3.2.0 - 2019-07-09

Expand Down
2 changes: 1 addition & 1 deletion src/base/Element.php
Original file line number Diff line number Diff line change
Expand Up @@ -1125,7 +1125,7 @@ public function rules()
}

if (!empty($fieldsWithColumns)) {
$rules[] = [$fieldsWithColumns, 'validateCustomFieldContentSize', 'on' => [self::SCENARIO_DEFAULT, self::SCENARIO_LIVE]];
$rules[] = [$fieldsWithColumns, 'validateCustomFieldContentSize', 'on' => [self::SCENARIO_DEFAULT, self::SCENARIO_LIVE, self::SCENARIO_ESSENTIALS]];
}
}

Expand Down
16 changes: 15 additions & 1 deletion src/controllers/EntryRevisionsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ public function actionSaveDraft()
/** @var Entry|DraftBehavior $draft */
$draft = Craft::$app->getDrafts()->createDraft($entry, Craft::$app->getUser()->getId());
} else {
$transaction = null;

if ($draftId) {
$draft = Entry::find()
->draftId($draftId)
Expand All @@ -201,6 +203,10 @@ public function actionSaveDraft()
throw new NotFoundHttpException('Entry not found');
}
$this->enforceEditEntryPermissions($entry);

// Create the draft in a transaction so we can undo it if something goes wrong
$transaction = Craft::$app->getDb()->beginTransaction();

/** @var Entry|DraftBehavior $draft */
$draft = Craft::$app->getDrafts()->createDraft($entry, Craft::$app->getUser()->getId());
}
Expand All @@ -215,9 +221,13 @@ public function actionSaveDraft()
}

if (!$elementsService->saveElement($draft)) {
if ($transaction !== null) {
$transaction->rollBack();
}

if ($request->getAcceptsJson()) {
return $this->asJson([
'errors' => $draft->getErrors(),
'errors' => $draft->getErrorSummary(true),
]);
}

Expand All @@ -227,6 +237,10 @@ public function actionSaveDraft()
]);
return null;
}

if ($transaction !== null) {
$transaction->commit();
}
}

// Make sure the user is authorized to preview the draft
Expand Down
2 changes: 1 addition & 1 deletion src/templates/_includes/revisionmenu.html
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,4 @@ <h6>{{ "Recent Revisions"|t('app') }}</h6>
</div>

<div id="revision-spinner" class="spinner hidden" title="{{ 'Saving'|t('app') }}"></div>
<div id="revision-saved" class="checkmark-icon invisible" title="{{ 'Saved'|t('app') }}"></div>
<div id="revision-status" class="invisible"></div>
3 changes: 2 additions & 1 deletion src/web/assets/cp/CpAsset.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ private function _registerTranslations(View $view)
'Replace the folder (all existing files will be deleted)',
'Save as a new asset',
'Save',
'Saved',
'Saving',
'Score',
'Search in subfolders',
Expand All @@ -217,6 +216,8 @@ private function _registerTranslations(View $view)
'Structure',
'Submit',
'Table Columns',
'The draft could not be saved.',
'The draft has been saved.',
'This can be left blank if you just want an unlabeled separator.',
'Transfer it to:',
'Try again',
Expand Down
24 changes: 21 additions & 3 deletions src/web/assets/cp/dist/css/_main.scss
Original file line number Diff line number Diff line change
Expand Up @@ -356,20 +356,38 @@ li.sel a {
box-shadow: 0 0 0 1px rgba(0,0,0,0.1);
}

/* checkmark icon */
.checkmark-icon {
/* status icons */
.checkmark-icon,
.alert-icon {
padding: 5px;
margin-bottom: 0 !important;
line-height: 10px;
border-radius: 20px;
background: darken($bgColor, 10%);
cursor: pointer;

&:before {
@include icon;
}
}

.checkmark-icon {
background: darken($bgColor, 10%);

&:before {
content: 'check';
color: $successColor;
}
}

.alert-icon {
background: darken($bgColor, 10%);

&:before {
content: 'alert';
color: $errorColor;
}
}

/* toggles */
.toggle,
a.fieldtoggle:before {
Expand Down
12 changes: 9 additions & 3 deletions src/web/assets/cp/dist/css/craft.css

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

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

Large diffs are not rendered by default.

Loading

0 comments on commit ee12dfe

Please sign in to comment.