fix modal upload validation and tests

app/assets/javascripts/application.js

@@ -24,18 +24,15 @@
 
 
 $(document).ready(function () {
-
 	$('.dropdown-toggle').dropdown();
 
 	// disable in browser form validations
 	$('form').find('input').removeAttr('required');
 
-
 	$('.theses .options a').click(function (e) {
 		e.preventDefault();
 		$(this).tab('show');
 		$(".theses .options a").removeClass("active")
 		$(this).addClass("active")
 	})
-
-});
+});

app/assets/javascripts/theses.js

@@ -37,10 +37,4 @@ $(document).ready(function () {
 			source: committee_member_names_list
 		});
 	});
-
-	$('form.file-upload input.file').change ( function() {
-	    if ($(this).val()) {
-	        $(this).parents('form:first').find('input:submit').removeAttr('disabled');
-	    } 
-	});
 });

app/models/document.rb

@@ -29,7 +29,31 @@ class Document < ApplicationRecord
 
   attribute :usage, default: :thesis
 
+
+  PRIMARY_FILE_EXT = [ '.pdf' ].freeze
+  SUPPLEMENTAL_FILE_EXT = ['.pdf', '.doc', '.docx', '.txt', '.html', '.htm', '.odt', '.odp',
+    '.ods', '.png', '.tif', '.jpg', '.csv', '.xml', '.avi', '.flac', '.wav', '.mp3', '.mp4', '.mov'].freeze
+  EMBARGO_FILE_EXT = ['.pdf', '.txt', '.html', '.htm', '.odt', '.odp', '.ods'].freeze
+  LICENCE_FILE_EXT = [ '.pdf' ].freeze
+
   #### ADDITIONAL METHODS
+  def allowed_extensions
+    list = PRIMARY_FILE_EXT
+
+    case document_type
+    when 'primary'
+      list = PRIMARY_FILE_EXT
+    when 'supplemental'
+      list = SUPPLEMENTAL_FILE_EXT
+    when 'licence'
+      list = LICENCE_FILE_EXT
+    when 'embargo'
+      list = EMBARGO_FILE_EXT
+    end
+
+    return list
+  end
+
   def image?
     file.to_s.include?('.gif') or file.to_s.include?('.png') or file.to_s.include?('.jpg')
   end
@@ -55,15 +79,17 @@ def primary_file_must_be_pdf
 
     return if file.filename.downcase.end_with?('.pdf')
 
-    errors.add(:file, 'Primary file must be a PDF')
+    errors.add(:file, "extension #{ext} is not allowed.")
   end
 
   def supplemental_file_must_be_file_types
-    supplemental_file_types = ['.pdf', '.doc', '.docx', '.txt', '.html', '.htm', '.odt', '.odp', '.ods', '.png', '.tif', '.jpg', '.csv', '.xml', '.avi', '.flac', '.wav', '.mp3', '.mp4', '.mov']
-    embargo_file_types = ['.pdf', '.txt', '.html', '.htm', '.odt', '.odp', '.ods']
+    supplemental_file_types = SUPPLEMENTAL_FILE_EXT
+    embargo_file_types = EMBARGO_FILE_EXT
 
     return unless file.filename.present? && supplemental
 
+    ext = '.' + file.filename.downcase.split('.').pop
+
     if document_type == "licence"
       return if file.filename.downcase.end_with?('.pdf')
     elsif document_type == "supplemental"
@@ -72,7 +98,7 @@ def supplemental_file_must_be_file_types
       return if embargo_file_types.include?(File.extname(file.filename.downcase))
     end
 
-    errors.add(:file, "#{document_type.capitalize} file must be a valid file type")
+    errors.add(:file, "extension #{ext} is not allowed.")
   end
 
   def document_type

app/views/documents/_form.html.erb

@@ -1,4 +1,5 @@
-<%= simple_form_for [@student, @thesis, @document], authenticity_token: true, html: {  multipart: true, class: "file-upload" } do |f| %>
+<%= simple_form_for [@student, @thesis, @document], authenticity_token: true, 
+html: {  multipart: true, class: "file-upload" } do |f| %>
 
   <h2 class="fs-6">
   	<% if @document.usage == 'thesis' && !@document.supplemental %>
@@ -37,11 +38,13 @@
   <% end %>
 
   <% f.input :name, required: true, input_html: { autocomplete: "off"} %>
-	<%= f.input :file, required: true, as: :file %>
+	<%= f.input :file, required: true, as: :file, input_html: { "data-ext": "#{@document.allowed_extensions.join(',')}" } %>
+  <div class="invalid-feedback"></div>
+
 	<%= f.hidden_field :usage, value: @document.usage %>
 	<%= f.hidden_field :supplemental, value: @document.supplemental %>
-  
-  <%= f.button :submit, "Upload", class: "btn btn-primary file-upload", disabled: true %>
+
+  <%= f.button :submit, "Upload", class: "btn btn-primary file-upload", id: "file_upload_button" %>
 
   <% if (defined? modal).nil? || !modal %>
     <% if current_user.role == User::STUDENT && @document.usage == 'licence' %>

