spread attribute storage over bcf_issue and wp

modules/bcf/app/contracts/bcf/issues/base_contract.rb

@@ -32,6 +32,9 @@ module Bcf::Issues
   class BaseContract < ::ModelContract
     attribute :uuid
     attribute :work_package
+    attribute :stage
+    attribute :labels
+    attribute :index
 
     def validate
       validate_user_allowed_to_manage

modules/bcf/app/controllers/bcf/api/v2_1/topics_api.rb

@@ -52,7 +52,17 @@ module Bcf::API::V2_1
                   api_name: 'Topics',
                   params_modifier: ->(attributes) {
                     attributes[:project_id] = @project.id
+
+                    wp_attributes = Bcf::Issues::TransformAttributesService
+                                    .new
+                                    .call(attributes)
+                                    .result
+
                     attributes
+                      .slice(:stage,
+                             :index,
+                             :labels)
+                      .merge(wp_attributes)
                   })
              .mount
 

modules/bcf/app/models/bcf/issue.rb

@@ -11,6 +11,8 @@ module Bcf
 
     validates :work_package, presence: true
 
+    # TODO: remove xml extraction as properties are either stored
+    # in specific columns of the bcf_issues table or in the referenced work package
     class << self
       def with_markup
         select '*',
@@ -123,10 +125,6 @@ module Bcf
       end
     end
 
-    def labels
-      from_attributes_or_doc :labels, multiple: true
-    end
-
     def markup_doc
       @markup_doc ||= Nokogiri::XML markup, nil, 'UTF-8'
     end

modules/bcf/app/representers/bcf/api/v2_1/topics/single_representer.rb

@@ -35,45 +35,99 @@ module Bcf::API::V2_1
     property :uuid,
              as: :guid
 
-    property :type_text,
-             as: :topic_type
+    property :type,
+             as: :topic_type,
+             getter: ->(*) {
+               work_package
+                 .type
+                 .name
+             }
 
-    property :status_text,
-             as: :topic_status
+    property :status,
+             as: :topic_status,
+             getter: ->(*) {
+               work_package
+                .status
+                .name
+             }
 
     property :reference_links,
              getter: ->(decorator:, **) {
                [decorator.api_v3_paths.work_package(work_package.id)]
              }
 
-    property :title
+    property :title,
+             getter: ->(*) {
+               work_package.subject
+             }
 
-    property :index_text,
-             as: :index
+    property :index
 
     property :labels
 
     property :creation_date_text,
-             as: :creation_date
+             as: :creation_date,
+             getter: ->(decorator:, **) {
+               decorator
+                 .formatted_date_time(:created_at)
+             }
 
     property :creation_author_text,
-             as: :creation_author
+             as: :creation_author,
+             getter: ->(*) {
+               work_package
+                 .author
+                 .mail
+             }
 
     property :modified_date_text,
-             as: :modified_date
+             as: :modified_date,
+             getter: ->(decorator:, **) {
+               decorator
+                 .formatted_date_time(:updated_at)
+             }
 
     property :modified_author_text,
-             as: :modified_author
+             as: :modified_author,
+             getter: ->(*) {
+               work_package
+                 .journals
+                 .max_by(&:version)
+                 .user
+                 .mail
+             }
 
-    property :assignee_text,
-             as: :assigned_to
+    property :assignee,
+             as: :assigned_to,
+             getter: ->(decorator:, **) {
+               decorator
+                 .assigned_to
+                 &.mail
+             }
 
-    property :stage_text,
-             as: :stage
+    property :stage
 
-    property :description
+    property :description,
+             getter: ->(*) {
+               work_package.description
+             }
 
     property :due_date_text,
              as: :due_date
+
+    def datetime_formatter
+      ::API::V3::Utilities::DateTimeFormatter
+    end
+
+    def formatted_date_time(method)
+      datetime_formatter
+        .format_datetime(represented.work_package.send(method), allow_nil: true)
+    end
+
+    def assigned_to
+      represented
+        .work_package
+        .assigned_to
+    end
   end
 end

modules/bcf/app/services/bcf/issues/create_service.rb

@@ -33,13 +33,13 @@ module Bcf::Issues
     private
 
     def before_perform(params)
-      wp_call = Bcf::WorkPackages::CreateService
+      wp_call = WorkPackages::CreateService
                 .new(user: user)
                 .call(params)
 
       issue_params = {
         work_package: wp_call.result
-      }
+      }.merge(params.slice(:stage, :labels, :index))
 
       super(issue_params)
     end

