Improve handling of missing uploaded files for bulk registration.

When used with a browser, the support for uploading photos as part of
bulk registration resulted in errors when a ZIP file was not uploaded.
This did not show up in testing because MechanicalSoup versions 0.11
and earlier do not handle missing file uploads the same way as a
browser does
<MechanicalSoup/MechanicalSoup#250>.
Similar issues apply when no CSV file is uploaded (should produce an
error, did not with a browser).

This change fixes the code handling uploads to handle empty filenames,
as used by browsers when no file is uploaded, the same as the form
field for the upload being missing, with consequent changes to handle
the case of empty zip_ref following a missing ZIP upload.  Tests are
added for the browser-like case of missing CSV uploads, though those
are only properly effective with newer MechanicalSoup.  This passes
testing with both older and newer MechanicalSoup.

matholymp/roundupreg/rounduputil.py

@@ -393,11 +393,12 @@ def bulk_csv_data(form, comma_sep_fields=()):
     for whether just to check the results or act on them, or a string
     error message.
     """
-    # A csv_file form input means data has been uploaded for initial
-    # checking.  A csv_contents form input means the previously
-    # checked data has been submitted for actual bulk registration
-    # (but should still be rechecked rather than trusting the client).
-    if 'csv_file' in form:
+    # A csv_file form input with nonempty filename means data has been
+    # uploaded for initial checking.  A csv_contents form input means
+    # the previously checked data has been submitted for actual bulk
+    # registration (but should still be rechecked rather than trusting
+    # the client).
+    if 'csv_file' in form and form['csv_file'].filename != '':
         check_only = True
         file_bytes = form['csv_file'].value
         if not isinstance(file_bytes, bytes):
@@ -498,9 +499,10 @@ def bulk_zip_data(db, form):
     Return a ZipFile object for an uploaded ZIP file, or None if no
     such file was uploaded, or a string on error.
     """
-    # A zip_file form input means a ZIP file has been uploaded now.  A
-    # zip_ref form input refers to such a file previously uploaded.
-    if 'zip_file' in form:
+    # A zip_file form input with nonempty filename means a ZIP file
+    # has been uploaded now.  A nonempty zip_ref form input refers to
+    # such a file previously uploaded.
+    if 'zip_file' in form and form['zip_file'].filename != '':
         file_bytes = form['zip_file'].value
         if not isinstance(file_bytes, bytes):
             return 'zip_file not an uploaded file'
@@ -514,7 +516,7 @@ def bulk_zip_data(db, form):
         except (zipfile.BadZipFile, zipfile.LargeZipFile, RuntimeError,
                 ValueError, NotImplementedError, EOFError) as exc:
             return str(exc)
-    elif 'zip_ref' in form:
+    elif 'zip_ref' in form and form['zip_ref'].value != '':
         hashstr = form['zip_ref'].value
         if not isinstance(hashstr, str):
             return 'zip_ref an uploaded file'

matholymp/roundupreg/templating.py

@@ -472,7 +472,9 @@ def bulk_csv_contents(form):
     # avoid errors to the registration system admin when the template
     # using this function is accessed directly without such an upload,
     # silently return an empty string in such a case.
-    if 'csv_file' not in form or not isinstance(form['csv_file'].value, bytes):
+    if ('csv_file' not in form
+        or form['csv_file'].filename == ''
+        or not isinstance(form['csv_file'].value, bytes)):
         return ''
     return base64.b64encode(form['csv_file'].value).decode('ascii')
 
@@ -593,11 +595,14 @@ def show_bulk_csv_person(db, form):
 
 def bulk_zip_ref(form):
     """Return hash of a previously uploaded ZIP file."""
-    # In valid uses, zip_file is present as an uploaded file.  To
-    # avoid errors to the registration system admin when the template
-    # using this function is accessed directly without such an upload,
-    # silently return an empty string in such a case.
-    if 'zip_file' not in form or not isinstance(form['zip_file'].value, bytes):
+    # In valid uses, zip_file is present as a possibly uploaded file
+    # (empty filename if no file was uploaded).  To avoid errors to
+    # the registration system admin when the template using this
+    # function is accessed directly without such an upload, silently
+    # return an empty string in such a case.
+    if ('zip_file' not in form
+        or form['zip_file'].filename == ''
+        or not isinstance(form['zip_file'].value, bytes)):
         return ''
     return hashlib.sha256(form['zip_file'].value).hexdigest()
 

matholymp/test/test_reg_system.py

@@ -3278,7 +3278,15 @@ def test_country_bulk_register_errors(self):
         admin_session.set({'@template': 'item'})
         admin_session.check_submit_selected(error='Node id specified for bulk '
                                             'registration')
-        # Errors missing uploaded CSV data.
+        # Errors missing uploaded CSV data.  The first case (csv_file
+        # present in the form but no file submitted there) is wrongly
+        # handled the same as the second (no csv_file in the form) by
+        # MechanicalSoup versions 0.11 and earlier
+        # <https://github.com/MechanicalSoup/MechanicalSoup/issues/250>.
+        admin_session.check_open_relative('country?@template=bulkregister')
+        admin_session.select_main_form()
+        form = admin_session.get_main_form()
+        admin_session.check_submit_selected(error='no CSV file uploaded')
         admin_session.check_open_relative('country?@template=bulkregister')
         admin_session.select_main_form()
         form = admin_session.get_main_form()
@@ -12021,7 +12029,15 @@ def test_person_bulk_register_errors(self):
         admin_session.set({'@template': 'item'})
         admin_session.check_submit_selected(error='Node id specified for bulk '
                                             'registration')
-        # Errors missing uploaded CSV data.
+        # Errors missing uploaded CSV data.  The first case (csv_file
+        # present in the form but no file submitted there) is wrongly
+        # handled the same as the second (no csv_file in the form) by
+        # MechanicalSoup versions 0.11 and earlier
+        # <https://github.com/MechanicalSoup/MechanicalSoup/issues/250>.
+        admin_session.check_open_relative('person?@template=bulkregister')
+        admin_session.select_main_form()
+        form = admin_session.get_main_form()
+        admin_session.check_submit_selected(error='no CSV file uploaded')
         admin_session.check_open_relative('person?@template=bulkregister')
         admin_session.select_main_form()
         form = admin_session.get_main_form()