extract attachment storing into service

- service cares about adding a journal too
- only store the attachment when the journal can be saved (i.e. the WP is valid)

app/services/add_attachment_service.rb

@@ -0,0 +1,58 @@
+#-- copyright
+# OpenProject is a project management system.
+# Copyright (C) 2012-2015 the OpenProject Foundation (OPF)
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See doc/COPYRIGHT.rdoc for more details.
+#++
+
+class AddAttachmentService
+  attr_reader :container, :author
+
+  def initialize(container, author:)
+    @container = container
+    @author = author
+  end
+
+  def add_attachment(uploaded_file:, description:)
+    attachment = Attachment.new(file: uploaded_file,
+                                container: container,
+                                description: description,
+                                author: author)
+    save attachment
+
+    attachment
+  end
+
+  private
+
+  def save(attachment)
+    ActiveRecord::Base.transaction do
+      attachment.save!
+      if container.respond_to? :add_journal
+        container.add_journal author
+        container.save!
+      end
+    end
+  end
+end

lib/api/v3/attachments/attachments_by_work_package_api.rb

@@ -48,16 +48,6 @@ module API
 
               metadata
             end
-
-            def make_attachment(metadata, file)
-              uploaded_file = OpenProject::Files.build_uploaded_file file[:tempfile],
-                                                                     file[:type],
-                                                                     file_name: metadata.file_name
-              Attachment.new(file: uploaded_file,
-                             container: @work_package,
-                             description: metadata.description,
-                             author: current_user)
-            end
           end
 
           get do
@@ -79,9 +69,16 @@ module API
                       I18n.t('api_v3.errors.multipart_body_error'))
             end
 
-            attachment = make_attachment(metadata, file)
-            unless attachment.save
-              raise ::API::Errors::ErrorBase.create_and_merge_errors(attachment.errors)
+            uploaded_file = OpenProject::Files.build_uploaded_file file[:tempfile],
+                                                                   file[:type],
+                                                                   file_name: metadata.file_name
+
+            begin
+              service = AddAttachmentService.new(@work_package, author: current_user)
+              attachment = service.add_attachment uploaded_file: uploaded_file,
+                                                  description: metadata.description
+            rescue ActiveRecord::RecordInvalid => error
+              raise ::API::Errors::ErrorBase.create_and_merge_errors(error.record.errors)
             end
 
             ::API::V3::Attachments::AttachmentRepresenter.new(attachment)