Merge branch '6293-persisted-roadmap-preset' into 'master'

Persist Epic Roadmap timescale choice

Closes #6293

See merge request gitlab-org/gitlab-ee!6637

db/schema.rb

@@ -2789,6 +2789,7 @@ ActiveRecord::Schema.define(version: 20180803001726) do
     t.integer "accepted_term_id"
     t.string "feed_token"
     t.boolean "private_profile"
+    t.integer "roadmap_layout", limit: 2
   end
 
   add_index "users", ["admin"], name: "index_users_on_admin", using: :btree

ee/app/controllers/groups/roadmap_controller.rb

 module Groups
   class RoadmapController < Groups::ApplicationController
     before_action :group
+    before_action :persist_roadmap_layout, only: [:show]
 
     def show
       # show roadmap for a group
       @epics_count = EpicsFinder.new(current_user, group_id: @group.id).execute.count
     end
+
+    private
+
+    def persist_roadmap_layout
+      return unless current_user
+
+      roadmap_layout = params[:layout]&.downcase
+
+      return unless User.roadmap_layouts[roadmap_layout]
+      return if current_user.roadmap_layout == roadmap_layout
+
+      Users::UpdateService.new(current_user, user: current_user, roadmap_layout: roadmap_layout).execute
+    end
   end
 end

ee/app/helpers/roadmaps_helper.rb

+# frozen_string_literal: true
+
+module RoadmapsHelper
+  def roadmap_layout
+    (current_user&.roadmap_layout || params[:layout].presence || EE::User::DEFAULT_ROADMAP_LAYOUT).upcase
+  end
+end

ee/app/models/ee/user.rb

@@ -7,6 +7,8 @@ module EE
     extend ActiveSupport::Concern
     include AuditorUserHelper
 
+    DEFAULT_ROADMAP_LAYOUT = 'months'.freeze
+
     included do
       EMAIL_OPT_IN_SOURCE_ID_GITLAB_COM = 1
 
@@ -35,6 +37,8 @@ module EE
       has_many :protected_branch_unprotect_access_levels, dependent: :destroy, class_name: ::ProtectedBranch::UnprotectAccessLevel # rubocop:disable Cop/ActiveRecordDependent
 
       scope :excluding_guests, -> { joins(:members).where('members.access_level > ?', ::Gitlab::Access::GUEST).distinct }
+
+      enum roadmap_layout: { weeks: 1, months: 4, quarters: 12 }
     end
 
     module ClassMethods
@@ -102,5 +106,9 @@ module EE
                          params: { search: search, sort: 'name_asc' })
                     .execute
     end
+
+    def roadmap_layout
+      super || DEFAULT_ROADMAP_LAYOUT
+    end
   end
 end

ee/app/views/groups/roadmap/show.html.haml

@@ -2,7 +2,6 @@
 - @no_container = true
 - @content_wrapper_class = "group-epics-roadmap-wrapper"
 - @content_class = "group-epics-roadmap"
-- preset_layout = params[:layout].present? ? params[:layout] : "MONTHS"
 - breadcrumb_title _("Epics Roadmap")
 
 - has_filters_applied = params[:label_name].present? || params[:author_username].present? || params[:search].present?
@@ -10,6 +9,6 @@
 - if @epics_count != 0
   = render 'shared/epic/search_bar', type: :epics, hide_sort_dropdown: true, show_roadmap_presets: true
 
-  #js-roadmap{ data: { epics_path: group_epics_path(@group, format: :json), group_id: @group.id, empty_state_illustration: image_path('illustrations/epics/roadmap.svg'), has_filters_applied: "#{has_filters_applied}", new_epic_endpoint: group_epics_path(@group), preset_type: preset_layout } }
+  #js-roadmap{ data: { epics_path: group_epics_path(@group, format: :json), group_id: @group.id, empty_state_illustration: image_path('illustrations/epics/roadmap.svg'), has_filters_applied: "#{has_filters_applied}", new_epic_endpoint: group_epics_path(@group), preset_type: roadmap_layout } }
 - else
   = render 'shared/empty_states/roadmap'

ee/app/views/shared/epic/_search_bar.html.haml

@@ -2,7 +2,7 @@
 - hide_sort_dropdown = local_assigns.fetch(:hide_sort_dropdown, false)
 - show_roadmap_presets = local_assigns.fetch(:show_roadmap_presets, false)
 - full_path = @project.present? ? @project.full_path : @group.full_path
