Alexander Wels has posted comments on this change. Change subject: webadmin: Simplify report redirect ......................................................................
Patch Set 4: (3 comments) http://gerrit.ovirt.org/#/c/33550/4/backend/manager/modules/services/src/main/webapp/WEB-INF/web.xml File backend/manager/modules/services/src/main/webapp/WEB-INF/web.xml: Line 71: <servlet> Line 72: <servlet-name>reports-dashboard-redirect</servlet-name> Line 73: <servlet-class>org.ovirt.engine.core.utils.servlet.RedirectServlet</servlet-class> Line 74: <init-param> Line 75: <param-name>url</param-name> > Is this "url" parameter necessary? (Doesn't GWT UI send its value alongside This is the target URL, that you get redirected to. This way the GWT application doesn't need to know the target. it just needs to know the redirect URL. I will rename it so its purpose is more clear. Line 76: <param-value>%{ENGINE_REPORTS_DASHBOARD_URL}</param-value> Line 77: </init-param> Line 78: </servlet> Line 79: <servlet-mapping> http://gerrit.ovirt.org/#/c/33550/4/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/WebAdminHostPageServlet.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/WebAdminHostPageServlet.java: Line 83: // Set attribute for engineSessionTimeout object Line 84: Integer engineSessionTimeout = getEngineSessionTimeout(getEngineSessionId(request)); Line 85: request.setAttribute(ATTR_ENGINE_SESSION_TIMEOUT, getEngineSessionTimeoutObject(engineSessionTimeout)); Line 86: Line 87: request.setAttribute(ATTR_ENGINE_REPORTS_BASE_URL, > Please add some comment like following: Done Line 88: getReportInit(reportRedirectUrl.substring(reportBaseUrl.length()), Line 89: reportRightClickRedirectUrl.substring(reportBaseUrl.length()))); Line 90: Line 91: super.doGet(request, response); http://gerrit.ovirt.org/#/c/33550/4/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/ReportsUrls.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/ReportsUrls.java: Line 1: package org.ovirt.engine.ui.uicommonweb; Line 2: Line 3: import com.google.gwt.core.client.JavaScriptObject; Line 4: Line 5: public final class ReportsUrls extends JavaScriptObject { > Double whitespace after "ReportsUrls" :-) Done Line 6: Line 7: protected ReportsUrls() { Line 8: } Line 9: -- To view, visit http://gerrit.ovirt.org/33550 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic4718bf7f734ea72d2494fc463a87cb7675df0c4 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: Shirly Radco <sra...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: Yaniv Dary <yd...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
