Hello,
this is a proposed extension to commit 1ea830a in master:
Validation of a kickstart profile is currently done by downloading it from the
locally
running cobbler. In case of validation problems there is a DownloadException
thrown that
we can catch and work with.
Therefore in case of an error, it makes IMO not very much sense to point to the
'Kickstart File' tab, where the downloaded file contents should actually be
shown to the
user. The cobbler returns a status 500 in this case, so what we see on this
page is the
source code of the apache error page. Further, the download link is of course
not working.
The attached patch contains the following proposed changes to the code:
1. Change the error message text to point to the file contents on the 'Details'
tab
2. Hide the content on the 'Kickstart File' tab in case the download failed
Regards,
Johannes
--
SUSE LINUX Products GmbH, HRB 16746 (AG Nürnberg)
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer
>From 5e754d93de7475a88e38578faa0d32fce7581cdb Mon Sep 17 00:00:00 2001
From: Johannes Renner <jren...@suse.de>
Date: Tue, 6 Nov 2012 12:10:34 +0100
Subject: [PATCH] Improve kickstart profiles error handling
---
.../kickstart/KickstartFileDownloadAction.java | 12 +++++++--
.../frontend/strings/jsp/StringResource_en_US.xml | 2 +-
.../rhn/manager/kickstart/KickstartManager.java | 4 +--
.../kickstart/wizard/profile/advanced/download.jsp | 28 ++++++++++----------
4 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartFileDownloadAction.java b/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartFileDownloadAction.java
index 7efc1c4..fd8acea 100644
--- a/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartFileDownloadAction.java
+++ b/java/code/src/com/redhat/rhn/frontend/action/kickstart/KickstartFileDownloadAction.java
@@ -14,10 +14,13 @@
*/
package com.redhat.rhn.frontend.action.kickstart;
+import com.redhat.rhn.common.localization.LocalizationService;
import com.redhat.rhn.common.util.download.DownloadException;
import com.redhat.rhn.common.validator.ValidatorError;
+import com.redhat.rhn.common.validator.ValidatorException;
import com.redhat.rhn.domain.kickstart.KickstartData;
import com.redhat.rhn.frontend.struts.RequestContext;
+import com.redhat.rhn.frontend.struts.RhnValidationHelper;
import com.redhat.rhn.manager.kickstart.BaseKickstartCommand;
import com.redhat.rhn.manager.kickstart.KickstartFileDownloadCommand;
import com.redhat.rhn.manager.kickstart.KickstartManager;
@@ -81,8 +84,13 @@ public class KickstartFileDownloadAction extends BaseKickstartEditAction {
KickstartManager.getInstance().renderKickstart(data)));
}
catch (DownloadException de) {
- request.setAttribute(FILEDATA,
- StringEscapeUtils.escapeHtml(de.getContent()));
+ RhnValidationHelper.setFailedValidation(ctx.getRequest());
+ try {
+ ValidatorException.raiseException("kickstart.jsp.error.template_generation",
+ LocalizationService.getInstance().getMessage("Details"));
+ } catch (ValidatorException ve) {
+ getStrutsDelegate().saveMessages(request, ve.getResult());
+ }
}
request.setAttribute(KSURL, KickstartUrlHelper.getCobblerProfileUrl(data));
diff --git a/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml b/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
index 0b21908..2010f1e 100644
--- a/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
+++ b/java/code/src/com/redhat/rhn/frontend/strings/jsp/StringResource_en_US.xml
@@ -9050,7 +9050,7 @@ Please note that some manual configuration of these scripts may still be require
<trans-unit id="kickstart.jsp.error.template_generation">
- <source>There are errors in your kickstart template. Please check the '{0}' tab to determine the problem with the template.</source>
+ <source>There are errors in your kickstart template. Please check the file contents on the '{0}' tab to determine problems with the template.</source>
<context-group name="ctx">
<context context-type="sourcefile">Kickstart Details</context>
</context-group>
diff --git a/java/code/src/com/redhat/rhn/manager/kickstart/KickstartManager.java b/java/code/src/com/redhat/rhn/manager/kickstart/KickstartManager.java
index ee0d13e1..055c658 100644
--- a/java/code/src/com/redhat/rhn/manager/kickstart/KickstartManager.java
+++ b/java/code/src/com/redhat/rhn/manager/kickstart/KickstartManager.java
@@ -122,13 +122,13 @@ public class KickstartManager extends BaseManager {
ValidatorException.
raiseException("kickstart.jsp.error.template_generation",
LocalizationService.getInstance().
- getMessage("kickstartdownload.jsp.header"));
+ getMessage("Details"));
}
}
}
catch (DownloadException de) {
ValidatorException.raiseException("kickstart.jsp.error.template_generation",
- LocalizationService.getInstance().getMessage("kickstartdownload.jsp.header"));
+ LocalizationService.getInstance().getMessage("Details"));
}
}
diff --git a/java/code/webapp/WEB-INF/pages/kickstart/wizard/profile/advanced/download.jsp b/java/code/webapp/WEB-INF/pages/kickstart/wizard/profile/advanced/download.jsp
index 1212d86..e531b52 100644
--- a/java/code/webapp/WEB-INF/pages/kickstart/wizard/profile/advanced/download.jsp
+++ b/java/code/webapp/WEB-INF/pages/kickstart/wizard/profile/advanced/download.jsp
@@ -8,20 +8,20 @@
<body>
<%@ include file="/WEB-INF/pages/common/fragments/kickstart/advanced/header.jspf"%>
-<div class="page-summary">
- <h2><bean:message key="kickstartdownload.jsp.header"/></h2>
- <p><bean:message key="kickstartdownload.jsp.summary"/></p>
- <table class="details" border="0">
- <tr><td>
- <a href="${ksurl}" target="_new"><bean:message key="kickstartdownload.jsp.download"/></a>
-
- </td></tr>
- <tr><td>
- <pre style="overflow: scroll; width: 800px; height: 800px">${filedata}</pre>
- </td></tr>
- </table>
-</div>
-
+<c:if test="${not rhn_validation_failed}">
+ <div class="page-summary">
+ <h2><bean:message key="kickstartdownload.jsp.header"/></h2>
+ <p><bean:message key="kickstartdownload.jsp.summary"/></p>
+ <table class="details" border="0">
+ <tr><td>
+ <a href="${ksurl}" target="_new"><bean:message key="kickstartdownload.jsp.download"/></a>
+ </td></tr>
+ <tr><td>
+ <pre style="overflow: scroll; width: 800px; height: 800px">${filedata}</pre>
+ </td></tr>
+ </table>
+ </div>
+</c:if>
</body>
</html:html>
\ No newline at end of file
--
1.7.10.4
_______________________________________________
Spacewalk-devel mailing list
Spacewalk-devel@redhat.com
https://www.redhat.com/mailman/listinfo/spacewalk-devel
