From 7d006044f35301f7c5b11440d3a72a9cb3a9a650 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Karel=20Pi=C4=8Dman?= <karel.picman@kontron.com>
Date: Tue, 4 Feb 2020 13:39:01 +0100
Subject: [PATCH] #1080 Context menu tests

---
 .../dmsf_context_menus_controller.rb          |  8 +++---
 app/controllers/dmsf_upload_controller.rb     |  3 +++
 app/views/dmsf_upload/upload_files.html.erb   | 26 ++++++++++---------
 lib/redmine_dmsf/webdav/base_resource.rb      |  2 +-
 lib/redmine_dmsf/webdav/custom_middleware.rb  | 15 +----------
 lib/redmine_dmsf/webdav/dmsf_resource.rb      |  7 +++--
 lib/redmine_dmsf/webdav/project_resource.rb   | 14 ++++------
 test/fixtures/dmsf_links.yml                  | 14 +++++++++-
 .../dmsf_context_menus_controller_test.rb     | 15 ++++++-----
 .../rest_api/dmsf_file_api_test.rb            |  1 -
 .../dmsf_webdav_custom_middleware_test.rb     |  4 +--
 test/unit/project_patch_test.rb               |  4 +--
 12 files changed, 57 insertions(+), 56 deletions(-)

diff --git a/app/controllers/dmsf_context_menus_controller.rb b/app/controllers/dmsf_context_menus_controller.rb
index 5c2ff5a5..e7cca722 100644
--- a/app/controllers/dmsf_context_menus_controller.rb
+++ b/app/controllers/dmsf_context_menus_controller.rb
@@ -31,15 +31,15 @@ class DmsfContextMenusController < ApplicationController
   def dmsf
     if @dmsf_file
       @locked = @dmsf_file.locked?
-      @unlockable = @dmsf_file.unlockable? && (!@dmsf_file.locked_for_user? ||
-          User.current.allowed_to?(:force_file_unlock, @project))
       @allowed = User.current.allowed_to? :file_manipulation, @project
+      @unlockable = @allowed && @dmsf_file.unlockable? && (!@dmsf_file.locked_for_user? ||
+          User.current.allowed_to?(:force_file_unlock, @project))
       @email_allowed = User.current.allowed_to?(:email_documents, @project)
     elsif @dmsf_folder
       @locked = @dmsf_folder.locked?
-      @unlockable = @dmsf_folder.unlockable? && (!@dmsf_folder.locked_for_user?) &&
-          User.current.allowed_to?(:force_file_unlock, @project)
       @allowed = User.current.allowed_to?(:folder_manipulation, @project)
+      @unlockable = @allowed && @dmsf_folder.unlockable? && (!@dmsf_folder.locked_for_user?) &&
+          User.current.allowed_to?(:force_file_unlock, @project)
       @email_allowed = User.current.allowed_to?(:email_documents, @project)
     elsif @dmsf_link
       @allowed = User.current.allowed_to? :file_manipulation, @project
diff --git a/app/controllers/dmsf_upload_controller.rb b/app/controllers/dmsf_upload_controller.rb
index a71947aa..335ac516 100644
--- a/app/controllers/dmsf_upload_controller.rb
+++ b/app/controllers/dmsf_upload_controller.rb
@@ -58,6 +58,9 @@ class DmsfUploadController < ApplicationController
         end
       end
     end
+    if @uploads.empty?
+      flash.now[:error] = l(:label_attachment) + ' ' + l('activerecord.errors.messages.invalid')
+    end
   end
 
   # async single file upload handling
diff --git a/app/views/dmsf_upload/upload_files.html.erb b/app/views/dmsf_upload/upload_files.html.erb
index 40badae5..e1834eac 100644
--- a/app/views/dmsf_upload/upload_files.html.erb
+++ b/app/views/dmsf_upload/upload_files.html.erb
@@ -24,8 +24,8 @@
 
 <% html_title l(:dmsf) %>
 
-<%= render(:partial => '/dmsf/path', 
-  :locals => {:folder => @folder, :filename => nil, :title => l(:heading_uploaded_files)}) %>
+<%= render(partial: '/dmsf/path',
+  locals: { folder: @folder, filename: nil, title: l(:heading_uploaded_files) }) %>
 
 <% if (@folder && @folder.description.present?) || @project.dmsf_description.present? %>
   <div class="wiki">
@@ -33,16 +33,18 @@
   </div>
 <% end %>
 
