Presenter changes and tests

Changes to presenter splitting into functions
Added tests for the new presenter
Changed to_json to happen inside helper
Modified packages factory to support new tests

ee/app/helpers/ee/packages_helper.rb

@@ -27,7 +27,7 @@ module EE
     def package_from_presenter(package)
       presenter = ::Packages::Detail::PackagePresenter.new(package)
 
-      presenter.detail_view
+      presenter.detail_view.to_json
     end
   end
 end

ee/app/presenters/packages/detail/package_presenter.rb

@@ -11,24 +11,48 @@ module Packages
         package_detail = {
           created_at: @package.created_at,
           name: @package.name,
-          package_files: @package.package_files.as_json(methods: :download_path),
+          package_files: @package.package_files.map { |pf| build_package_file_view(pf) },
           package_type: @package.package_type,
           project_id: @package.project_id,
           tags: @package.tags.as_json,
           updated_at: @package.updated_at,
           version: @package.version,
-          maven_metadatum: @package.maven_metadatum
         }
 
-        if @package.build_info
-          package_detail[:pipeline] = @package.build_info.pipeline.as_json(
-            only: [:created_at, :git_commit_message, :id, :sha],
-            include: [user: { methods: :avatar_url, only: [:avatar_url, :name] }],
-            methods: :git_commit_message
-          )
-        end
+        package_detail[:maven_metadatum] = @package.maven_metadatum if @package.maven_metadatum
+        package_detail[:pipeline] = build_pipeline_info(@package.build_info.pipeline) if @package.build_info
 
-        package_detail.to_json
+        package_detail
+      end
+
+      def build_package_file_view(package_file)
+        {
+          created_at: package_file.created_at,
+          download_path: package_file.download_path,
+          file_name: package_file.file_name,
+          size: package_file.size
+        }
+      end
+
+      def build_pipeline_info(pipeline_info)
+        return unless pipeline_info
+
+        {
+          created_at: pipeline_info.created_at,
+          id: pipeline_info.id,
+          sha: pipeline_info.sha,
+          git_commit_message: pipeline_info.git_commit_message,
+          user: build_user_info(pipeline_info.user)
+        }
+      end
+
+      def build_user_info(user)
+        return unless user
+
+        {
+          avatar_url: user.avatar_url,
+          name: user.name
+        }
       end
     end
   end

ee/spec/factories/packages.rb

@@ -31,7 +31,10 @@ FactoryBot.define do
 
       trait :with_build do
         after :create do |package|
-          create :package_build_info, package: package, pipeline: create(:ci_build, user: package.project.creator).pipeline
+          user = package.project.creator
+          pipeline = create(:ci_pipeline, user: user)
+          create(:ci_build, user: user, pipeline: pipeline)
+          create :package_build_info, package: package, pipeline: pipeline
         end
       end
     end

ee/spec/presenters/packages/detail/package_presenter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::Packages::Detail::PackagePresenter do
+  let_it_be(:user) { create(:user) }
+  let!(:project) { create(:project, creator: user) }
+  let!(:package) { create(:npm_package, :with_build, project: project) }
+  let(:presenter) { described_class.new(package) }
+
+  let_it_be(:user_info) { { name: user.name, avatar_url: user.avatar_url } }
+  let!(:expected_package_files) do
+    npm_file = package.package_files.first
+    [{
+      created_at: npm_file.created_at,
+      download_path: npm_file.download_path,
+      file_name: npm_file.file_name,
+      size: npm_file.size
+    }]
+  end
+  let(:pipeline_info) do
+    pipeline = package.build_info.pipeline
+    {
+      created_at: pipeline.created_at,
+      id: pipeline.id,
+      sha: pipeline.sha,
+      git_commit_message: pipeline.git_commit_message,
+      user: user_info
+    }
+  end
+  let!(:expected_package_details) do
+    {
+      created_at: package.created_at,
+      name: package.name,
+      package_files: expected_package_files,
+      package_type: package.package_type,
+      project_id: package.project_id,
+      tags: package.tags.as_json,
+      updated_at: package.updated_at,
+      version: package.version
+    }
+  end
+
+  context 'detail_view' do
+    context 'with build_info' do
+      let!(:package) { create(:npm_package, :with_build, project: project) }
+
+      it 'returns details with pipeline' do
+        expected_package_details[:pipeline] = pipeline_info
+
+        expect(presenter.detail_view).to eq expected_package_details
+      end
+    end
+
+    context 'without build info' do
+      let!(:package) { create(:npm_package, project: project) }
+
+      it 'returns details without pipeline' do
+        expect(presenter.detail_view).to eq expected_package_details
+      end
+    end
+  end
+
+  it 'build_package_file_view returns correct file data' do
+    package_files_result = package.package_files.map { |pf| presenter.build_package_file_view(pf) }
+
+    expect(package_files_result).to eq expected_package_files
+  end
+
+  context 'build_pipeline_info' do
+    it 'returns correct data when there is pipeline_info' do
+      expect(presenter.build_pipeline_info(package.build_info.pipeline)).to eq pipeline_info
+    end
+
+    it 'returns nil when there is no pipeline_info' do
+      expect(presenter.build_pipeline_info(nil)).to eq nil
+    end
+  end
+
+  context 'build_user_info' do
+    it 'returns correct data when there is a user' do
+      expect(presenter.build_user_info(package.build_info.pipeline.user)).to eq user_info
+    end
+
+    it 'returns nil when there is not a user' do
+      expect(presenter.build_user_info(nil)).to eq nil
+    end
+  end
+end