Improved error reporting in the controller.Files.upload() method. Now

correctly displays an error to the user when uploading without a filename or
with an invalid file.

controller/Files.py

@@ -14,7 +14,7 @@ from model.User import User
 from view.Upload_page import Upload_page
 from view.Blank_page import Blank_page
 from view.Json import Json
-from view.Progress_bar import stream_progress, stream_quota_error, stop_upload_script
+from view.Progress_bar import stream_progress, stream_quota_error, quota_error_script, general_error_script
 
 
 class Access_error( Exception ):
@@ -326,7 +326,7 @@ class Files( object ):
 
     uploaded_file = current_uploads.get( file_id )
     if not uploaded_file:
-      raise Upload_error()
+      return dict( script = general_error_script % u"Please select a file to upload." )
 
     current_uploads_lock.acquire()
     try:
@@ -334,27 +334,23 @@ class Files( object ):
     finally:
       current_uploads_lock.release()
 
-    if not self.__users.check_access( user_id, notebook_id, read_write = True ):
+    user = self.__database.load( User, user_id )
+    if not user or not self.__users.check_access( user_id, notebook_id, read_write = True ):
       uploaded_file.delete()
-      raise Access_error()
+      return dict( script = general_error_script % u"Sorry, you don't have access to do that. Please make sure you're logged in as the correct user." )
 
     content_type = upload.headers.get( "content-type" )
 
     # if we didn't receive all of the expected data, abort
     if uploaded_file.total_received_bytes < uploaded_file.content_length:
       uploaded_file.delete()
-      raise Upload_error( u"The file did not complete uploading." )
-
-    user = self.__database.load( User, user_id )
-    if not user:
-      uploaded_file.delete()
-      raise Access_error()
+      return dict() # hopefully, the call to progress() will report this to the user
 
     # if the uploaded file's size would put the user over quota, bail and inform the user
     rate_plan = self.__users.rate_plan( user.rate_plan )
     if user.storage_bytes + uploaded_file.total_received_bytes > rate_plan.get( u"storage_quota_bytes", 0 ):
       uploaded_file.delete()
-      return dict( script = stop_upload_script )
+      return dict( script = quota_error_script )
 
     # record metadata on the upload in the database
     db_file = File.create( file_id, notebook_id, note_id, uploaded_file.filename, uploaded_file.file_received_bytes, content_type )

static/js/Wiki.js

@@ -2352,26 +2352,32 @@ Upload_pulldown.prototype.update_position = function ( anchor, relative_to ) {
   Pulldown.prototype.update_position.call( this, anchor, relative_to );
 }
 
-Upload_pulldown.prototype.shutdown = function ( force, display_quota_error ) {
-  // if there's an upload in progress and the force flag is not set, then bail without performing a
-  // shutdown
-  if ( this.uploading ) {
-    if ( !force )
-      return;
-    if ( !display_quota_error )
-      this.wiki.display_message( "The file upload has been cancelled." )
-  }
+Upload_pulldown.prototype.cancel_due_to_click = function () {
+  this.wiki.display_message( "The file upload has been cancelled." )
+  this.shutdown( true );
+}
+
+Upload_pulldown.prototype.cancel_due_to_quota = function () {
+  this.wiki.display_error(
+    "That file is too large for your available storage space. Before uploading, please delete some notes or files, empty the trash, or",
+    [ createDOM( "a", { "href": "/upgrade" }, "upgrade" ), " your account." ]
+  );
+
+  this.shutdown( true );
+}
+
+Upload_pulldown.prototype.cancel_due_to_error = function ( message ) {
+  this.wiki.display_error( message )
+  this.shutdown( true );
+}
+
+Upload_pulldown.prototype.shutdown = function ( force ) {
+  if ( this.uploading && !force )
+    return;
 
   Pulldown.prototype.shutdown.call( this );
   if ( this.link )
     this.link.pulldown = null;
-
-  if ( display_quota_error ) {
-    this.wiki.display_message(
-      "That file is too large for your available storage space. Before uploading, please delete some notes or files, empty the trash, or",
-      [ createDOM( "a", { "href": "/upgrade" }, "upgrade" ), " your account." ]
-    );
-  }
 }
 
 function File_link_pulldown( wiki, notebook_id, invoker, editor, link ) {

view/Progress_bar.py

@@ -33,7 +33,7 @@ def stream_progress( uploading_file, filename, fraction_reported ):
     <td></td>
     <td><span id="status"></span></td>
     <td></td>
-    <td><input type="submit" id="cancel_button" class="button" value="cancel" onclick="withDocument( window.parent.document, function () { getElement( 'upload_frame' ).pulldown.shutdown( true ); } );" /></td>
+    <td><input type="submit" id="cancel_button" class="button" value="cancel" onclick="withDocument( window.parent.document, function () { getElement( 'upload_frame' ).pulldown.cancel_due_to_click(); } );" /></td>
     </tr></table>
     <script type="text/javascript">
     function tick( fraction ) {
@@ -75,9 +75,15 @@ def stream_progress( uploading_file, filename, fraction_reported ):
     """
 
 
-stop_upload_script = \
+general_error_script = \
   """
-  withDocument( window.parent.document, function () { getElement( 'upload_frame' ).pulldown.shutdown( true, true ); } );
+  withDocument( window.parent.document, function () { getElement( 'upload_frame' ).pulldown.cancel_due_to_error( "%s" ); } );
+  """
+
+
+quota_error_script = \
+  """
+  withDocument( window.parent.document, function () { getElement( 'upload_frame' ).pulldown.cancel_due_to_quota(); } );
   """
 
 
@@ -96,4 +102,4 @@ def stream_quota_error():
     </script>
     </body>
     </html>
-    """ % stop_upload_script
+    """ % quota_error_script