-<%= form_tag({:action => 'commit_files', :id => @project, :folder_id => @folder},
-  :method => :post) do %>  
-  <% @uploads.each_with_index do |upload, i| %>
-    <% if upload.locked %>
-      <%= render(:partial => 'upload_file_locked', :locals => {:upload => upload, :i => i}) %>
-    <% else %>
-      <%= render(:partial => 'upload_file', :locals => {:upload => upload, :i => i}) %>
+<% unless@uploads.empty? %>
+  <%= form_tag({ action: 'commit_files', id: @project, folder_id: @folder },
+    method: :post) do %>
+    <% @uploads.each_with_index do |upload, i| %>
+      <% if upload.locked %>
+        <%= render(partial: 'upload_file_locked', locals: { upload: upload, i: i }) %>
+      <% else %>
+        <%= render(partial: 'upload_file', locals: { upload: upload, i: i }) %>
+      <% end %>
     <% end %>
+    <div class="form-actions">
+      <%= submit_tag l(:submit_commit), class: 'button-positive' %>
+    </div>
   <% end %>
-  <div class="form-actions">
-    <%= submit_tag l(:submit_commit), :class => 'button-positive' %>
-  </div>
 <% end %>
\ No newline at end of file
diff --git a/lib/redmine_dmsf/webdav/base_resource.rb b/lib/redmine_dmsf/webdav/base_resource.rb
index f0ba3846..5def382f 100644
--- a/lib/redmine_dmsf/webdav/base_resource.rb
+++ b/lib/redmine_dmsf/webdav/base_resource.rb
@@ -86,7 +86,7 @@ module RedmineDmsf
       
       def child_project(p)
         project_display_name = ProjectResource.create_project_name(p)
-        new_path = +@path
+        new_path = @path
         new_path = new_path + '/' unless new_path[-1,1] == '/'
         new_path = '/' + new_path unless new_path[0,1] == '/'
         new_path += project_display_name
diff --git a/lib/redmine_dmsf/webdav/custom_middleware.rb b/lib/redmine_dmsf/webdav/custom_middleware.rb
index 89287a15..ec4b0bfd 100644
--- a/lib/redmine_dmsf/webdav/custom_middleware.rb
+++ b/lib/redmine_dmsf/webdav/custom_middleware.rb
@@ -57,20 +57,7 @@ module RedmineDmsf
         # If the URL map generated by Rack::Builder did not find a matching path,
         # it will return a 404 along with the X-Cascade header set to 'pass'.
         if (status == 404) && (headers['X-Cascade'] == 'pass')
-          # The MS web redirector webdav client likes to go up a level and try
-          # OPTIONS and PROPFIND there. We catch that here and respond telling it that just
-          # plain HTTP is going on.
-          if %w(/ /dmsf).include?(env['PATH_INFO'])
-            if 'OPTIONS'.casecmp(env['REQUEST_METHOD']) == 0
-              [ '200', { 'Allow' => 'OPTIONS,HEAD,GET,PUT,POST,DELETE' }, [''] ]
-            elsif 'PROPFIND'.casecmp(env['REQUEST_METHOD']) == 0
-              [ '404', {}, [''] ]
-            else
-              @rails_app.call env # let Rails handle the request
-            end
-          else
-            @rails_app.call env # let Rails handle the request
-          end
+          @rails_app.call env # Let Rails handle the request
         else
           [status, headers, body]
         end
diff --git a/lib/redmine_dmsf/webdav/dmsf_resource.rb b/lib/redmine_dmsf/webdav/dmsf_resource.rb
index 5b4c3470..0c0f5e59 100644
--- a/lib/redmine_dmsf/webdav/dmsf_resource.rb
+++ b/lib/redmine_dmsf/webdav/dmsf_resource.rb
@@ -71,15 +71,14 @@ module RedmineDmsf
 
       # Is this entity a folder?
       def collection?
-        folder.present? # No need to check if entity exists, as false is returned if entity does not exist anyways
+        !folder.nil?
       end
 
       # Check if current entity is a folder and return DmsfFolder object if found (nil if not)
       def folder
         unless @folder
-          return nil unless project
-          @folder = DmsfFolder.visible.where(:project_id => project.id, :title => basename,
-            :dmsf_folder_id => parent.folder ? parent.folder.id : nil).first
+          @folder = DmsfFolder.visible.where(project_id: project.id, title: basename,
+            dmsf_folder_id: parent.folder ? parent.folder.id : nil).first if project
         end
         @folder
       end
diff --git a/lib/redmine_dmsf/webdav/project_resource.rb b/lib/redmine_dmsf/webdav/project_resource.rb
index f7840ad7..2a099119 100644
--- a/lib/redmine_dmsf/webdav/project_resource.rb
+++ b/lib/redmine_dmsf/webdav/project_resource.rb
@@ -46,15 +46,11 @@ module RedmineDmsf
       end      
 
       def exist?
