Lior Vernia has posted comments on this change. Change subject: webadmin: Aggregate error messages in Frontend.runMultipleActions() ......................................................................
Patch Set 2: (5 comments) http://gerrit.ovirt.org/#/c/25837/2/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/Frontend.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/Frontend.java: Line 707: final List<VdcActionParametersBase> parameters, Line 708: final List<IFrontendActionAsyncCallback> callbacks, Line 709: final IFrontendActionAsyncCallback failureCallback, Line 710: final Object state) { Line 711: runMultipleActions(actionTypes, parameters, callbacks, failureCallback, state, false, null); > I agree, however the question is, how will this change (aggregate error mes I agree as well, but didn't know what's the required behavior for code already written, as Vojtech pointed out. Left as is. Line 712: } Line 713: Line 714: /** Line 715: * This method allows us to run a transaction like set of actions. If one Line 744: final boolean aggregateErrors, Line 745: final List<String> errorMessages) { Line 746: if (actionTypes.isEmpty() || parameters.isEmpty() || callbacks.isEmpty()) { Line 747: if (aggregateErrors && errorMessages != null && !errorMessages.isEmpty()) { Line 748: failureEventHandler(null, errorMessages); > Each returnValue has a description. IMO you should show it. Done, using runMultipleActionFailed(). Line 749: } Line 750: return; Line 751: } Line 752: Line 764: actionTypes.remove(0); Line 765: parameters.remove(0); Line 766: callbacks.remove(0); Line 767: if (aggregateErrors && returnValue != null && !returnValue.getSucceeded()) { Line 768: errorMessages.addAll(returnValue.getCanDoActionMessages()); > 1. What about returnValue.getFault()? Done, extended runMultipleActionFailed() implementation to handle cases where the failure isn't caused by canDoAction(). Line 769: } Line 770: runMultipleActions(actionTypes, Line 771: parameters, Line 772: callbacks, Line 971: && (getEventsHandler().isRaiseErrorModalPanel(actionType, result.getFault()))) { Line 972: if (result.getCanDoActionMessages().size() <= 1) { Line 973: String errorMessage = !result.getCanDoAction() || !result.getCanDoActionMessages().isEmpty() Line 974: ? getRunActionErrorMessage(result.getCanDoActionMessages()) : result.getFault().getMessage(); Line 975: > Not in the scope of your patch. But- Left as is, as not in scope. Line 976: failureEventHandler(result.getDescription(), errorMessage); Line 977: } else { Line 978: failureEventHandler(result.getDescription(), result.getCanDoActionMessages()); Line 979: } Line 973: String errorMessage = !result.getCanDoAction() || !result.getCanDoActionMessages().isEmpty() Line 974: ? getRunActionErrorMessage(result.getCanDoActionMessages()) : result.getFault().getMessage(); Line 975: Line 976: failureEventHandler(result.getDescription(), errorMessage); Line 977: } else { > It is not in the scope of your patch. Not relevant anymore (as far as the scope of my patch is concerned), used runMultipleActionFailed instead of failureEventHandler(). Line 978: failureEventHandler(result.getDescription(), result.getCanDoActionMessages()); Line 979: } Line 980: } Line 981: } -- To view, visit http://gerrit.ovirt.org/25837 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0236b7d9316dff3d9bd22f2127318eefc37bbda1 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@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