modules/bcf/app/services/bcf/issues/transform_attributes_service.rb

@@ -28,14 +28,15 @@
 # See doc/COPYRIGHT.rdoc for more details.
 #++
 
-module Bcf::WorkPackages
-  class SetAttributesService < ::WorkPackages::SetAttributesService
-    private
-
-    def set_attributes(attributes)
-      super(work_package_attributes(attributes))
+module Bcf::Issues
+  class TransformAttributesService
+    def call(attributes)
+      ServiceResult.new success: true,
+                        result: work_package_attributes(attributes)
     end
 
+    private
+
     def author(project, attributes)
       find_user_in_project(project, attributes[:author]) || User.system
     end
@@ -70,10 +71,6 @@ module Bcf::WorkPackages
       end
     end
 
-    def start_date(attributes)
-      extractor.creation_date.to_date unless is_update
-    end
-
     ##
     # Get mapped and raw attributes from MarkupExtractor
     # and return all values that are non-nil

modules/bcf/app/services/bcf/work_packages/create_service.rb

@@ -1,39 +0,0 @@
-#-- encoding: UTF-8
-
-#-- copyright
-# OpenProject is a project management system.
-# Copyright (C) 2012-2019 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-2017 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.
-#++
-
-module Bcf::WorkPackages
-  class CreateService < ::WorkPackages::CreateService
-    private
-
-    def attributes_service_class
-      ::Bcf::WorkPackages::SetAttributesService
-    end
-  end
-end

modules/bcf/db/migrate/20191119144123_add_issue_columns.rb

@@ -0,0 +1,9 @@
+class AddIssueColumns < ActiveRecord::Migration[6.0]
+  def change
+    change_table :bcf_issues do |i|
+      i.string :stage
+      i.integer :index
+      i.text :labels, array: true, default: []
+    end
+  end
+end

modules/bcf/spec/contracts/bcf/issues/create_contract_spec.rb

@@ -33,15 +33,12 @@ describe Bcf::Issues::CreateContract do
   it_behaves_like 'issues contract' do
     let(:issue) do
       Bcf::Issue.new(uuid: issue_uuid,
-                     work_package: issue_work_package)
+                     work_package: issue_work_package,
+                     stage: issue_stage,
+                     index: issue_index,
+                     labels: issue_labels)
     end
     let(:permissions) { [:manage_bcf] }
-    let!(:allowed_to) do
-      allow(current_user)
-        .to receive(:allowed_to?) do |permission, project|
-        permissions.include?(permission) && project == project
-      end
-    end
 
     subject(:contract) { described_class.new(issue, current_user) }
   end

modules/bcf/spec/contracts/bcf/issues/shared_contract_examples.rb

@@ -51,6 +51,9 @@ shared_examples_for 'issues contract' do
 
     id
   end
+  let(:issue_stage) { 'some stage' }
+  let(:issue_labels) { %w(some labels) }
+  let(:issue_index) { 8 }
 
   before do
     allow(issue)
@@ -95,4 +98,22 @@ shared_examples_for 'issues contract' do
       expect_valid(false, base: %i(error_unauthorized))
     end
   end
+
+  context 'if the stage is nil' do
+    let(:issue_stage) { nil }
+
+    it_behaves_like 'is valid'
+  end
+
+  context 'if labels is nil' do
+    let(:issue_labels) { nil }
+
+    it_behaves_like 'is valid'
+  end
+
+  context 'if index is nil' do
+    let(:issue_index) { nil }
+
+    it_behaves_like 'is valid'
+  end
 end

modules/bcf/spec/factories/bcf_issue_factory.rb

@@ -109,6 +109,9 @@ FactoryBot.define do
     </Markup>
       MARKUP
     end
+    stage { 'Construction stop' }
+    labels { ['Structural', 'IT Development', 'Huge'] }
+    sequence(:index) { |n| n }
 
     factory :bcf_issue_with_comment do
       after(:create) do |issue|

modules/bcf/spec/representers/bcf/api/v2_1/topics/single_representer_rendering_spec.rb

@@ -33,7 +33,24 @@ require_relative '../shared_examples'
 describe Bcf::API::V2_1::Topics::SingleRepresenter, 'rendering' do
   include API::V3::Utilities::PathHelper
 
