request not defined in export functions

Issue #490 resolved
Tim de Wit created an issue

By accident (not having defined XLSX_DATE in settings.py) during xml export I encountered a bug in the exception code, showing an error message that "request" was not defined:

 except:
     messages.error(request, "Unexpected error ....)

Possible solutions:
1) remove message.error() lines from the export functions
2) pass the complete request object as first parameter to the export functions in exportviews.py instead of request.GET

Personally I think option 2 is best; do you agree @edmcdonagh ?

Comments (8)

  1. Ed McDonagh

    I'm not sure. I haven't looked at the code, and I don't think I'll get time to today.

    Instinctively, I'm not sure how useful the request object is in the export error messages - what do we want to find out from it?

  2. Tim de Wit reporter

    you tell me.. I didn't write the code :-)
    My best guess would be that messages.error() is responsible for showing the error message in the export-GUI.

  3. Ed McDonagh

    That's the thing. It won't! It may have be done with an earlier implementation, but now exports are asynchronous and nothing to do with the request once they have been started.

    The message in the interface is controlled by the status element.

  4. Tim de Wit reporter

    In that case, I suggest replacing:

    messages.error(request, "Unexpected error creating temporary file - please contact an administrator: {0}".format(sys.exc_info()[0]))
    

    by:

    tsk.progress = 'Unexpected error creating temporary file - please contact an administrator: {0}'.format(sys.exc_info()[0])   
    tsk.save()
    

    Btw.. I also noticed that after pressing "Export to XLSX" the status field is not updated automatically; it requires a page refresh or manual click on the "exports" menu item. Does that happen at your end as well, or should I open a new issue for that?

  5. Ed McDonagh

    Regarding the last point, the exports page checks for current jobs on page load, and if it finds one it sets a page reload time of 3s. What you are finding is that the page loads before the task starts, so the reload isn't set.

    The proper solution is to replace that with an AJAX section so that it continuously checks and just reloads that section of the page, not the whole thing.

    Another thing that's been on my to do list for years, never quite getting to the top as it kind of works as is!

  6. Log in to comment