Skip to content

Commit 8f2ac4f

Browse files
committed
Fixes saving an issue when the new Html is empty/deleted.
1 parent 5053783 commit 8f2ac4f

File tree

5 files changed

+76
-76
lines changed

5 files changed

+76
-76
lines changed

assets/js/Components/FixIssuesPage.js

Lines changed: 62 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,10 @@ export default function FixIssuesPage({
463463

464464
setFilteredIssues(tempFilteredContent)
465465
setGroupedList(groupList(tempFilteredContent))
466-
setActiveIssue(holdoverActiveIssue)
466+
if(holdoverActiveIssue) {
467+
setActiveIssue(holdoverActiveIssue)
468+
}
469+
467470
}, [report])
468471

469472

@@ -702,7 +705,7 @@ export default function FixIssuesPage({
702705
return tempDoc.body.innerHTML
703706
}
704707

705-
const handleIssueSave = (issue, markAsReviewed = false) => {
708+
const handleIssueSave = async (issue, markAsReviewed = false) => {
706709

707710
if(!activeContentItem || !activeContentItem?.body || !issue) {
708711
return
@@ -734,83 +737,73 @@ export default function FixIssuesPage({
734737
let fullPageDoc = new DOMParser().parseFromString(fullPageHtml, 'text/html')
735738
let newElement = Html.findElementWithError(fullPageDoc, issue?.newHtml)
736739
let newXpath = Html.findXpathFromElement(newElement)
737-
if(newXpath) {
738-
issue.xpath = newXpath
739-
}
740-
else {
741-
issue.xpath = ""
742-
}
743-
activeContentItem.body = fullPageHtml
744-
740+
issue.xpath = newXpath || ''
741+
745742
// Save the updated issue using the LMS API
746743
let api = new Api(settings)
747744
try {
748-
api.saveIssue(issue, fullPageHtml, markAsReviewed)
749-
.then((responseStr) => {
750-
// Check for HTTP errors before parsing JSON
751-
if (!responseStr.ok) {
752-
processServerError(responseStr)
753-
updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.ERROR)
754-
removeItemFromBeingScanned(issue.contentItemId)
755-
return null
756-
}
757-
return responseStr.json()
758-
})
759-
.then((response) => {
745+
const saveResponse = await api.saveIssue(issue, fullPageHtml, markAsReviewed)
746+
if(!saveResponse.ok){
747+
processServerError(saveResponse)
748+
updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.ERROR)
749+
removeItemFromBeingScanned(issue.contentItemId)
750+
throw Error('Save returned invalid server response.')
751+
}
760752

761-
// If the save falied, show the relevant error message
762-
if (response.data.failed) {
763-
updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.ERROR)
764-
removeItemFromBeingScanned(issue.contentItemId)
765-
response.messages.forEach((msg) => addMessage(msg))
766-
767-
if (Array.isArray(response.data.issues)) {
768-
response.data.issues.forEach((issue) => {
769-
addMessage({
770-
severity: 'error',
771-
message: t(`form.error.${issue.ruleId}`)
772-
})
773-
})
774-
}
753+
const saveResponseJson = await saveResponse.json()
775754

776-
if (Array.isArray(response.data.errors)) {
777-
response.data.errors.forEach((error) => {
778-
addMessage({
779-
severity: 'error',
780-
message: error
781-
})
755+
if(saveResponseJson?.errors && saveResponseJson.errors.length > 0) {
756+
updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.ERROR)
757+
removeItemFromBeingScanned(issue.contentItemId)
758+
saveResponseJson.messages.forEach((msg) => addMessage(msg))
759+
760+
if (Array.isArray(saveResponseJson.errors)) {
761+
saveResponseJson.errors.forEach((error) => {
762+
addMessage({
763+
severity: 'error',
764+
message: t(error)
782765
})
783-
}
766+
})
784767
}
785-
else {
786-
787-
// If the save was successful, show the success message
788-
response.messages.forEach((msg) => addMessage(msg))
789-
790-
if (response.data.issue) {
791-
// Update the report object by rescanning the content
792-
const newIssue = Object.assign({}, issue, response.data.issue)
793-
// const formattedData = formatIssueData(newIssue)
794-
// setActiveIssue(formattedData)
795-
updateActiveSessionIssue(newIssue.id, settings.ISSUE_STATE.SAVED)
796-
797-
api.scanContent(newIssue.contentItemId)
798-
.then((responseStr) => responseStr.json())
799-
.then((res) => {
800-
const tempReport = Object.assign({}, res?.data)
801-
processNewReport(tempReport)
802-
removeItemFromBeingScanned(newIssue.contentItemId)
803-
})
804-
}
805-
else {
806-
// setActiveIssue(formatIssueData(issue))
807-
updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.SAVED)
808-
removeItemFromBeingScanned(issue.contentItemId)
768+
throw Error('Save failed.')
769+
}
770+
771+
// Successful save!
772+
saveResponseJson.messages.forEach((msg) => addMessage(msg))
773+
if(issue.contentItemId === activeContentItem.id) {
774+
activeContentItem.body = fullPageHtml
775+
}
776+
777+
// If there isn't a new issue created, we're done.
778+
if(!saveResponseJson?.data?.issue) {
779+
updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.SAVED)
780+
removeItemFromBeingScanned(issue.contentItemId)
781+
return
782+
}
783+
784+
if(saveResponseJson?.data?.issue) {
785+
// Update the report object by rescanning the content
786+
const newIssue = Object.assign({}, issue, saveResponseJson.data.issue)
787+
updateActiveSessionIssue(newIssue.id, settings.ISSUE_STATE.SAVED)
788+
789+
const scanResponse = await api.scanContent(newIssue.contentItemId)
790+
791+
if(scanResponse.ok) {
792+
const scanResponseJson = await scanResponse.json()
793+
if(scanResponseJson.data) {
794+
const tempReport = Object.assign({}, scanResponseJson.data)
795+
processNewReport(tempReport)
796+
removeItemFromBeingScanned(newIssue.contentItemId)
797+
return
809798
}
810799
}
811-
})
800+
}
801+
802+
updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.SAVED)
803+
removeItemFromBeingScanned(issue.contentItemId)
804+
812805
} catch (error) {
813-
console.error(error)
806+
console.warn(error)
814807
updateActiveSessionIssue(issue.id, settings.ISSUE_STATE.ERROR)
815808
}
816809
}

assets/js/Components/Widgets/FixIssuesContentPreview.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,12 @@ export default function FixIssuesContentPreview({
8989
}
9090

9191
const addPreviewHelperElements = (doc, errorElement) => {
92-
if(!activeIssue || !doc || !errorElement) {
92+
if(!activeIssue || !doc) {
9393
return doc
9494
}
9595

9696
// If the issue edits the alt text, we need to show the auto-updating alt text preview
97-
if (ALT_TEXT_RELATED.includes(formNameFromRule(activeIssue.scanRuleId))) {
97+
if (ALT_TEXT_RELATED.includes(formNameFromRule(activeIssue.scanRuleId)) && errorElement) {
9898
let altText = Html.getAccessibleName(errorElement)
9999

100100
// If there is alt text to show...

assets/js/Services/Html.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ export function findElementWithIssue(content, issue) {
488488
else {
489489
let errorHtml = issue?.sourceHtml || undefined
490490
if(issue.status.toString() === '1') {
491-
errorHtml = issue?.newHtml || errorHtml
491+
errorHtml = issue?.newHtml || undefined
492492
}
493493

494494
if(errorHtml === undefined || errorHtml === '') {

src/Controller/IssuesController.php

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,16 @@ public function saveIssue(
6161

6262
$contentUpdated = true;
6363
// Check if new HTML is different from original HTML
64-
if ($sourceHtml === $newHtml || $issue->getPreviewHtml() === $newHtml || $issue->getNewHtml() === $newHtml) {
64+
if ($newHtml !== '' &&
65+
(
66+
$sourceHtml === $newHtml ||
67+
$issue->getPreviewHtml() === $newHtml ||
68+
$issue->getNewHtml() === $newHtml
69+
)
70+
) {
6571
$contentUpdated = false;
6672
}
6773

68-
$output->writeln("Review updated: ".($reviewUpdated ? 'true' : 'false'));
69-
$output->writeln("Content updated: ".($contentUpdated ? 'true' : 'false'));
70-
7174
if(!$reviewUpdated && !$contentUpdated) {
7275
throw new \Exception('form.error.same_html');
7376
}

src/Response/ApiResponse.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ public function addMessage($msg, $severity = 'info', $timeout = 10000, $visible
3232
'timeout' => $timeout,
3333
'visible' => $visible,
3434
];
35+
36+
if($severity === 'error') {
37+
$this->addError($msg);
38+
}
3539
}
3640

3741
// Undocumented function

0 commit comments

Comments
 (0)