Merge branch 'pl-remove-virtus' into 'master'

Prefer ActiveModel's attributes over Virtus

See merge request gitlab-org/gitlab-ce!31835

6 files changed, 46 insertions, 37 deletions

diff --git a/Gemfile b/Gemfile
index 59aa67e642e..116c94bdaf8 100644
--- a/Gemfile
+++ b/Gemfile
@@ -275,7 +275,6 @@ gem 'font-awesome-rails', '~> 4.7'
 gem 'gemojione', '~> 3.3'
 gem 'gon', '~> 6.2'
 gem 'request_store', '~> 1.3'
-gem 'virtus', '~> 1.0.1'
 gem 'base32', '~> 0.3.0'
 
 # Sentry integration
diff --git a/Gemfile.lock b/Gemfile.lock
index d8f7ca994da..0ecf3aa2840 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -1252,7 +1252,6 @@ DEPENDENCIES
   unicorn-worker-killer (~> 0.4.4)
   validates_hostname (~> 1.0.6)
   version_sorter (~> 2.2.4)
-  virtus (~> 1.0.1)
   vmstat (~> 2.3.0)
   webmock (~> 3.5.1)
   webpack-rails (~> 0.9.10)
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index b04ffe80db4..4125f44d00a 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -92,7 +92,7 @@ class Projects::BlobController < Projects::ApplicationController
   def diff
     apply_diff_view_cookie!
 
-    @form = Blobs::UnfoldPresenter.new(blob, params.to_unsafe_h)
+    @form = Blobs::UnfoldPresenter.new(blob, diff_params)
 
     # keep only json rendering when
     # https://gitlab.com/gitlab-org/gitlab-ce/issues/44988 is done
@@ -239,4 +239,8 @@ class Projects::BlobController < Projects::ApplicationController
   def tree_path
     @path.rpartition('/').first
   end
+
+  def diff_params
+    params.permit(:full, :since, :to, :bottom, :unfold, :offset, :indent)
+  end
 end
diff --git a/app/presenters/blobs/unfold_presenter.rb b/app/presenters/blobs/unfold_presenter.rb
index 4c6dd6895cf..a256dd05a4d 100644
--- a/app/presenters/blobs/unfold_presenter.rb
+++ b/app/presenters/blobs/unfold_presenter.rb
@@ -1,30 +1,34 @@
 # frozen_string_literal: true
 
-require 'gt_one_coercion'
-
 module Blobs
   class UnfoldPresenter < BlobPresenter
-    include Virtus.model
+    include ActiveModel::Attributes
+    include ActiveModel::AttributeAssignment
     include Gitlab::Utils::StrongMemoize
 
-    attribute :full, Boolean, default: false
-    attribute :since, GtOneCoercion
-    attribute :to, Integer
-    attribute :bottom, Boolean
-    attribute :unfold, Boolean, default: true
-    attribute :offset, Integer
-    attribute :indent, Integer, default: 0
+    attribute :full, :boolean, default: false
+    attribute :since, :integer, default: 1
+    attribute :to, :integer, default: 1
+    attribute :bottom, :boolean, default: false
+    attribute :unfold, :boolean, default: true
+    attribute :offset, :integer, default: 0
+    attribute :indent, :integer, default: 0
+
+    alias_method :full?, :full
+    alias_method :bottom?, :bottom
+    alias_method :unfold?, :unfold
 
     def initialize(blob, params)
+      super(blob)
+      self.attributes = params
+
       # Load all blob data first as we need to ensure they're all loaded first
       # so we can accurately show the rest of the diff when unfolding.
       load_all_blob_data
 
-      @subject = blob
       @all_lines = blob.data.lines
-      super(params)
 
-      self.attributes = prepare_attributes
+      handle_full_or_end!
     end
 
     # Returns an array of Gitlab::Diff::Line with match line added
@@ -56,21 +60,18 @@ module Blobs
 
     private
 
-    def prepare_attributes
-      return attributes unless full? || to == -1
+    def handle_full_or_end!
+      return unless full? || to == -1
 
-      full_opts = {
-        since: 1,
+      self.since = 1 if full?
+
+      self.attributes = {
         to: all_lines_size,
         bottom: false,
         unfold: false,
         offset: 0,
         indent: 0
       }
-
-      return full_opts if full?
-
-      full_opts.merge(attributes.slice(:since))
     end
 
     def all_lines_size
diff --git a/lib/gt_one_coercion.rb b/lib/gt_one_coercion.rb
deleted file mode 100644
index 99be51bc8c6..00000000000
--- a/lib/gt_one_coercion.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-# frozen_string_literal: true
-
-class GtOneCoercion < Virtus::Attribute
-  def coerce(value)
-    [1, value.to_i].max
-  end
-end
diff --git a/spec/presenters/blobs/unfold_presenter_spec.rb b/spec/presenters/blobs/unfold_presenter_spec.rb
index ab3f8080257..83004809536 100644
--- a/spec/presenters/blobs/unfold_presenter_spec.rb
+++ b/spec/presenters/blobs/unfold_presenter_spec.rb
@@ -10,16 +10,31 @@ describe Blobs::UnfoldPresenter do
   let(:subject) { described_class.new(blob, params) }
 
   describe '#initialize' do
+    let(:result) { subject }
+
+    context 'with empty params' do
+      let(:params) { {} }
+
+      it 'sets default attributes' do
+        expect(result.full?).to eq(false)
+        expect(result.since).to eq(1)
+        expect(result.to).to eq(1)
+        expect(result.bottom).to eq(false)
+        expect(result.unfold).to eq(true)
+        expect(result.offset).to eq(0)
+        expect(result.indent).to eq(0)
+      end
+    end
+
     context 'when full is false' do
       let(:params) { { full: false, since: 2, to: 3, bottom: false, offset: 1, indent: 1 } }
 
       it 'sets attributes' do
-        result = subject
-
         expect(result.full?).to eq(false)
         expect(result.since).to eq(2)
         expect(result.to).to eq(3)
         expect(result.bottom).to eq(false)
+        expect(result.unfold).to eq(true)
         expect(result.offset).to eq(1)
         expect(result.indent).to eq(1)
       end
@@ -29,12 +44,11 @@ describe Blobs::UnfoldPresenter do
       let(:params) { { full: true, since: 2, to: 3, bottom: false, offset: 1, indent: 1 } }
 
       it 'sets other attributes' do
-        result = subject
-
         expect(result.full?).to eq(true)
         expect(result.since).to eq(1)
         expect(result.to).to eq(blob.lines.size)
         expect(result.bottom).to eq(false)
+        expect(result.unfold).to eq(false)
         expect(result.offset).to eq(0)
         expect(result.indent).to eq(0)
       end
@@ -44,12 +58,11 @@ describe Blobs::UnfoldPresenter do
       let(:params) { { full: false, since: 2, to: -1, bottom: true, offset: 1, indent: 1 } }
 
       it 'sets other attributes' do
-        result = subject
-
         expect(result.full?).to eq(false)
         expect(result.since).to eq(2)
         expect(result.to).to eq(blob.lines.size)
         expect(result.bottom).to eq(false)
+        expect(result.unfold).to eq(false)
         expect(result.offset).to eq(0)
         expect(result.indent).to eq(0)
       end