Fix logic and spec for views(pre feature)

9 files changed, 156 insertions, 24 deletions

diff --git a/app/presenters/ci/group_variable_presenter.rb b/app/presenters/ci/group_variable_presenter.rb
index 64f56be2502..0d3363970cc 100644
--- a/app/presenters/ci/group_variable_presenter.rb
+++ b/app/presenters/ci/group_variable_presenter.rb
@@ -1,13 +1,17 @@
 module Ci
   class GroupVariablePresenter < Gitlab::View::Presenter::Delegated
-    presents :group_variable
+    presents :variable
 
     def placeholder
       'GROUP_VARIABLE'
     end
 
-    def create_path
-      [group, group_variable]
+    def create_update_path
+      if variable.persisted?
+        group_variable_path(group, variable)
+      else
+        group_variables_path(group)
+      end
     end
 
     def edit_path
diff --git a/app/presenters/ci/variable_presenter.rb b/app/presenters/ci/variable_presenter.rb
index 1a3eb755140..78699a43197 100644
--- a/app/presenters/ci/variable_presenter.rb
+++ b/app/presenters/ci/variable_presenter.rb
@@ -6,8 +6,12 @@ module Ci
       'PROJECT_VARIABLE'
     end
 
-    def create_path
-      [project.namespace.becomes(Namespace), project, variable]
+    def create_update_path
+      if variable.persisted?
+        namespace_project_variable_path(project.namespace, project, variable)
+      else
+        namespace_project_variables_path(project.namespace, project)
+      end
     end
 
     def edit_path
diff --git a/app/views/ci/variables/_form.html.haml b/app/views/ci/variables/_form.html.haml
index 96545f73777..78348e0588d 100644
--- a/app/views/ci/variables/_form.html.haml
+++ b/app/views/ci/variables/_form.html.haml
@@ -1,4 +1,4 @@
-= form_for @variable.create_path do |f|
+= form_for @variable, as: :variable, url: @variable.create_update_path do |f|
   = form_errors(@variable)
 
   .form-group
diff --git a/app/views/ci/variables/_show.html.haml b/app/views/ci/variables/_show.html.haml
new file mode 100644
index 00000000000..2bfb290629d
--- /dev/null
+++ b/app/views/ci/variables/_show.html.haml
@@ -0,0 +1,9 @@
+- page_title "Variables"
+
+.row.prepend-top-default.append-bottom-default
+  .col-lg-3
+    = render "ci/variables/content"
+  .col-lg-9
+    %h5.prepend-top-0
+      Update variable
+    = render "ci/variables/form", btn_text: "Save variable"
diff --git a/app/views/groups/settings/ci_cd/show.html.haml b/app/views/groups/settings/ci_cd/show.html.haml
index d76c9ed4b8e..bf36baf48ab 100644
--- a/app/views/groups/settings/ci_cd/show.html.haml
+++ b/app/views/groups/settings/ci_cd/show.html.haml
@@ -1,3 +1,4 @@
+- page_title "Pipelines"
 = render "groups/settings_head"
 
 = render 'ci/variables/index'
diff --git a/app/views/groups/variables/show.html.haml b/app/views/groups/variables/show.html.haml
index 2bfb290629d..df533952b76 100644
--- a/app/views/groups/variables/show.html.haml
+++ b/app/views/groups/variables/show.html.haml
@@ -1,9 +1 @@
-- page_title "Variables"
-
-.row.prepend-top-default.append-bottom-default
-  .col-lg-3
-    = render "ci/variables/content"
-  .col-lg-9
-    %h5.prepend-top-0
-      Update variable
-    = render "ci/variables/form", btn_text: "Save variable"
+= render 'ci/variables/show'
diff --git a/app/views/projects/variables/show.html.haml b/app/views/projects/variables/show.html.haml
index 2bfb290629d..df533952b76 100644
--- a/app/views/projects/variables/show.html.haml
+++ b/app/views/projects/variables/show.html.haml
@@ -1,9 +1 @@
-- page_title "Variables"
-
-.row.prepend-top-default.append-bottom-default
-  .col-lg-3
-    = render "ci/variables/content"
-  .col-lg-9
-    %h5.prepend-top-0
-      Update variable
-    = render "ci/variables/form", btn_text: "Save variable"
+= render 'ci/variables/show'
diff --git a/spec/presenters/ci/group_variable_presenter_spec.rb b/spec/presenters/ci/group_variable_presenter_spec.rb
new file mode 100644
index 00000000000..510361ae0f8
--- /dev/null
+++ b/spec/presenters/ci/group_variable_presenter_spec.rb
@@ -0,0 +1,65 @@
+require 'spec_helper'
+
+describe Ci::GroupVariablePresenter do
+  let(:group) { create(:group) }
+  let(:variable) { create(:ci_group_variable, group: group) }
+
+  subject(:presenter) do
+    described_class.new(variable)
+  end
+
+  it 'inherits from Gitlab::View::Presenter::Delegated' do
+    expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated)
+  end
+
+  describe '#initialize' do
+    it 'takes a variable and optional params' do
+      expect { presenter }.not_to raise_error
+    end
+
+    it 'exposes variable' do
+      expect(presenter.variable).to eq(variable)
+    end
+
+    it 'forwards missing methods to pipeline' do
+      expect(presenter.key).to eq(variable.key)
+    end
+  end
+
+  describe '#placeholder' do
+    subject { described_class.new(variable).placeholder }
+
+    it { is_expected.to eq('GROUP_VARIABLE') }
+  end
+
+  describe '#create_update_path' do
+    context 'when variable is persisted' do
+      let(:path) { "/groups/#{group.name}/variables/#{variable.id}" }
+      subject { described_class.new(variable).create_update_path }
+
+      it { is_expected.to eq(path) }
+    end
+
+    context 'when variable is not persisted' do
+      let(:path) { "/groups/#{group.name}/variables" }
+      let(:variable) { build(:ci_group_variable, group: group) }
+      subject { described_class.new(variable).create_update_path }
+
+      it { is_expected.to eq(path) }
+    end
+  end
+
+  describe '#edit_path' do
+    let(:path) { "/groups/#{group.name}/variables/#{variable.id}" }
+    subject { described_class.new(variable).edit_path }
+
+    it { is_expected.to eq(path) }
+  end
+
+  describe '#delete_path' do
+    let(:path) { "/groups/#{group.name}/variables/#{variable.id}" }
+    subject { described_class.new(variable).delete_path }
+
+    it { is_expected.to eq(path) }
+  end
+end
diff --git a/spec/presenters/ci/variable_presenter_spec.rb b/spec/presenters/ci/variable_presenter_spec.rb
new file mode 100644
index 00000000000..d040af43c66
--- /dev/null
+++ b/spec/presenters/ci/variable_presenter_spec.rb
@@ -0,0 +1,65 @@
+require 'spec_helper'
+
+describe Ci::VariablePresenter do
+  let(:project) { create(:empty_project) }
+  let(:variable) { create(:ci_variable, project: project) }
+
+  subject(:presenter) do
+    described_class.new(variable)
+  end
+
+  it 'inherits from Gitlab::View::Presenter::Delegated' do
+    expect(described_class.superclass).to eq(Gitlab::View::Presenter::Delegated)
+  end
+
+  describe '#initialize' do
+    it 'takes a variable and optional params' do
+      expect { presenter }.not_to raise_error
+    end
+
+    it 'exposes variable' do
+      expect(presenter.variable).to eq(variable)
+    end
+
+    it 'forwards missing methods to pipeline' do
+      expect(presenter.key).to eq(variable.key)
+    end
+  end
+
+  describe '#placeholder' do
+    subject { described_class.new(variable).placeholder }
+
+    it { is_expected.to eq('PROJECT_VARIABLE') }
+  end
+
+  describe '#create_update_path' do
+    context 'when variable is persisted' do
+      let(:path) { "/#{project.namespace.name}/#{project.name}/variables/#{variable.id}" }
+    subject { described_class.new(variable).create_update_path }
+
+    it { is_expected.to eq(path) }
+    end
+
+    context 'when variable is not persisted' do
+      let(:variable) { build(:ci_variable, project: project) }
+      let(:path) { "/#{project.namespace.name}/#{project.name}/variables" }
+      subject { described_class.new(variable).create_update_path }
+
+      it { is_expected.to eq(path) }
+    end
+  end
+
+  describe '#edit_path' do
+    let(:path) { "/#{project.namespace.name}/#{project.name}/variables/#{variable.id}" }
+    subject { described_class.new(variable).edit_path }
+
+    it { is_expected.to eq(path) }
+  end
+
+  describe '#delete_path' do
+    let(:path) { "/#{project.namespace.name}/#{project.name}/variables/#{variable.id}" }
+    subject { described_class.new(variable).delete_path }
+
+    it { is_expected.to eq(path) }
+  end
+end