-  let(:work_package) { FactoryBot.build_stubbed(:stubbed_work_package, type: FactoryBot.build_stubbed(:type)) }
+  let(:assignee) { FactoryBot.build_stubbed(:user) }
+  let(:creator) { FactoryBot.build_stubbed(:user) }
+  let(:modifier) { FactoryBot.build_stubbed(:user) }
+  let(:first_journal) { FactoryBot.build_stubbed(:journal, version: 1, user: creator) }
+  let(:last_journal) { FactoryBot.build_stubbed(:journal, version: 2, user: modifier) }
+  let(:journals) { [first_journal, last_journal] }
+  let(:type) { FactoryBot.build_stubbed(:type) }
+  let(:status) { FactoryBot.build_stubbed(:status) }
+  let(:work_package) do
+    FactoryBot.build_stubbed(:stubbed_work_package,
+                             assigned_to: assignee,
+                             status: status,
+                             type: type).tap do |wp|
+      allow(wp)
+        .to receive(:journals)
+        .and_return(journals)
+    end
+  end
   let(:issue) { FactoryBot.build_stubbed(:bcf_issue, work_package: work_package) }
 
   let(:instance) { described_class.new(issue) }
@@ -50,14 +67,14 @@ describe Bcf::API::V2_1::Topics::SingleRepresenter, 'rendering' do
 
     context 'topic_type' do
       it_behaves_like 'attribute' do
-        let(:value) { issue.type_text }
+        let(:value) { type.name }
         let(:path) { 'topic_type' }
       end
     end
 
     context 'topic_status' do
       it_behaves_like 'attribute' do
-        let(:value) { issue.status_text }
+        let(:value) { status.name }
         let(:path) { 'topic_status' }
       end
     end
@@ -71,14 +88,14 @@ describe Bcf::API::V2_1::Topics::SingleRepresenter, 'rendering' do
 
     context 'title' do
       it_behaves_like 'attribute' do
-        let(:value) { issue.title }
+        let(:value) { work_package.subject }
         let(:path) { 'title' }
       end
     end
 
     context 'index' do
       it_behaves_like 'attribute' do
-        let(:value) { issue.index_text }
+        let(:value) { issue.index }
         let(:path) { 'index' }
       end
     end
@@ -92,35 +109,35 @@ describe Bcf::API::V2_1::Topics::SingleRepresenter, 'rendering' do
 
     context 'creation_date' do
       it_behaves_like 'attribute' do
-        let(:value) { issue.creation_date_text }
+        let(:value) { work_package.created_at.iso8601 }
         let(:path) { 'creation_date' }
       end
     end
 
     context 'creation_author' do
       it_behaves_like 'attribute' do
-        let(:value) { issue.creation_author_text }
+        let(:value) { work_package.author.mail }
         let(:path) { 'creation_author' }
       end
     end
 
     context 'modified_date' do
       it_behaves_like 'attribute' do
-        let(:value) { issue.modified_date_text }
+        let(:value) { work_package.updated_at.iso8601 }
         let(:path) { 'modified_date' }
       end
     end
 
     context 'modified_author' do
       it_behaves_like 'attribute' do
-        let(:value) { issue.modified_author_text }
+        let(:value) { modifier.mail }
         let(:path) { 'modified_author' }
       end
     end
 
     context 'description' do
       it_behaves_like 'attribute' do
-        let(:value) { issue.description }
+        let(:value) { work_package.description }
         let(:path) { 'description' }
       end
     end
@@ -134,14 +151,14 @@ describe Bcf::API::V2_1::Topics::SingleRepresenter, 'rendering' do
 
     context 'assigned_to' do
       it_behaves_like 'attribute' do
-        let(:value) { issue.assignee_text }
+        let(:value) { work_package.assigned_to.mail }
         let(:path) { 'assigned_to' }
       end
     end
 
     context 'stage' do
       it_behaves_like 'attribute' do
-        let(:value) { issue.stage_text }
+        let(:value) { issue.stage }
         let(:path) { 'stage' }
       end
     end

modules/bcf/spec/requests/api/bcf/v2_1/topics_api_spec.rb

@@ -58,7 +58,12 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma
     FactoryBot.create(:project,
                       enabled_module_names: %i[bcf work_package_tracking])
   end
