Commit 7b96d898 authored by Tom Quirk's avatar Tom Quirk

Add error feedback for non-existent design

Previously, when opening Design View, if the design
did not exist (i.e. bad url, wrong version) we
would not handle this case and the loading
spinner would endlessly display.

This commit adds better guards to extractDesign()
util to prevent an exception being thrown in this case,
and handles it explicitly by redirecting user and using
createFlash.

In addition, a few full stops are added to error messages
for consistency.
parent 12a5a651
......@@ -31,7 +31,7 @@ import {
ADD_IMAGE_DIFF_NOTE_ERROR,
UPDATE_IMAGE_DIFF_NOTE_ERROR,
DESIGN_NOT_FOUND_ERROR,
DESIGN_NOT_EXIST_ERROR,
DESIGN_VERSION_NOT_EXIST_ERROR,
designDeletionError,
} from '../../utils/error_messages';
import { DESIGNS_ROUTE_NAME } from '../../router/constants';
......@@ -84,18 +84,8 @@ export default {
return this.designVariables;
},
update: data => extractDesign(data),
result({ data, loading }) {
// On the initial load with cache-and-network policy data is undefined while loading is true
// To prevent throwing an error, we don't perform any logic until loading is false
if (loading) {
return;
}
if (!data) {
this.onQueryError(DESIGN_NOT_FOUND_ERROR);
}
if (this.$route.query.version && !this.hasValidVersion) {
this.onQueryError(DESIGN_NOT_EXIST_ERROR);
}
result(res) {
this.onDesignQueryResult(res);
},
error() {
this.onQueryError(DESIGN_NOT_FOUND_ERROR);
......@@ -215,6 +205,19 @@ export default {
return this.$apollo.mutate(mutationPayload).catch(e => this.onUpdateImageDiffNoteError(e));
},
onDesignQueryResult({ data, loading }) {
// On the initial load with cache-and-network policy data is undefined while loading is true
// To prevent throwing an error, we don't perform any logic until loading is false
if (loading) {
return;
}
if (!data || !extractDesign(data)) {
this.onQueryError(DESIGN_NOT_FOUND_ERROR);
} else if (this.$route.query.version && !this.hasValidVersion) {
this.onQueryError(DESIGN_VERSION_NOT_EXIST_ERROR);
}
},
onQueryError(message) {
// because we redirect user to /designs (the issue page),
// we want to create these flashes on the issue page
......
......@@ -39,7 +39,9 @@ export const findVersionId = id => (id.match('::Version/(.+$)') || [])[1];
export const findNoteId = id => (id.match('DiffNote/(.+$)') || [])[1];
export const extractDesign = data => data.project.issue.designCollection.designs.edges[0].node;
export const extractDesigns = data => data.project.issue.designCollection.designs.edges;
export const extractDesign = data => (extractDesigns(data) || [])[0]?.node;
/**
* Generates optimistic response for a design upload mutation
......
......@@ -20,9 +20,9 @@ export const UPLOAD_DESIGN_INVALID_FILETYPE_ERROR = __(
'Could not upload your designs as one or more files uploaded are not supported.',
);
export const DESIGN_NOT_FOUND_ERROR = __('Could not find design');
export const DESIGN_NOT_FOUND_ERROR = __('Could not find design.');
export const DESIGN_NOT_EXIST_ERROR = __('Requested design version does not exist');
export const DESIGN_VERSION_NOT_EXIST_ERROR = __('Requested design version does not exist.');
const DESIGN_UPLOAD_SKIPPED_MESSAGE = s__('DesignManagement|Upload skipped.');
......
---
title: Fix infinte loading spinner when visiting non-existent design
merge_request: 30263
author:
type: fixed
......@@ -5983,7 +5983,7 @@ msgstr ""
msgid "Could not delete chat nickname %{chat_name}."
msgstr ""
msgid "Could not find design"
msgid "Could not find design."
msgstr ""
msgid "Could not remove the trigger."
......@@ -17440,7 +17440,7 @@ msgstr ""
msgid "Requested %{time_ago}"
msgstr ""
msgid "Requested design version does not exist"
msgid "Requested design version does not exist."
msgstr ""
msgid "Requested states are invalid"
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment