Add OperationResult class and begin handling sql errors more gracefully #608
Conversation
Update to comply with new use of OperationResult class
Accept response and other params, parse the xml for the 'error' node and fill alert with message
web/js/projectManagement.js
Outdated
| 'exception': function(proxy, type, action, exception, res){ | ||
| var parser = new DOMParser(); | ||
| var errorDoc = parser.parseFromString(res.responseText, "text/xml"); | ||
| var errorMessage = errorDoc.getElementsByTagName("error")[0].childNodes[0].nodeValue; |
There was a problem hiding this comment.
I'm not sure how the format of the XML response is but is it possible to have multiple error elements? In this case we may need to be more specific to be sure we are displaying all the errors, but I'm happy to fetch this and test it next week as well as I'm not completely sure how it works.
There was a problem hiding this comment.
We should be able to since we control the shape of the xml sent back (in the service). We could either add as many distinct <error> elements to the object as needed or concat multiple errors into one message. Then just parse this out to the alert. I'll test the js here with multiple <error> elements; I probably need to modify the last bit of code since it's taking the first items out of the arrays instead of looping. Good catch on this. I'll do some local testing on multiple messages.
|
Maybe you already know or have a better way to do it, but there's a |
|
I just noticed that even with an error, the project isn't created in the database but it is shown in the project table, so I think we should also remove it from the front end in case it fails to avoid confusion like people thinking the project was created even with errors when it wasn't. Btw, this is a behavior that was not caused by your patch, but I think is a good opportunity to fix it too. |
- Return array of OperationResults if we are creating multiple projects - Check for null dates and format appropriately if not null - Check in service for multiple items saved - Service handle multiple errors
|
@anarute That's a good catch. I just added a call to reload the data store if creation fails so that we don't have a bad phantom record listed. |
- Use spaces instead of tabs in OperationResult object - Make props private since we have public getters - Update service to use getters - Update DAO to init object with isSuccessful set to false
This PR begins to address #496 by adding an OperationResult class and updates code related to the creation of Projects to begin using this class. Changes in these commits include:
Comments and suggestions welcome as my PHP is a bit rusty.