app/views/documents/_modal.html.erb

@@ -1,5 +1,5 @@
 <div class="modal" id="file_upload_modal" tabindex="-1" aria-hidden="true">
-  <div class="modal-dialog modal-dialog-centered modal-dialog-scrollable">
+  <div class="modal-dialog modal-dialog-centered">
     <div class="modal-content">
       <div class="modal-header">
         <h1 class="modal-title fs-5">

app/views/documents/_upload_modal.js.erb

@@ -0,0 +1,22 @@
+$('#file_upload_modal').remove();
+$('body').append('<%= j render "modal" %>');
+$('#file_upload_modal .invalid-feedback').empty().hide();
+(new bootstrap.Modal($('#file_upload_modal'))).show();
+
+$(document).on('click', '#file_upload_button', function(e) {
+    file = $(this).parents('form:first').find('input:file');
+    if (file.val()) {
+        var allowed = file.data('ext').split(',');
+        var ext = '.' + file.val().split('.').pop().toLowerCase();
+        if($.inArray(ext, allowed) == -1) {
+            div = $(this).parents('form:first').find('div.invalid-feedback');
+            var error = '<p>File extension ' + ext + ' is not allowed.</p>';
+            div.empty().append(error);
+            div.show();
+        } else {
+            return true;
+        }
+    }
+    e.preventDefault();
+    return false;
+});

app/views/documents/edit.js.erb

@@ -1,8 +1 @@
-$('body').append('<%= j render "modal" %>');
-(new bootstrap.Modal($('#file_upload_modal'))).show();
-
-$('form.file-upload input.file').change ( function() {
-    if ($(this).val()) {
-        $(this).parents('form:first').find('input:submit').removeAttr('disabled');
-    } 
-});
+<%= render partial: 'upload_modal', formats: :js %>

app/views/documents/new.js.erb

@@ -1,8 +1 @@
-$('body').append('<%= j render "modal" %>');
-(new bootstrap.Modal($('#file_upload_modal'))).show();
-
-$('form.file-upload input.file').change ( function() {
-    if ($(this).val()) {
-        $(this).parents('form:first').find('input:submit').removeAttr('disabled');
-    } 
-});
+<%= render partial: 'upload_modal', formats: :js %>

test/models/document_test.rb

@@ -422,4 +422,32 @@ class DocumentTest < ActiveSupport::TestCase
     assert_equal 4, Document.supplemental.count, 'The should be 4 supplemental documents'
     assert_equal 2, Document.primary.count, 'There should be 5 primary documents'
   end
+
+  should 'allowed_extensions should match document usage' do
+    d = Document.new
+
+    d.usage = 'thesis'
+    d.supplemental = false
+    assert_equal Document::PRIMARY_FILE_EXT, d.allowed_extensions
+
+    d.usage = 'thesis'
+    d.supplemental = true
+    assert_equal Document::SUPPLEMENTAL_FILE_EXT, d.allowed_extensions
+
+    d.usage = 'licence'
+    d.supplemental = false
+    assert_equal Document::LICENCE_FILE_EXT, d.allowed_extensions
+
+    d.usage = 'licence'
+    d.supplemental = true
+    assert_equal Document::LICENCE_FILE_EXT, d.allowed_extensions
+
+    d.usage = 'embargo'
+    d.supplemental = false
+    assert_equal Document::EMBARGO_FILE_EXT, d.allowed_extensions
+
+    d.usage = 'embargo'
+    d.supplemental = true
+    assert_equal Document::EMBARGO_FILE_EXT, d.allowed_extensions
+  end
 end

test/system/theses_test.rb

@@ -206,7 +206,7 @@ class ThesesTest < ApplicationSystemTestCase
     attach_file("document_file", Rails.root.join('test/fixtures/files/image-example.jpg'))
     click_button('Upload')
 
-    assert_selector(".invalid-feedback", text: "Primary file must be a PDF")
+    assert_selector(".invalid-feedback", text: "File extension .jpg is not allowed.")
   end
 
   should "not upload supplmentary document with incorrect file format as student" do
@@ -228,9 +228,10 @@ class ThesesTest < ApplicationSystemTestCase
 
     click_on("Upload Supplementary Thesis Files")
     attach_file("document_file", Rails.root.join('test/fixtures/files/zip-file.zip'))
-    click_button('Upload')
 
-    assert_selector(".invalid-feedback", text: "File Supplemental file must be a valid file type")
+    click_button('Upload')
+
+    assert_selector(".invalid-feedback", text: "File extension .zip is not allowed.")
   end
 
   should "not upload supplmentary document with incorrect file format as admin" do
@@ -241,7 +242,7 @@ class ThesesTest < ApplicationSystemTestCase
     attach_file("document_file", Rails.root.join('test/fixtures/files/zip-file.zip'))
     click_button('Upload')
 
-    assert_selector(".invalid-feedback", text: "File Supplemental file must be a valid file type")
+    assert_selector(".invalid-feedback", text: "File extension .zip is not allowed.")
   end
 
   should "be able to upload supplementary document by admin/staff" do