-- preset_layout = params[:layout].present? ? params[:layout] : "MONTHS"
+- preset_layout = roadmap_layout
 - is_quarters = preset_layout == "QUARTERS"
 - is_months = preset_layout == "MONTHS"
 - is_weeks = preset_layout == "WEEKS"

ee/changelogs/unreleased/6293-persisted-roadmap-preset.yml

+---
+title: Persist Epic Roadmap timescale choice
+merge_request: 6637
+author:
+type: added

ee/db/migrate/20180723081631_add_roadmap_layout_to_users.rb

+# frozen_string_literal: true
+
+class AddRoadmapLayoutToUsers < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :users, :roadmap_layout, :integer, limit: 2
+  end
+end

ee/spec/helpers/roadmaps_helper_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+describe RoadmapsHelper do
+  describe '#roadmap_layout' do
+    before do
+      allow(helper).to receive(:current_user) { user }
+    end
+
+    context 'guest' do
+      let(:user) { nil }
+
+      it 'is sourced from params if exists' do
+        allow(helper).to receive(:params).and_return(layout: 'WEEKS')
+
+        expect(helper.roadmap_layout).to eq('WEEKS')
+      end
+
+      it 'returns default if params do not exist' do
+        allow(helper).to receive(:params).and_return({})
+
+        expect(helper.roadmap_layout).to eq('MONTHS')
+      end
+    end
+
+    context 'logged in' do
+      let(:user) { double(:user) }
+
+      it 'is sourced from User#roadmap_layout' do
+        allow(helper).to receive(:params).and_return(layout: 'WEEKS')
+        expect(user).to receive(:roadmap_layout).and_return('quarters')
+
+        expect(helper.roadmap_layout).to eq('QUARTERS')
+      end
+    end
+  end
+end

ee/spec/models/ee/user_spec.rb

@@ -218,4 +218,22 @@ describe EE::User do
       end
     end
   end
+
+  describe '#roadmap_layout' do
+    context 'not set' do
+      subject { build(:user, roadmap_layout: nil) }
+
+      it 'returns default value' do
+        expect(subject.roadmap_layout).to eq(EE::User::DEFAULT_ROADMAP_LAYOUT)
+      end
+    end
+
+    context 'set' do
+      subject { build(:user, roadmap_layout: 'quarters') }
+
+      it 'returns set value' do
+        expect(subject.roadmap_layout).to eq('quarters')
+      end
+    end
+  end
 end

ee/spec/requests/groups/roadmap_controller_spec.rb

+# frozen_string_literal: true
+require 'rails_helper'
+
+describe Groups::RoadmapController do
+  let(:user) { create(:user) }
+  let(:group) { create(:group, :public) }
+
+  before do
+    stub_licensed_features(epics: true)
+  end
+
+  describe 'GET /groups/*namespace_id/-/roadmap' do
+    let(:layout) { 'WEEKS' }
+
+    context 'guest' do
+      it 'renders without persisting layout' do
+        expect do
+          get group_roadmap_path(group, layout: layout)
+        end.not_to change { user.reload.roadmap_layout }
+
+        expect(response).to have_gitlab_http_status(200)
+      end
+    end
+
+    context 'logged in' do
+      before do
+        allow(Users::UpdateService).to receive(:new).and_call_original
+        group.add_maintainer(user)
+        login_as user
+      end
+
+      context 'not specifying layout' do
+        it 'renders without persisting layout' do
+          expect(Users::UpdateService).not_to receive(:new).with(user, user: user, roadmap_layout: a_kind_of(String))
+          expect do
+            get group_roadmap_path(group)
+          end.not_to change { user.reload.roadmap_layout }
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+
+      context 'specifying invalid layout' do
+        it 'renders without persisting layout' do
+          expect(Users::UpdateService).not_to receive(:new).with(user, user: user, roadmap_layout: a_kind_of(String))
+          get group_roadmap_path(group, layout: 'FOO')
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+
+      context 'specifying layout' do
+        it 'persists roadmap_layout if different than current layout' do
+          expect(Users::UpdateService).to receive(:new).with(user, user: user, roadmap_layout: layout.downcase).once.and_call_original
+
+          expect do
+            get group_roadmap_path(group, layout: layout)
+            get group_roadmap_path(group, layout: layout)
+          end.to change { user.reload.roadmap_layout }.to(layout.downcase)
+
+          expect(response).to have_gitlab_http_status(200)
+        end
+      end
+    end
+  end
+end