-        return false if (project.nil? || User.current.anonymous?)                        
-        return false unless project.module_enabled?('dmsf')        
-        User.current.admin? || User.current.allowed_to?(:view_dmsf_folders, project)
+        really_exist? && User.current.allowed_to?(:view_dmsf_folders, project)
       end
 
       def really_exist?
-        return false if project.nil?
-        return false unless project.module_enabled?('dmsf')    
-        true
+        project && project.module_enabled?('dmsf')
       end
 
       def collection?
@@ -62,11 +58,11 @@ module RedmineDmsf
       end
 
       def creation_date
-        project.created_on unless project.nil?
+        project.created_on if project
       end
 
       def last_modified
-        project.updated_on unless project.nil?
+        project.updated_on if project
       end
 
       def etag
@@ -78,7 +74,7 @@ module RedmineDmsf
       end
 
       def long_name
-        project.name unless project.nil?
+        project.name if project
       end
 
       def content_type
diff --git a/test/fixtures/dmsf_links.yml b/test/fixtures/dmsf_links.yml
index 69fbce27..b9729b66 100644
--- a/test/fixtures/dmsf_links.yml
+++ b/test/fixtures/dmsf_links.yml
@@ -54,7 +54,6 @@ file_link2:
 url_link:
   id: 5
   target_project_id: 1
-  target_id: 4
   target_type: DmsfUrl
   name: url_link
   project_id: 1
@@ -62,4 +61,17 @@ url_link:
   deleted: 0
   deleted_by_user_id: NULL
   created_at: <%= DateTime.current %>
+  updated_at: <%= DateTime.current %>
+
+file_link3:
+  id: 6
+  target_project_id: 1
+  target_id: 1
+  target_type: DmsfFile
+  name: file1_link
+  project_id: 1
+  dmsf_folder_id: NULL
+  deleted: 0
+  deleted_by_user_id: NULL
+  created_at: <%= DateTime.current %>
   updated_at: <%= DateTime.current %>
\ No newline at end of file
diff --git a/test/functional/dmsf_context_menus_controller_test.rb b/test/functional/dmsf_context_menus_controller_test.rb
index a215894a..4e583ad4 100644
--- a/test/functional/dmsf_context_menus_controller_test.rb
+++ b/test/functional/dmsf_context_menus_controller_test.rb
@@ -36,6 +36,7 @@ class DmsfContextMenusControllerTest < RedmineDmsf::Test::TestCase
     @folder1 = DmsfFolder.find 1
     @folder5 = DmsfFolder.find 5
     @file_link2 = DmsfLink.find 2
+    @file_link6 = DmsfLink.find 6
     @folder_link1 = DmsfLink.find 1
     @url_link5 = DmsfLink.find 5
     User.current = nil
@@ -51,6 +52,7 @@ class DmsfContextMenusControllerTest < RedmineDmsf::Test::TestCase
     assert_kind_of DmsfFolder, @folder1
     assert_kind_of DmsfFolder, @folder5
     assert_kind_of DmsfLink, @file_link2
+    assert_kind_of DmsfLink, @file_link6
     assert_kind_of DmsfLink, @folder_link1
     assert_kind_of DmsfLink, @url_link5
     assert_kind_of Role, @role1
@@ -160,7 +162,7 @@ class DmsfContextMenusControllerTest < RedmineDmsf::Test::TestCase
 
   def test_dmsf_file_link
     get :dmsf, params: {
-        id: @file_link2.project.id, folder_id: @file_link2.dmsf_folder.id, ids: ["file-link-#{@file_link2.id}"] }
+        id: @file_link6.project.id, folder_id: @file_link6.dmsf_folder, ids: ["file-link-#{@file_link6.id}"] }
     assert_response :success
     assert_select 'a.icon-edit', text: l(:button_edit)
     assert_select 'a.icon-lock', text: l(:button_lock)
@@ -172,7 +174,7 @@ class DmsfContextMenusControllerTest < RedmineDmsf::Test::TestCase
   end
 
   def test_dmsf_file_link_locked
-    @file_link2.target_file.lock!
+    assert @file_link2.target_file.locked?
     get :dmsf, params: {
         id: @file_link2.project.id, folder_id: @file_link2.dmsf_folder.id, ids: ["file-link-#{@file_link2.id}"] }
     assert_response :success
@@ -190,7 +192,7 @@ class DmsfContextMenusControllerTest < RedmineDmsf::Test::TestCase
   end
 
   def test_dmsf_folder
-    get :dmsf, params: { id: @folder5.project.id, ids: ["folder-#{@folder5.id}"] }
+    get :dmsf, params: { id: @folder1.project.id, ids: ["folder-#{@folder1.id}"] }
     assert_response :success
     assert_select 'a.icon-edit', text: l(:button_edit)
     assert_select 'a.icon-lock', text: l(:button_lock)
