diff --git a/.gitignore b/.gitignore index 03cac973..9c26da50 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,3 @@ /redmine_dmsf.iml +/test/fixtures/files/** +!/test/fixtures/files/2017/** diff --git a/app/models/dmsf_file.rb b/app/models/dmsf_file.rb index be9950b7..2c08dc87 100644 --- a/app/models/dmsf_file.rb +++ b/app/models/dmsf_file.rb @@ -58,7 +58,7 @@ class DmsfFile < ActiveRecord::Base validate :validates_name_uniqueness def validates_name_uniqueness - existing_file = DmsfFile.visible.findn_file_by_name(self.project_id, self.dmsf_folder, self.name) + existing_file = DmsfFile.select(:id).findn_file_by_name(self.project_id, self.dmsf_folder, self.name) errors.add(:name, l('activerecord.errors.messages.taken')) unless (existing_file.nil? || existing_file.id == self.id) end @@ -259,9 +259,8 @@ class DmsfFile < ActiveRecord::Base end def copy_to_filename(project, folder, filename) - source = "#{project.identifier}: #{self.dmsf_path_str}" file = DmsfFile.new - file.dmsf_folder = folder + file.dmsf_folder_id = folder.id if folder file.project_id = project.id file.name = filename file.notification = Setting.plugin_redmine_dmsf['dmsf_default_notifications'].present? @@ -286,7 +285,7 @@ class DmsfFile < ActiveRecord::Base if File.exist? self.last_revision.disk_file FileUtils.cp self.last_revision.disk_file, new_revision.disk_file(false) end - new_revision.comment = l(:comment_copied_from, :source => source) + new_revision.comment = l(:comment_copied_from, :source => "#{project.identifier}: #{self.dmsf_path_str}") new_revision.custom_values = [] self.last_revision.custom_values.each do |cv| v = CustomValue.new @@ -294,9 +293,19 @@ class DmsfFile < ActiveRecord::Base v.value = cv.value new_revision.custom_values << v end - file.delete(true) unless new_revision.save + unless new_revision.save + Rails.logger.error new_revision.errors.full_messages.to_sentence + file.delete(true) + file = nil + else + file.set_last_revision new_revision + end + else + Rails.logger.error file.errors.full_messages.to_sentence + file.delete(true) + file = nil end - return file + file end # To fulfill searchable module expectations diff --git a/lib/redmine_dmsf/webdav/dmsf_resource.rb b/lib/redmine_dmsf/webdav/dmsf_resource.rb index 14aa8509..eee4763e 100644 --- a/lib/redmine_dmsf/webdav/dmsf_resource.rb +++ b/lib/redmine_dmsf/webdav/dmsf_resource.rb @@ -77,7 +77,6 @@ module RedmineDmsf def folder unless @folder return nil unless project - #f = parent.folder @folder = DmsfFolder.visible.where(:project_id => project.id, :title => basename, :dmsf_folder_id => parent.folder ? parent.folder.id : nil).first end @@ -236,6 +235,11 @@ module RedmineDmsf raise Forbidden unless (!parent.exist? || !parent.folder || DmsfFolder.permissions?(parent.folder, false)) if collection? + + if dest.exist? + return overwrite ? NotImplemented : PreconditionFailed + end + # At the moment we don't support cross project destinations return MethodNotImplemented unless (project.id == resource.project.id) raise Forbidden unless User.current.admin? || User.current.allowed_to?(:folder_manipulation, project) @@ -243,7 +247,7 @@ module RedmineDmsf # Current object is a folder, so now we need to figure out information about Destination if dest.exist? - MethodNotAllowed + return overwrite ? NotImplemented : PreconditionFailed else if(parent.projectless_path == '/') #Project root folder.dmsf_folder_id = nil @@ -260,6 +264,7 @@ module RedmineDmsf User.current.allowed_to?(:folder_manipulation, resource.project) if dest.exist? + return PreconditionFailed unless overwrite if (project == resource.project) && file.name.match(/.\.tmp$/i) # Renaming a *.tmp file to an existing file in the same project, probably Office that is saving a file. Rails.logger.info "WebDAV MOVE: #{file.name} -> #{resource.basename} (exists), possible MSOffice rename from .tmp when saving" @@ -290,7 +295,7 @@ module RedmineDmsf else # Files cannot be merged at this point, until a decision is made on how to merge them # ideally, we would merge revision history for both, ensuring the origin file wins with latest revision. - MethodNotAllowed + NotImplemented end else if(parent.projectless_path == '/') #Project root @@ -322,7 +327,7 @@ module RedmineDmsf file.name = resource.basename # Save Changes - (file.last_revision.save! && file.save!) ? Created : PreconditionFailed + (file.last_revision.save && file.save) ? Created : PreconditionFailed end end end @@ -347,10 +352,13 @@ module RedmineDmsf parent = resource.parent raise Forbidden unless (!parent.exist? || !parent.folder || DmsfFolder.permissions?(parent.folder, false)) - if collection? - # Current object is a folder, so now we need to figure out information about Destination - return MethodNotAllowed if(dest.exist?) + if dest.exist? + return overwrite ? NotImplemented : PreconditionFailed + end + return Conflict unless dest.parent.exist? + + if collection? # Permission check if they can manipulate folders and view folders # Can they: # Manipulate folders on destination project :folder_manipulation @@ -370,37 +378,32 @@ module RedmineDmsf return PreconditionFailed if new_folder.nil? || new_folder.id.nil? Created else - if dest.exist? - MethodNotAllowed - # Files cannot be merged at this point, until a decision is made on how to merge them - # ideally, we would merge revision history for both, ensuring the origin file wins with latest revision. + # Permission check if they can manipulate folders and view folders + # Can they: + # Manipulate files on destination project :file_manipulation + # View files on destination project :view_dmsf_files + # View files on the source project :view_dmsf_files + raise Forbidden unless User.current.admin? || + (User.current.allowed_to?(:file_manipulation, resource.project) && + User.current.allowed_to?(:view_dmsf_files, resource.project) && + User.current.allowed_to?(:view_dmsf_files, project)) + + if(parent.projectless_path == '/') #Project root + f = nil else - # Permission check if they can manipulate folders and view folders - # Can they: - # Manipulate files on destination project :file_manipulation - # View files on destination project :view_dmsf_files - # View files on the source project :view_dmsf_files - raise Forbidden unless User.current.admin? || - (User.current.allowed_to?(:file_manipulation, resource.project) && - User.current.allowed_to?(:view_dmsf_files, resource.project) && - User.current.allowed_to?(:view_dmsf_files, project)) - - if(parent.projectless_path == '/') #Project root - f = nil - else - return PreconditionFailed unless parent.exist? && parent.folder - f = parent.folder - end - return PreconditionFailed unless exist? && file - return InternalServerError unless file.copy_to(resource.project, f) - - # Update Revision and names of file [We can link to old physical resource, as it's not changed] - file.last_revision.name = resource.basename if file.last_revision - file.name = resource.basename - - # Save Changes - (file.last_revision.save! && file.save!) ? Created : PreconditionFailed + return PreconditionFailed unless parent.exist? && parent.folder + f = parent.folder end + return PreconditionFailed unless exist? && file + new_file = file.copy_to(resource.project, f) + return InternalServerError unless (new_file && new_file.last_revision) + + # Update Revision and names of file [We can link to old physical resource, as it's not changed] + new_file.last_revision.name = resource.basename + new_file.name = resource.basename + + # Save Changes + (new_file.last_revision.save && new_file.save) ? Created : PreconditionFailed end end diff --git a/test/integration/webdav/dmsf_webdav_move_test.rb b/test/integration/webdav/dmsf_webdav_move_test.rb index f5106625..87ce741b 100644 --- a/test/integration/webdav/dmsf_webdav_move_test.rb +++ b/test/integration/webdav/dmsf_webdav_move_test.rb @@ -200,7 +200,7 @@ class DmsfWebdavMoveTest < RedmineDmsf::Test::IntegrationTest assert_no_difference '@file1.dmsf_file_revisions.count' do xml_http_request :move, "/dmsf/webdav/#{@project1.identifier}/#{@file1.name}", nil, @jsmith.merge!({:destination => "http://www.example.com/dmsf/webdav/#{@project1.identifier}/#{new_name}"}) - assert_response 405 # MethodNotAllowed + assert_response 501 # NotImplemented end end end