Added error message for scan formatting error #52
Conversation
Try catch is added to catch JSONException thrown by ScanJson . After error occurs scan formatting is set to false for the remaining rows and error message is displayed .
|
Can one of the admins verify this patch? Also need an authorization to run tests. |
|
runtests |
| { | ||
| e.printStackTrace(); | ||
| DialogUtils.showError("Scan formatting is not possible for the selected table",true); | ||
| config.setScanFormatting(false); |
There was a problem hiding this comment.
I think it makes more sense to abort the operation instead of selecting a different option. That also avoids adding an extra publicly mutable field to CsvConfig.
Is there a way to check if the attachment exists instead of catching the exception?
There was a problem hiding this comment.
Thanks for your suggestion @linl33 , The table I uploaded did had attachments and the download attachments option was also working fine . I used the geotagger table ( https://github.com/odk-x/suitcase/tree/master/testfiles/dataToUpload ) . Only this scan formatting option is returning an error . In the table metadata fetched I was unable to find any parameter which could determine if scan formatting is possible or not .
I can add code to abort the operation if a JSONException is thrown by this method . Please suggest
There was a problem hiding this comment.
What I meant by "attachment exists" is to check if the particular attachment required to apply scan formatting exists. I think it makes more sense to abort the download when a JSONException is caught here.
|
Thanks @linl33 , I have made the changes to abort the process. There was a try catch block in the ODKCsv file that was catching the exception and printing it in the log . This resulted in the process running even after there was an error . I have added the exceptions to method signature and now that exception will end the process and display an error message to the user . Please review. |
|
@linl33 does this do what you were thinking? |
|
@Shubhamr837 is this still good to go after all your other changes? I am guessing so, just confirming. |
1 similar comment
|
@Shubhamr837 is this still good to go after all your other changes? I am guessing so, just confirming. |
|
@Shubhamr837 is this PR still good to merge after all your other changes? I am guessing so, just confirming. |
|
@wbrunette sorry for late reply, yes this PR is needed for error handling in suitcase |
Scan json object is only created if scan formatting is checked as true .Try catch is added to catch JSONException thrown by ScanJson . After error occurs scan formatting is set to false for the remaining rows and error message is displayed .
should fix odk-x/tool-suite-X#220