@@ -208,7 +210,7 @@ class DmsfContextMenusControllerTest < RedmineDmsf::Test::TestCase
   end
 
   def test_dmsf_folder_locked
-    @folder5.lock!
+    assert @folder5.locked?
     get :dmsf, params: { id: @folder5.project.id, ids: ["folder-#{@folder5.id}"] }
     assert_response :success
     assert_select 'a.icon-edit.disabled', text: l(:button_edit)
@@ -228,7 +230,7 @@ class DmsfContextMenusControllerTest < RedmineDmsf::Test::TestCase
 
   def test_dmsf_folder_manipulation_permmissions_off
     @role1.remove_permission! :folder_manipulation
-    get :dmsf, params: { id: @folder5.project.id, ids: ["folder-#{@folder5.id}"] }
+    get :dmsf, params: { id: @folder1.project.id, ids: ["folder-#{@folder1.id}"] }
     assert_response :success
     assert_select 'a.icon-edit.disabled', text: l(:button_edit)
     assert_select 'a.icon-lock.disabled', text: l(:button_lock)
@@ -238,7 +240,8 @@ class DmsfContextMenusControllerTest < RedmineDmsf::Test::TestCase
 
   def test_dmsf_folder_manipulation_permmissions_on
     @role1.add_permission! :folder_manipulation
-    get :dmsf, params: { id: @folder5.project.id, ids: ["folder-#{@folder5.id}"] }
+    #assert !@folder5.locked?
+    get :dmsf, params: { id: @folder1.project.id, ids: ["folder-#{@folder1.id}"] }
     assert_response :success
     assert_select 'a:not(icon-edit.disabled)', text: l(:button_edit)
     assert_select 'a:not(icon-lock.disabled)', text: l(:button_lock)
diff --git a/test/integration/rest_api/dmsf_file_api_test.rb b/test/integration/rest_api/dmsf_file_api_test.rb
index 7e6775ee..33d3da1b 100644
--- a/test/integration/rest_api/dmsf_file_api_test.rb
+++ b/test/integration/rest_api/dmsf_file_api_test.rb
@@ -118,7 +118,6 @@ class DmsfFileApiTest < RedmineDmsf::Test::IntegrationTest
     #     </dmsf_file_revision>
     #   </dmsf_file_revisions>
     # </dmsf_file>
-    #puts response.body
     assert_select 'dmsf_file > id', text: @file1.id.to_s
     assert_select 'dmsf_file > title', text: @file1.title
     assert_select 'dmsf_file > name', text: @file1.name
diff --git a/test/integration/webdav/dmsf_webdav_custom_middleware_test.rb b/test/integration/webdav/dmsf_webdav_custom_middleware_test.rb
index a834bdaa..359c4428 100644
--- a/test/integration/webdav/dmsf_webdav_custom_middleware_test.rb
+++ b/test/integration/webdav/dmsf_webdav_custom_middleware_test.rb
@@ -34,12 +34,12 @@ class DmsfWebdavCustomMiddlewareTest < RedmineDmsf::Test::IntegrationTest
 
   def test_options_for_root_path
     process :options, '/'
-    assert_response :success
+    assert_response :not_found
   end
 
   def test_options_for_dmsf_root_path
     process :options, '/dmsf'
-    assert_response :success
+    assert_response :not_found
   end
 
   def test_propfind_for_root_path
diff --git a/test/unit/project_patch_test.rb b/test/unit/project_patch_test.rb
index 5214e460..34c49055 100644
--- a/test/unit/project_patch_test.rb
+++ b/test/unit/project_patch_test.rb
@@ -81,7 +81,7 @@ class ProjectPatchTest < RedmineDmsf::Test::UnitTest
   def test_copy_dmsf
     assert_equal 4, @project1.dmsf_files.visible.all.size
     assert_equal 4, @project1.dmsf_folders.visible.all.size
-    assert_equal 1, @project1.file_links.visible.all.size
+    assert_equal 2, @project1.file_links.visible.all.size
     assert_equal 1, @project1.folder_links.visible.all.size
     assert_equal 1, @project1.url_links.visible.all.size
     assert_equal 0, @project3.dmsf_files.visible.all.size
@@ -92,7 +92,7 @@ class ProjectPatchTest < RedmineDmsf::Test::UnitTest
     @project3.copy_dmsf(@project1)
     assert_equal 4, @project3.dmsf_files.visible.all.size
     assert_equal 4, @project3.dmsf_folders.all.size
-    assert_equal 1, @project3.file_links.visible.all.size
+    assert_equal 2, @project3.file_links.visible.all.size
     assert_equal 1, @project3.folder_links.visible.all.size
     assert_equal 1, @project3.url_links.visible.all.size
   end