[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #840: GUACAMOLE-926: Add options to replace connections / permissions.

2023-04-21 Thread via GitHub


mike-jumper commented on code in PR #840:
URL: https://github.com/apache/guacamole-client/pull/840#discussion_r1174044457


##
guacamole/src/main/frontend/src/app/import/templates/connectionImport.html:
##
@@ -38,6 +38,28 @@ {{'IMPORT.SECTION_HEADER_CONNECTION_IMPORT' | 
translate}}
 
 
 
+
+
+
+
+{{'IMPORT.FIELD_HEADER_EXISTING_CONNECTION_MODE' | 
translate}}
+
+

Review Comment:
   This should be `ng-attr-title` to avoid there being a point where the 
attribute is populated with the raw Angular expression. For example:
   
   
https://github.com/apache/guacamole-client/blob/165bd413c0ab9c55b0c0e3614fcb829ef5f8ca95/guacamole/src/main/frontend/src/app/player/templates/player.html#L17
   
   Same below with the other `title` attribute.



##
guacamole/src/main/frontend/src/translations/en.json:
##
@@ -200,38 +200,50 @@
 "DIALOG_HEADER_ERROR"   : "@:APP.DIALOG_HEADER_ERROR",
 "DIALOG_HEADER_SUCCESS" : "Success",
 
-"ERROR_AMBIGUOUS_CSV_HEADER" : "Ambiguous CSV Header \"{HEADER}\" 
could be either a connection attribute or parameter",
-"ERROR_AMBIGUOUS_PARENT_GROUP"   : "Both group and parentIdentifier 
may be not specified at the same time",
-"ERROR_ARRAY_REQUIRED"   : "The provided file must contain a 
list of connections",
-"ERROR_DUPLICATE_CSV_HEADER" : "Duplicate CSV Header: {HEADER}",
-"ERROR_EMPTY_FILE"   : "The provided file is empty",
-"ERROR_INVALID_CSV_HEADER"   : "Invalid CSV Header \"{HEADER}\" is 
neither an attribute or parameter",
-"ERROR_INVALID_MIME_TYPE": "Unsupported file type: \"{TYPE}\"",
-"ERROR_DETECTED_INVALID_TYPE": "Unsupported file type. Please make 
sure the file is valid CSV, JSON, or YAML.",
-"ERROR_INVALID_GROUP": "No group matching \"{GROUP}\" 
found",
-"ERROR_INVALID_GROUP_IDENTIFIER" : "No connection group with 
identifier \"{IDENTIFIER}\" found",
-"ERROR_NO_FILE_SUPPLIED" : "Please select a file to import",
-"ERROR_PARSE_FAILURE_CSV": "Please make sure your file is 
valid CSV. Parsing failed with error \"{ERROR}\". ",
-"ERROR_PARSE_FAILURE_JSON"   : "Please make sure your file is 
valid JSON. Parsing failed with error \"{ERROR}\". ",
-"ERROR_PARSE_FAILURE_YAML"   : "Please make sure your file is 
valid YAML. Parsing failed with error \"{ERROR}\". ",
-"ERROR_REQUIRED_NAME": "No connection name found in the 
provided file",
-"ERROR_REQUIRED_PROTOCOL": "No connection protocol found in 
the provided file",
+"ERROR_AMBIGUOUS_CSV_HEADER" : "Ambiguous CSV Header 
\"{HEADER}\" could be either a connection attribute or parameter",
+"ERROR_AMBIGUOUS_PARENT_GROUP"   : "Both group and 
parentIdentifier may be not specified at the same time",
+"ERROR_ARRAY_REQUIRED"   : "The provided file must contain 
a list of connections",
+"ERROR_DETECTED_INVALID_TYPE": "Unsupported file type. Please 
make sure the file is valid CSV, JSON, or YAML.",
+"ERROR_DUPLICATE_CONNECTION_IN_FILE" : "Duplicated connection 
\"{NAME}\" at \"{PATH}\" in import file",
+"ERROR_DUPLICATE_CSV_HEADER" : "Duplicate CSV Header: 
{HEADER}",
+"ERROR_EMPTY_FILE"   : "The provided file is empty",
+"ERROR_INVALID_CSV_HEADER"   : "Invalid CSV Header 
\"{HEADER}\" is neither an attribute or parameter",
+"ERROR_INVALID_MIME_TYPE": "Unsupported file type: 
\"{TYPE}\"",
+"ERROR_INVALID_GROUP": "No group matching \"{GROUP}\" 
found",
+"ERROR_INVALID_GROUP_IDENTIFIER" : "No connection group with 
identifier \"{IDENTIFIER}\" found",
+"ERROR_NO_FILE_SUPPLIED" : "Please select a file to 
import",
+"ERROR_PARSE_FAILURE_CSV": "Please make sure your file is 
valid CSV. Parsing failed with error \"{ERROR}\". ",
+"ERROR_PARSE_FAILURE_JSON"   : "Please make sure your file is 
valid JSON. Parsing failed with error \"{ERROR}\". ",
+"ERROR_PARSE_FAILURE_YAML"   : "Please make sure your file is 
valid YAML. Parsing failed with error \"{ERROR}\". ",
+"ERROR_REJECT_UPDATE_CONNECTION" : "Connection \"{NAME}\" already 
exists at \"{PATH}\"",
+"ERROR_REQUIRED_NAME": "No connection name found in 
the provided file",
+"ERROR_REQUIRED_PROTOCOL": "No connection protocol found 
in the provided file",
 
 "FIELD_PLACEHOLDER_FILTER" : "@:APP.FIELD_PLACEHOLDER_FILTER",
 
+"FIELD_HEADER_EXISTING_CONNECTION_MODE" : "Replace/Update existing 
connections",
+"FIELD_HEADER_EXISTING_PERMISSION_MODE" : "Reset permissions",
+
+"FIELD_OPTION_DUPLI

[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #840: GUACAMOLE-926: Add options to replace connections / permissions.

2023-04-21 Thread via GitHub


mike-jumper commented on code in PR #840:
URL: https://github.com/apache/guacamole-client/pull/840#discussion_r1174010688


##
guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryResource.java:
##
@@ -590,7 +591,56 @@ public void executeOperation(boolean atomic, 
Directory directory)
 
 }
 
-// Append each identifier to the list, to be removed 
atomically
+else if (op == APIPatch.Operation.replace) {
+
+// The identifier of the object to be replaced
+String identifier = path.substring(1);
+
+InternalType original = null;
+
+try {
+
+// Fetch the object to be updated
+original = directory.get(identifier);

Review Comment:
   Yeah, I agree. There'd otherwise be no context for failures in audit logs.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #840: GUACAMOLE-926: Add options to replace connections / permissions.

2023-04-21 Thread via GitHub


mike-jumper commented on code in PR #840:
URL: https://github.com/apache/guacamole-client/pull/840#discussion_r1173987909


##
guacamole/src/main/java/org/apache/guacamole/rest/directory/DirectoryResource.java:
##
@@ -590,7 +591,56 @@ public void executeOperation(boolean atomic, 
Directory directory)
 
 }
 
-// Append each identifier to the list, to be removed 
atomically
+else if (op == APIPatch.Operation.replace) {
+
+// The identifier of the object to be replaced
+String identifier = path.substring(1);
+
+InternalType original = null;
+
+try {
+
+// Fetch the object to be updated
+original = directory.get(identifier);

Review Comment:
   If a user attempts to update a single connection via the non-batch REST API, 
but that connection does not exist, they'll currently get a `GET` failure event 
but no `UPDATE` failure event (the attempt to update doesn't occur because the 
retrieval failed). Do you think these changes should match that behavior, or 
that the REST API changes that are part of GUACAMOLE-1224 should be modified to 
ensure both are fired?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



[GitHub] [guacamole-client] mike-jumper commented on a diff in pull request #840: GUACAMOLE-926: Add options to replace connections / permissions.

2023-04-19 Thread via GitHub


mike-jumper commented on code in PR #840:
URL: https://github.com/apache/guacamole-client/pull/840#discussion_r1171954389


##
guacamole/src/main/frontend/src/images/question.svg:
##
@@ -0,0 +1,64 @@
+
+http://www.inkscape.org/namespaces/inkscape";
+   xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd";
+   xmlns="http://www.w3.org/2000/svg";
+   xmlns:svg="http://www.w3.org/2000/svg";>
+  
+  
+  
+
+  
+
+
+?

Review Comment:
   Embedding the "?" as actual text will result in this SVG being dependent on 
the fonts installed on the user's system. It should instead be converted to a 
path.
   
   I'd also recommend running it through `svgo` to trim down the size: 
https://www.npmjs.com/package/svgo



##
guacamole/src/main/frontend/src/translations/en.json:
##
@@ -200,38 +200,52 @@
 "DIALOG_HEADER_ERROR"   : "@:APP.DIALOG_HEADER_ERROR",
 "DIALOG_HEADER_SUCCESS" : "Success",
 
-"ERROR_AMBIGUOUS_CSV_HEADER" : "Ambiguous CSV Header \"{HEADER}\" 
could be either a connection attribute or parameter",
-"ERROR_AMBIGUOUS_PARENT_GROUP"   : "Both group and parentIdentifier 
may be not specified at the same time",
-"ERROR_ARRAY_REQUIRED"   : "The provided file must contain a 
list of connections",
-"ERROR_DUPLICATE_CSV_HEADER" : "Duplicate CSV Header: {HEADER}",
-"ERROR_EMPTY_FILE"   : "The provided file is empty",
-"ERROR_INVALID_CSV_HEADER"   : "Invalid CSV Header \"{HEADER}\" is 
neither an attribute or parameter",
-"ERROR_INVALID_MIME_TYPE": "Unsupported file type: \"{TYPE}\"",
-"ERROR_DETECTED_INVALID_TYPE": "Unsupported file type. Please make 
sure the file is valid CSV, JSON, or YAML.",
-"ERROR_INVALID_GROUP": "No group matching \"{GROUP}\" 
found",
-"ERROR_INVALID_GROUP_IDENTIFIER" : "No connection group with 
identifier \"{IDENTIFIER}\" found",
-"ERROR_NO_FILE_SUPPLIED" : "Please select a file to import",
-"ERROR_PARSE_FAILURE_CSV": "Please make sure your file is 
valid CSV. Parsing failed with error \"{ERROR}\". ",
-"ERROR_PARSE_FAILURE_JSON"   : "Please make sure your file is 
valid JSON. Parsing failed with error \"{ERROR}\". ",
-"ERROR_PARSE_FAILURE_YAML"   : "Please make sure your file is 
valid YAML. Parsing failed with error \"{ERROR}\". ",
-"ERROR_REQUIRED_NAME": "No connection name found in the 
provided file",
-"ERROR_REQUIRED_PROTOCOL": "No connection protocol found in 
the provided file",
+"ERROR_AMBIGUOUS_CSV_HEADER" : "Ambiguous CSV Header 
\"{HEADER}\" could be either a connection attribute or parameter",
+"ERROR_AMBIGUOUS_PARENT_GROUP"   : "Both group and 
parentIdentifier may be not specified at the same time",
+"ERROR_ARRAY_REQUIRED"   : "The provided file must contain 
a list of connections",
+"ERROR_DETECTED_INVALID_TYPE": "Unsupported file type. Please 
make sure the file is valid CSV, JSON, or YAML.",
+"ERROR_DUPLICATE_CONNECTION_IN_FILE" : "Duplicate connection in file 
at \"{PATH}\"",
+"ERROR_DUPLICATE_CSV_HEADER" : "Duplicate CSV Header: 
{HEADER}",
+"ERROR_EMPTY_FILE"   : "The provided file is empty",
+"ERROR_INVALID_CSV_HEADER"   : "Invalid CSV Header 
\"{HEADER}\" is neither an attribute or parameter",
+"ERROR_INVALID_MIME_TYPE": "Unsupported file type: 
\"{TYPE}\"",
+"ERROR_INVALID_GROUP": "No group matching \"{GROUP}\" 
found",
+"ERROR_INVALID_GROUP_IDENTIFIER" : "No connection group with 
identifier \"{IDENTIFIER}\" found",
+"ERROR_NO_FILE_SUPPLIED" : "Please select a file to 
import",
+"ERROR_PARSE_FAILURE_CSV": "Please make sure your file is 
valid CSV. Parsing failed with error \"{ERROR}\". ",
+"ERROR_PARSE_FAILURE_JSON"   : "Please make sure your file is 
valid JSON. Parsing failed with error \"{ERROR}\". ",
+"ERROR_PARSE_FAILURE_YAML"   : "Please make sure your file is 
valid YAML. Parsing failed with error \"{ERROR}\". ",
+"ERROR_REJECT_UPDATE_CONNECTION" : "Disallowed update to existing 
connection at \"{PATH}\"",
+"ERROR_REQUIRED_NAME": "No connection name found in 
the provided file",
+"ERROR_REQUIRED_PROTOCOL": "No connection protocol found 
in the provided file",
 
 "FIELD_PLACEHOLDER_FILTER" : "@:APP.FIELD_PLACEHOLDER_FILTER",
 
+"FIELD_HEADER_REPLACE_CONNECTIONS" : "Replace/Update existing 
connections",
+"FIELD_HEADER_REPLACE_PERMISSIONS" : "Reset permissions",
+
+"FIELD_OPTION_DUPLICATE_REPLACE" : "Replace duplicates",
+"FIELD_OPTION_DUPLICATE_IGNORE"  : "Ignore duplicates",
+"FIELD_OPTION_DUPLICATE_E