From 066df95d0c2958ba199549df360856af8441074d Mon Sep 17 00:00:00 2001 From: anhefti Date: Thu, 16 Jul 2020 16:16:30 +0200 Subject: [PATCH] fixed testing on LMS Setup and display messages HTML formatting --- .../ethz/seb/sebserver/gbl/api/APIMessage.java | 2 +- .../ch/ethz/seb/sebserver/gbl/util/Utils.java | 6 ++++++ .../seb/sebserver/gui/content/LmsSetupForm.java | 17 ++++++++++++++--- .../gui/content/action/ActionDefinition.java | 9 +++++++-- .../gui/service/page/impl/PageAction.java | 7 +++++++ .../gui/service/page/impl/PageContextImpl.java | 2 +- .../gui/service/page/impl/PageServiceImpl.java | 2 ++ .../ethz/seb/sebserver/gui/widget/Message.java | 1 - .../servicelayer/dao/impl/LmsSetupDAOImpl.java | 12 ++++++++---- .../weblayer/api/APIExceptionHandler.java | 13 +++++++++++++ .../weblayer/api/EntityController.java | 1 - .../resources/config/application.properties | 2 +- src/main/resources/messages.properties | 4 ++-- .../seb/sebserver/gbl/api/APIMessageTest.java | 2 +- 14 files changed, 63 insertions(+), 17 deletions(-) diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/api/APIMessage.java b/src/main/java/ch/ethz/seb/sebserver/gbl/api/APIMessage.java index dbf0c0f5..85410d7f 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/api/APIMessage.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/api/APIMessage.java @@ -200,7 +200,7 @@ public class APIMessage implements Serializable { public static String toHTML(final String errorMessage, final List messages) { final StringBuilder builder = new StringBuilder(); - builder.append("Failure: ").append("

").append(errorMessage).append("

"); + builder.append("Failure:").append("

").append(errorMessage).append("

"); builder.append("Detail Messages:

"); return buildHTML(messages, builder); } diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java b/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java index a33aac64..3ebc518f 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/util/Utils.java @@ -330,6 +330,12 @@ public final class Utils { : null; } + public static String formatHTMLLinesForceEscaped(final String message) { + return (message != null) + ? message.replace("\n", "
").replace("\\n", "
") + : null; + } + public static String formatLineBreaks(final String text) { if (text == null) { return null; diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/LmsSetupForm.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/LmsSetupForm.java index 9845561d..c22a3604 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/LmsSetupForm.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/LmsSetupForm.java @@ -305,6 +305,11 @@ public class LmsSetupForm implements TemplateComposer { .withEntityKey(entityKey) .publishIf(() -> modifyGrant && readonly && institutionActive) + .newAction(ActionDefinition.LMS_SETUP_TEST) + .withEntityKey(entityKey) + .withExec(action -> LmsSetupForm.testLmsSetup(action, formHandle, restService)) + .publishIf(() -> readonly) + .newAction(ActionDefinition.LMS_SETUP_DEACTIVATE) .withEntityKey(entityKey) .withSimpleRestCall(restService, DeactivateLmsSetup.class) @@ -322,6 +327,12 @@ public class LmsSetupForm implements TemplateComposer { .ignoreMoveAwayFromEdit() .publishIf(() -> !readonly) + .newAction(ActionDefinition.LMS_SETUP_TEST_EDIT) + .withEntityKey(entityKey) + .withExec(action -> this.testAdHoc(action, formHandle)) + .ignoreMoveAwayFromEdit() + .publishIf(() -> !readonly && !isNew.getAsBoolean()) + .newAction(ActionDefinition.LMS_SETUP_SAVE_AND_ACTIVATE) .withEntityKey(entityKey) .withExec(action -> { @@ -440,12 +451,12 @@ public class LmsSetupForm implements TemplateComposer { case TOKEN_REQUEST: { throw new PageMessageException(new LocTextKey( "sebserver.lmssetup.action.test.tokenRequestError", - error.message)); + Utils.formatHTMLLinesForceEscaped(Utils.escapeHTML_XML_EcmaScript(error.message)))); } case QUIZ_ACCESS_API_REQUEST: { throw new PageMessageException(new LocTextKey( "sebserver.lmssetup.action.test.quizRequestError", - error.message)); + Utils.formatHTMLLinesForceEscaped(Utils.escapeHTML_XML_EcmaScript(error.message)))); } case QUIZ_RESTRICTION_API_REQUEST: { // NOTE: quiz restriction is not mandatory for functional LmsSetup @@ -455,7 +466,7 @@ public class LmsSetupForm implements TemplateComposer { default: { throw new PageMessageException(new LocTextKey( "sebserver.lmssetup.action.test.unknownError", - error.message)); + Utils.formatHTMLLinesForceEscaped(Utils.escapeHTML_XML_EcmaScript(error.message)))); } } }); diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/content/action/ActionDefinition.java b/src/main/java/ch/ethz/seb/sebserver/gui/content/action/ActionDefinition.java index bb34c320..ad20c76a 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/content/action/ActionDefinition.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/content/action/ActionDefinition.java @@ -165,11 +165,16 @@ public enum ActionDefinition { ImageIcon.EDIT, PageStateDefinitionImpl.LMS_SETUP_EDIT, ActionCategory.FORM), - LMS_SETUP_SAVE_AND_TEST( - new LocTextKey("sebserver.lmssetup.action.savetest"), + LMS_SETUP_TEST( + new LocTextKey("sebserver.lmssetup.action.test"), ImageIcon.TEST, PageStateDefinitionImpl.LMS_SETUP_VIEW, ActionCategory.FORM), + LMS_SETUP_TEST_EDIT( + new LocTextKey("sebserver.lmssetup.action.test"), + ImageIcon.TEST, + PageStateDefinitionImpl.LMS_SETUP_EDIT, + ActionCategory.FORM), LMS_SETUP_TEST_AND_SAVE( new LocTextKey("sebserver.lmssetup.action.testsave"), ImageIcon.TEST, diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageAction.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageAction.java index e8af5979..d4d02337 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageAction.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageAction.java @@ -198,6 +198,13 @@ public final class PageAction { PageAction.this.getName(), e.getMessage(), Utils.getErrorCauseMessage(e)); + if (e.getCause() instanceof RestCallError) { + final RestCallError cause = (RestCallError) e.getCause(); + PageAction.this.pageContext.notifyError( + PageContext.UNEXPECTED_ERROR_KEY, + cause); + return Result.ofError(cause); + } return Result.ofError(e); } catch (final Exception e) { log.error("Failed to execute action: {} | error: {} | cause: {}", diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageContextImpl.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageContextImpl.java index e481d290..e390b1bd 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageContextImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageContextImpl.java @@ -258,7 +258,6 @@ public class PageContextImpl implements PageContext { this.i18nSupport.getText(message), SWT.NONE, this.i18nSupport); - messageBox.setMarkupEnabled(true); messageBox.open(null); } @@ -303,6 +302,7 @@ public class PageContextImpl implements PageContext { Utils.formatHTMLLines(errorMessage + "

Cause: " + error.getMessage()), SWT.ERROR, this.i18nSupport); + messageBox.setMarkupEnabled(true); messageBox.open(null); } diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageServiceImpl.java b/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageServiceImpl.java index c4557726..e91523b8 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageServiceImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/service/page/impl/PageServiceImpl.java @@ -340,6 +340,8 @@ public class PageServiceImpl implements PageService { } catch (final Exception e) { log.error("Failed to set current PageState: ", e); } + } else { + } callback.accept(result); }); diff --git a/src/main/java/ch/ethz/seb/sebserver/gui/widget/Message.java b/src/main/java/ch/ethz/seb/sebserver/gui/widget/Message.java index 1d3dfb53..df09b0f0 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gui/widget/Message.java +++ b/src/main/java/ch/ethz/seb/sebserver/gui/widget/Message.java @@ -37,7 +37,6 @@ public final class Message extends MessageBox { super(parent, type); super.setText(title); super.setMessage(message); - super.setMarkupEnabled(true); this.i18nSupport = i18nSupport; } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/LmsSetupDAOImpl.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/LmsSetupDAOImpl.java index 5a0e59b5..291977c7 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/LmsSetupDAOImpl.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/dao/impl/LmsSetupDAOImpl.java @@ -26,10 +26,10 @@ import org.springframework.transaction.annotation.Transactional; import ch.ethz.seb.sebserver.gbl.api.APIMessage; import ch.ethz.seb.sebserver.gbl.api.APIMessage.APIMessageException; +import ch.ethz.seb.sebserver.gbl.api.EntityType; import ch.ethz.seb.sebserver.gbl.client.ClientCredentialService; import ch.ethz.seb.sebserver.gbl.client.ClientCredentials; import ch.ethz.seb.sebserver.gbl.client.ProxyData; -import ch.ethz.seb.sebserver.gbl.api.EntityType; import ch.ethz.seb.sebserver.gbl.model.Domain; import ch.ethz.seb.sebserver.gbl.model.EntityKey; import ch.ethz.seb.sebserver.gbl.model.institution.LmsSetup; @@ -137,7 +137,7 @@ public class LmsSetupDAOImpl implements LmsSetupDAO { checkUniqueName(lmsSetup); - LmsSetupRecord savedRecord = recordById(lmsSetup.id) + final LmsSetupRecord savedRecord = recordById(lmsSetup.id) .getOrThrow(); final ClientCredentials lmsCredentials = createAPIClientCredentials(lmsSetup); @@ -149,8 +149,12 @@ public class LmsSetupDAOImpl implements LmsSetupDAO { (lmsSetup.lmsType != null) ? lmsSetup.lmsType.name() : null, lmsSetup.lmsApiUrl, lmsCredentials.clientIdAsString(), - lmsCredentials.secretAsString(), - lmsCredentials.accessTokenAsString(), + (lmsCredentials.hasSecret()) + ? lmsCredentials.secretAsString() + : savedRecord.getLmsClientsecret(), + (lmsCredentials.hasAccessToken()) + ? lmsCredentials.accessTokenAsString() + : savedRecord.getLmsRestApiToken(), lmsSetup.getProxyHost(), lmsSetup.getProxyPort(), proxyCredentials.clientIdAsString(), diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/APIExceptionHandler.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/APIExceptionHandler.java index eafc0855..dcf3da84 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/APIExceptionHandler.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/APIExceptionHandler.java @@ -86,6 +86,19 @@ public class APIExceptionHandler extends ResponseEntityExceptionHandler { Utils.createJsonContentHeader(), HttpStatus.BAD_REQUEST); } +// +// @ExceptionHandler(RuntimeException.class) +// public ResponseEntity handleRuntimeException( +// final RuntimeException ex, +// final WebRequest request) { +// +// log.error("Unexpected generic error catched at the API endpoint: ", ex); +// final List errors = Arrays.asList(APIMessage.ErrorMessage.GENERIC.of(ex.getMessage())); +// return new ResponseEntity<>( +// errors, +// Utils.createJsonContentHeader(), +// HttpStatus.INTERNAL_SERVER_ERROR); +// } @ExceptionHandler(OnlyMessageLogExceptionWrapper.class) public ResponseEntity onlyMessageLogExceptionWrapper( diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/EntityController.java b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/EntityController.java index 8ff49ae3..b23b7926 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/EntityController.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/weblayer/api/EntityController.java @@ -303,7 +303,6 @@ public abstract class EntityController { consumes = MediaType.APPLICATION_JSON_UTF8_VALUE, produces = MediaType.APPLICATION_JSON_UTF8_VALUE) public T savePut(@Valid @RequestBody final T modifyData) { - return this.checkModifyAccess(modifyData) .flatMap(this::validForSave) .flatMap(this.entityDAO::save) diff --git a/src/main/resources/config/application.properties b/src/main/resources/config/application.properties index dc986cfb..be6a3135 100644 --- a/src/main/resources/config/application.properties +++ b/src/main/resources/config/application.properties @@ -1,6 +1,6 @@ spring.application.name=SEB Server spring.profiles.active=ws,gui,dev -sebserver.version=1.0.1 +sebserver.version=@sebserver-version@ ########################################################## ### Global Server Settings diff --git a/src/main/resources/messages.properties b/src/main/resources/messages.properties index d730d86d..1e9e2890 100644 --- a/src/main/resources/messages.properties +++ b/src/main/resources/messages.properties @@ -283,10 +283,10 @@ sebserver.lmssetup.action.new=Add LMS Setup sebserver.lmssetup.action.list.view=View LMS Setup sebserver.lmssetup.action.list.modify=Edit LMS Setup sebserver.lmssetup.action.modify=Edit -sebserver.lmssetup.action.savetest=Test And Save +sebserver.lmssetup.action.test=Test LMS Connection sebserver.lmssetup.action.testsave=Test And Save sebserver.lmssetup.action.test.ok=Successfully connected to the course API -sebserver.lmssetup.action.test.tokenRequestError=The API access was denied: {0}
Please check the LMS connection details. +sebserver.lmssetup.action.test.tokenRequestError=The API access was denied:
{0}

Please check the LMS connection details. sebserver.lmssetup.action.test.quizRequestError=Unable to request courses or exams from the course API of the LMS. {0} sebserver.lmssetup.action.test.quizRestrictionError=Unable to access course restriction API of the LMS. {0} sebserver.lmssetup.action.test.missingParameter=There is one or more missing connection parameter.
Please check the connection parameter for this LMS Setup diff --git a/src/test/java/ch/ethz/seb/sebserver/gbl/api/APIMessageTest.java b/src/test/java/ch/ethz/seb/sebserver/gbl/api/APIMessageTest.java index 679e78c2..b0350701 100644 --- a/src/test/java/ch/ethz/seb/sebserver/gbl/api/APIMessageTest.java +++ b/src/test/java/ch/ethz/seb/sebserver/gbl/api/APIMessageTest.java @@ -68,7 +68,7 @@ public class APIMessageTest { final String html = APIMessage.toHTML("title message", messages); assertEquals( - "Failure:

title message

Detail Messages:

  code : 0
  system message : Generic error message
  details : --

  code : 1010
  system message : Illegal API request argument
  details : --

", + "Failure:

title message

Detail Messages:

  code : 0
  system message : Generic error message
  details : --

  code : 1010
  system message : Illegal API request argument
  details : --

", html); final String html2 = APIMessage.toHTML(messages);