diff --git a/guacamole/src/main/frontend/src/app/import/controllers/importConnectionsController.js b/guacamole/src/main/frontend/src/app/import/controllers/importConnectionsController.js index 73acb1cc3..ec1227e95 100644 --- a/guacamole/src/main/frontend/src/app/import/controllers/importConnectionsController.js +++ b/guacamole/src/main/frontend/src/app/import/controllers/importConnectionsController.js @@ -49,6 +49,21 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$ */ $scope.error = null; + /** + * The result of parsing the current upload, if successful. + * + * @type {ParseResult} + */ + $scope.parseResult = null; + + /** + * The failure associated with the current attempt to create connections + * through the API, if any. + * + * @type {Error} + */ + $scope.patchFailure = null;; + /** * True if the file is fully uploaded and ready to be processed, or false * otherwise. @@ -100,6 +115,8 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$ $scope.fileData = null; $scope.mimeType = null; $scope.fileReader = null; + $scope.parseResult = null; + $scope.patchFailure = null; // Broadcast an event to clear the file upload UI $scope.$broadcast('clearFile'); @@ -254,10 +271,7 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$ function cleanUpConnections(creationResponse) { return connectionService.patchConnections( - $routeParams.dataSource, createDeletionPatches(creationResponse)) - - // TODO: Better error handling? Make additional cleanup requests? - .catch(handleError); + $routeParams.dataSource, createDeletionPatches(creationResponse)); } @@ -274,10 +288,7 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$ function cleanUpUsers(creationResponse) { return userService.patchUsers( - $routeParams.dataSource, createDeletionPatches(creationResponse)) - - // TODO: Better error handling? Make additional cleanup requests? - .catch(handleError); + $routeParams.dataSource, createDeletionPatches(creationResponse)); } @@ -294,10 +305,7 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$ function cleanUpUserGroups(creationResponse) { return userGroupService.patchUserGroups( - $routeParams.dataSource, createDeletionPatches(creationResponse)) - - // TODO: Better error handling? Make additional cleanup requests? - .catch(handleError); + $routeParams.dataSource, createDeletionPatches(creationResponse)); } @@ -352,6 +360,15 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$ */ function handleParseSuccess(parseResult) { + $scope.processing = false; + $scope.parseResult = parseResult; + + // If errors were encounted during file parsing, abort further + // processing - the user will have a chance to fix the errors and try + // again + if (parseResult.hasErrors) + return; + const dataSource = $routeParams.dataSource; console.log("parseResult", parseResult); @@ -372,7 +389,12 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$ .then(resetUploadState) )); - }); + }) + + // If an error occured when the call to create the connections was made, + // skip any further processing - the user will have a chance to fix the + // problems and try again + .catch(patchFailure => { $scope.patchFailure = patchFailure; }); } /** @@ -433,7 +455,7 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$ else { handleError(new ParseError({ message: 'Invalid file type: ' + mimeType, - key: 'CONNECTION_IMPORT.INVALID_FILE_TYPE', + key: 'IMPORT.INVALID_FILE_TYPE', variables: { TYPE: mimeType } })); return; @@ -459,8 +481,8 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$ /** * @return {Boolean} - * True if import should be disabled, or false if cancellation - * should be allowed. + * True if import should be disabled, or false if import should be + * allowed. */ $scope.importDisabled = () => @@ -471,7 +493,8 @@ angular.module('import').controller('importConnectionsController', ['$scope', '$ $scope.processing; /** - * Cancel any in-progress upload, or clear any uploaded-but + * Cancel any in-progress upload, or clear any uploaded-but-errored-out + * batch. */ $scope.cancel = function() { diff --git a/guacamole/src/main/frontend/src/app/import/directives/connectionImportErrors.js b/guacamole/src/main/frontend/src/app/import/directives/connectionImportErrors.js new file mode 100644 index 000000000..dcf42695a --- /dev/null +++ b/guacamole/src/main/frontend/src/app/import/directives/connectionImportErrors.js @@ -0,0 +1,248 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/* global _ */ + +/** + * A directive that displays errors that occured during parsing of a connection + * import file, or errors that were returned from the API during the connection + * batch creation attempt. + */ +angular.module('import').directive('connectionImportErrors', [ + function connectionImportErrors() { + + const directive = { + restrict: 'E', + replace: true, + templateUrl: 'app/import/templates/connectionErrors.html', + scope: { + + /** + * The result of parsing the import file. Any errors in this file + * will be displayed to the user. + * + * @type ParseResult + */ + parseResult : '=', + + /** + * The error associated with an attempt to batch create the + * connections represented by the ParseResult, if the ParseResult + * had no errors. If the provided ParseResult has errors, no request + * should have been made, and any provided patch error will be + * ignored. + * + * @type Error + */ + patchFailure : '=', + + } + }; + + directive.controller = ['$scope', '$injector', + function connectionImportErrorsController($scope, $injector) { + + // Required types + const DisplayErrorList = $injector.get('DisplayErrorList'); + const ImportConnectionError = $injector.get('ImportConnectionError'); + const ParseError = $injector.get('ParseError'); + const SortOrder = $injector.get('SortOrder'); + + // Required services + const $q = $injector.get('$q'); + const $translate = $injector.get('$translate'); + + // There are errors to display if the parse result generated errors, or + // if the patch request failed + $scope.hasErrors = () => + !!_.get($scope, 'parseResult.hasErrors') || !!$scope.patchFailure; + + /** + * All connections with their associated errors for display. These may + * be either parsing failures, or errors returned from the API. Both + * error types will be adapted to a common display format, though the + * error types will never be mixed, because no REST request should ever + * be made if there are client-side parse errors. + * + * @type {ImportConnectionError[]} + */ + $scope.connectionErrors = []; + + /** + * SortOrder instance which maintains the sort order of the visible + * connection errors. + * + * @type SortOrder + */ + $scope.errorOrder = new SortOrder([ + 'rowNumber', + 'name', + 'protocol', + 'errors', + ]); + + /** + * Array of all connection error properties that are filterable. + * + * @type String[] + */ + $scope.filteredErrorProperties = [ + 'rowNumber', + 'name', + 'protocol', + 'errors', + ]; + + /** + * Generate a ImportConnectionError representing any errors associated + * with the row at the given index within the given parse result. + * + * @param {ParseResult} parseResult + * The result of parsing the connection import file. + * + * @param {Integer} index + * The current row within the import file, 0-indexed. + * + * @returns {ImportConnectionError} + * The connection error object associated with the given row in the + * given parse result. + */ + const generateConnectionError = (parseResult, index) => { + + // Get the patch associated with the current row + const patch = parseResult.patches[index]; + + // The value of a patch is just the Connection object + const connection = patch.value; + + return new ImportConnectionError({ + + // Add 1 to the index to get the position in the file + rowNumber: index + 1, + + // Basic connection information - name and protocol. + name: connection.name, + protocol: connection.protocol, + + // The group and parent identifiers, if any are set. Include + // both since these could be a potential source of conflict. + // TODO: Should we _really_ have both of these here? + group: connection.group, + parentIdentifier: connection.parentIdentifier, + + // Get the list of user and group identifiers from the parse + // result. There should one entry in each of these lists for + // each patch. + users: parseResult.users[index], + groups: parseResult.groups[index], + + // The human-readable error messages + errors: new DisplayErrorList( + [ ...(parseResult.errors[index] || []) ]) + }); + }; + + // If a new connection patch failure is seen, update the display list + $scope.$watch('patchFailure', async function patchFailureChanged(patchFailure) { + + const { parseResult } = $scope; + + // Do not attempt to process anything before the data has loaded + if (!patchFailure || !parseResult) + return; + + // Set up the list of connection errors based on the existing parse + // result, with error messages fetched from the patch failure + $scope.connectionErrors = parseResult.patches.map( + (patch, index) => { + + // Generate a connection error for display + const connectionError = generateConnectionError(parseResult, index); + + // Set the error from the PATCH request, if there is one + // TODO: These generally aren't translated from the backend - + // should we even bother trying to translate them? + const error = _.get(patchFailure, ['patches', index, 'error']); + if (error) + connectionError.errors = new DisplayErrorList([error]); + + return connectionError; + }); + }); + + // If a new parse result with errors is seen, update the display list + $scope.$watch('parseResult', async function parseResultChanged(parseResult) { + + // Do not process if there are no errors in the provided result + if (!parseResult || !parseResult.hasErrors) + return; + + // All promises from all translation requests. The scope will not be + // updated until all translations are ready. + const translationPromises = []; + + // The parse result should only be updated on a fresh file import; + // therefore it should be safe to skip checking the patch errors + // entirely - if set, they will be from the previous file and no + // longer relevant. + + // Set up the list of connection errors based on the updated parse + // result + const connectionErrors = parseResult.patches.map( + (patch, index) => { + + // Generate a connection error for display + const connectionError = generateConnectionError(parseResult, index); + + // Go through the errors and check if any are translateable + connectionError.errors.getArray().forEach( + (error, errorIndex) => { + + // If this error is a ParseError, it can be translated. + // NOTE: Generally one would translate error messages in the + // template, but in this case, the connection errors need to + // be raw strings in order to enable sorting and filtering. + if (error instanceof ParseError) + + // Fetch the translation and update it when it's ready + translationPromises.push($translate( + error.key, error.variables) + .then(translatedError => { + connectionError.errors.getArray()[errorIndex] = translatedError; + })); + + }); + + return connectionError; + + }); + + // Once all the translations have been completed, update the + // connectionErrors all in one go, to ensure no excessive reloading + $q.all(translationPromises).then(() => { + $scope.connectionErrors = connectionErrors; + }); + + }); + + }]; + + return directive; + +}]); \ No newline at end of file diff --git a/guacamole/src/main/frontend/src/app/import/directives/connectionImportFileUpload.js b/guacamole/src/main/frontend/src/app/import/directives/connectionImportFileUpload.js index 4e2c76b24..196e8f48f 100644 --- a/guacamole/src/main/frontend/src/app/import/directives/connectionImportFileUpload.js +++ b/guacamole/src/main/frontend/src/app/import/directives/connectionImportFileUpload.js @@ -170,7 +170,7 @@ angular.module('import').directive('connectionImportFileUpload', [ // If the provided file is not one of the supported types, // display an error and abort processing - setError('CONNECTION_IMPORT.ERROR_INVALID_FILE_TYPE', + setError('IMPORT.ERROR_INVALID_FILE_TYPE', { TYPE: mimeType }); return; } @@ -204,7 +204,7 @@ angular.module('import').directive('connectionImportFileUpload', [ // If more than one file was provided, print an error explaining // that only a single file is allowed and abort processing - setError('CONNECTION_IMPORT.ERROR_FILE_SINGLE_ONLY'); + setError('IMPORT.ERROR_FILE_SINGLE_ONLY'); return; } diff --git a/guacamole/src/main/frontend/src/app/import/importModule.js b/guacamole/src/main/frontend/src/app/import/importModule.js index 6480d62fc..46e5fa157 100644 --- a/guacamole/src/main/frontend/src/app/import/importModule.js +++ b/guacamole/src/main/frontend/src/app/import/importModule.js @@ -21,4 +21,4 @@ * The module for code supporting importing user-supplied files. Currently, only * connection import is supported. */ -angular.module('import', ['rest']); +angular.module('import', ['rest', 'list']); diff --git a/guacamole/src/main/frontend/src/app/import/services/connectionCSVService.js b/guacamole/src/main/frontend/src/app/import/services/connectionCSVService.js index 492b9388b..389749555 100644 --- a/guacamole/src/main/frontend/src/app/import/services/connectionCSVService.js +++ b/guacamole/src/main/frontend/src/app/import/services/connectionCSVService.js @@ -238,7 +238,7 @@ angular.module('import').factory('connectionCSVService', deferred.reject(new ParseError({ message: 'Duplicate CSV Header: ' + header, translatableMessage: new TranslatableMessage({ - key: 'CONNECTION_IMPORT.ERROR_DUPLICATE_CSV_HEADER', + key: 'IMPORT.ERROR_DUPLICATE_CSV_HEADER', variables: { HEADER: header } }) })); @@ -342,7 +342,7 @@ angular.module('import').factory('connectionCSVService', if (isAttribute && isParameter) throw new ParseError({ message: 'Ambiguous CSV Header: ' + header, - key: 'CONNECTION_IMPORT.ERROR_AMBIGUOUS_CSV_HEADER', + key: 'IMPORT.ERROR_AMBIGUOUS_CSV_HEADER', variables: { HEADER: header } }); @@ -350,7 +350,7 @@ angular.module('import').factory('connectionCSVService', else if (!isAttribute && !isParameter) throw new ParseError({ message: 'Invalid CSV Header: ' + header, - key: 'CONNECTION_IMPORT.ERROR_INVALID_CSV_HEADER', + key: 'IMPORT.ERROR_INVALID_CSV_HEADER', variables: { HEADER: header } }); @@ -372,21 +372,21 @@ angular.module('import').factory('connectionCSVService', if (!nameGetter) return deferred.reject(new ParseError({ message: 'The connection name must be provided', - key: 'CONNECTION_IMPORT.ERROR_REQUIRED_NAME' + key: 'IMPORT.ERROR_REQUIRED_NAME' })); // Fail if the protocol wasn't provided if (!protocolGetter) return deferred.reject(new ParseError({ message: 'The connection protocol must be provided', - key: 'CONNECTION_IMPORT.ERROR_REQUIRED_PROTOCOL' + key: 'IMPORT.ERROR_REQUIRED_PROTOCOL' })); // If both are specified, the parent group is ambigious if (parentIdentifierGetter && groupGetter) throw new ParseError({ message: 'Only one of group or parentIdentifier can be set', - key: 'CONNECTION_IMPORT.ERROR_AMBIGUOUS_PARENT_GROUP' + key: 'IMPORT.ERROR_AMBIGUOUS_PARENT_GROUP' }); // The function to transform a CSV row into a connection object diff --git a/guacamole/src/main/frontend/src/app/import/services/connectionParseService.js b/guacamole/src/main/frontend/src/app/import/services/connectionParseService.js index cf15a374e..dd9f49593 100644 --- a/guacamole/src/main/frontend/src/app/import/services/connectionParseService.js +++ b/guacamole/src/main/frontend/src/app/import/services/connectionParseService.js @@ -60,7 +60,7 @@ angular.module('import').factory('connectionParseService', if (!(parsedData instanceof Array)) return new ParseError({ message: 'Import data must be a list of connections', - key: 'CONNECTION_IMPORT.ERROR_ARRAY_REQUIRED' + key: 'IMPORT.ERROR_ARRAY_REQUIRED' }); // Make sure that the connection list is not empty - contains at least @@ -68,7 +68,7 @@ angular.module('import').factory('connectionParseService', if (!parsedData.length) return new ParseError({ message: 'The provided file is empty', - key: 'CONNECTION_IMPORT.ERROR_EMPTY_FILE' + key: 'IMPORT.ERROR_EMPTY_FILE' }); } @@ -148,7 +148,7 @@ angular.module('import').factory('connectionParseService', if (connection.parentIdentifier) throw new ParseError({ message: 'Only one of group or parentIdentifier can be set', - key: 'CONNECTION_IMPORT.ERROR_AMBIGUOUS_PARENT_GROUP' + key: 'IMPORT.ERROR_AMBIGUOUS_PARENT_GROUP' }); // Look up the parent identifier for the specified group path @@ -158,7 +158,7 @@ angular.module('import').factory('connectionParseService', if (!identifier) throw new ParseError({ message: 'No group found named: ' + connection.group, - key: 'CONNECTION_IMPORT.ERROR_INVALID_GROUP', + key: 'IMPORT.ERROR_INVALID_GROUP', variables: { GROUP: connection.group } }); diff --git a/guacamole/src/main/frontend/src/app/import/styles/import.css b/guacamole/src/main/frontend/src/app/import/styles/import.css index 5addb1933..8f038e4d1 100644 --- a/guacamole/src/main/frontend/src/app/import/styles/import.css +++ b/guacamole/src/main/frontend/src/app/import/styles/import.css @@ -27,5 +27,18 @@ display: flex; gap: 10px; justify-content: center; - -} \ No newline at end of file + +} + + +.import .errors table { + width: 100%; +} + +.import .errors .error-message { + color: red; +} + +.import .errors .error-message ul { + margin: 0px; +} diff --git a/guacamole/src/main/frontend/src/app/import/templates/connectionErrors.html b/guacamole/src/main/frontend/src/app/import/templates/connectionErrors.html new file mode 100644 index 000000000..eee23c260 --- /dev/null +++ b/guacamole/src/main/frontend/src/app/import/templates/connectionErrors.html @@ -0,0 +1,45 @@ +
+ {{'IMPORT.TABLE_HEADER_ROW_NUMBER' | translate}} + | ++ {{'IMPORT.TABLE_HEADER_NAME' | translate}} + | ++ {{'IMPORT.TABLE_HEADER_PROTOCOL' | translate}} + | ++ {{'IMPORT.TABLE_HEADER_ERRORS' | translate}} + | +
---|---|---|---|
{{error.rowNumber}} | +{{error.name}} | +{{error.protocol}} | + +
{{'CONNECTION_IMPORT.HELP_FILE_TYPE_DESCRIPTION' | translate}}
+{{'IMPORT.HELP_FILE_TYPE_DESCRIPTION' | translate}}
-{{'CONNECTION_IMPORT.HELP_CSV_DESCRIPTION' | translate}}
-{{'CONNECTION_IMPORT.HELP_CSV_MORE_DETAILS' | translate}}
+{{'IMPORT.HELP_CSV_DESCRIPTION' | translate}}
+{{'IMPORT.HELP_CSV_MORE_DETAILS' | translate}}
name,protocol,hostname,group,users,groups,guacd-encryption (attribute) conn1,vnc,conn1.web.com,ROOT,guac user 1;guac user 2,Connection 1 Users,none conn2,rdp,conn2.web.com,ROOT/Parent Group,guac user 1,,ssl conn3,ssh,conn3.web.com,ROOT/Parent Group/Child Group,guac user 2;guac user 3,, conn4,kubernetes,,,,,-
{{'CONNECTION_IMPORT.HELP_JSON_DESCRIPTION' | translate}}
-{{'CONNECTION_IMPORT.HELP_JSON_MORE_DETAILS' | translate}}
+{{'IMPORT.HELP_JSON_DESCRIPTION' | translate}}
+{{'IMPORT.HELP_JSON_MORE_DETAILS' | translate}}
[ { "name": "conn1", @@ -51,8 +51,8 @@ conn4,kubernetes,,,,,} ] -
{{'CONNECTION_IMPORT.HELP_YAML_DESCRIPTION' | translate}}
+{{'IMPORT.HELP_YAML_DESCRIPTION' | translate}}
--- - name: conn1 protocol: vnc @@ -87,7 +87,7 @@ conn4,kubernetes,,,,,protocol: kubernetes