-  let(:work_package) { FactoryBot.create(:work_package, project: project) }
+  let(:assignee) { FactoryBot.create(:user) }
+  let(:work_package) do
+    FactoryBot.create(:work_package,
+                      assigned_to: assignee,
+                      project: project)
+  end
   let(:bcf_issue) { FactoryBot.create(:bcf_issue, work_package: work_package) }
 
   subject(:response) { last_response }
@@ -77,26 +82,23 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma
       let(:expected_body) do
         [
           {
-            "assigned_to": "andy@example.com",
-            "creation_author": "mike@example.com",
-            "creation_date": "2015-06-21T12:00:00Z",
-            "description": "This is a topic with all information present.",
+            "assigned_to": assignee.mail,
+            "creation_author": work_package.author.mail,
+            "creation_date": work_package.created_at.iso8601,
+            "description": work_package.description,
             "due_date": nil,
-            guid: bcf_issue.uuid,
-            "index": "0",
-            "labels": [
-              "Structural",
-              "IT Development"
-            ],
-            "modified_author": "michelle@example.com",
-            "modified_date": "2015-06-21T14:22:47Z",
+            "guid": bcf_issue.uuid,
+            "index": bcf_issue.index,
+            "labels": bcf_issue.labels,
+            "modified_author": current_user.mail,
+            "modified_date": work_package.updated_at.iso8601,
             "reference_links": [
               api_v3_paths.work_package(work_package.id)
             ],
-            "stage": "Construction start",
-            "title": "Maximum Content",
-            "topic_status": "Open",
-            "topic_type": "Structural"
+            "stage": bcf_issue.stage,
+            "title": work_package.subject,
+            "topic_status": work_package.status.name,
+            "topic_type": work_package.type.name
           }
         ]
       end
@@ -128,26 +130,23 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma
     it_behaves_like 'bcf api successful response' do
       let(:expected_body) do
         {
-          "assigned_to": "andy@example.com",
-          "creation_author": "mike@example.com",
-          "creation_date": "2015-06-21T12:00:00Z",
-          "description": "This is a topic with all information present.",
+          "assigned_to": assignee.mail,
+          "creation_author": work_package.author.mail,
+          "creation_date": work_package.created_at.iso8601,
+          "description": work_package.description,
           "due_date": nil,
-          guid: bcf_issue.uuid,
-          "index": "0",
-          "labels": [
-            "Structural",
-            "IT Development"
-          ],
-          "modified_author": "michelle@example.com",
-          "modified_date": "2015-06-21T14:22:47Z",
+          "guid": bcf_issue.uuid,
+          "index": bcf_issue.index,
+          "labels": bcf_issue.labels,
+          "modified_author": current_user.mail,
+          "modified_date": work_package.updated_at.iso8601,
           "reference_links": [
             api_v3_paths.work_package(work_package.id)
           ],
-          "stage": "Construction start",
-          "title": "Maximum Content",
-          "topic_status": "Open",
-          "topic_type": "Structural"
+          "stage": bcf_issue.stage,
+          "title": work_package.subject,
+          "topic_status": work_package.status.name,
+          "topic_type": work_package.type.name
         }
       end
     end
@@ -191,13 +190,20 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma
     let!(:default_priority) do
       FactoryBot.create(:default_priority)
     end
+    let(:description) { 'some description' }
+    let(:stage) { 'current stage' }
+    let(:labels) { %w(some labels) }
+    let(:index) { 5 }
     let(:params) do
       {
         topic_type: type.name,
         topic_status: status.name,
         title: 'BCF topic 101',
-        labels: [],
-        assigned_to: view_only_user
+        labels: labels,
+        stage: stage,
+        index: index,
+        assigned_to: view_only_user.mail,
+        description: description
       }
     end
 
@@ -213,15 +219,24 @@ describe 'BCF 2.1 topics resource', type: :request, content_type: :json, with_ma
         work_package = WorkPackage.last
 
         {
-          guid: issue.uuid,
+          guid: issue&.uuid,
           topic_type: type.name,
           topic_status: status.name,
           title: 'BCF topic 101',
-          labels: [],
+          labels: labels,
+          index: index,
           reference_links: [
             api_v3_paths.work_package(work_package.id)
           ],
-          assigned_to: view_only_user.name
+          assigned_to: view_only_user.mail,
+          due_date: nil,
+          stage: stage,
+          creation_author: edit_member_user.mail,
+          creation_date: work_package.created_at.iso8601,
+          modified_author: edit_member_user.mail,
+          modified_date: work_package.updated_at.iso8601,
+          description: description
+
         }
       end
     end