enforce setting existing wiki page for version

app/contracts/versions/base_contract.rb

@@ -52,7 +52,9 @@ module Versions
     attribute :effective_date
     attribute :status
     attribute :sharing
-    attribute :wiki_page_title
+    attribute :wiki_page_title do
+      validate_page_title_in_wiki
+    end
 
     def assignable_statuses
       Version::VERSION_STATUSES
@@ -79,6 +81,16 @@ module Versions
       end
     end
 
+    def assignable_wiki_pages
+      wiki = model.project.wiki
+
+      if wiki
+        wiki.pages
+      else
+        WikiPage.where('1=0')
+      end
+    end
+
     def assignable_custom_field_values(custom_field)
       custom_field.possible_values
     end
@@ -100,5 +112,11 @@ module Versions
     def validate_project_is_set
       errors.add :project_id, :blank if model.project.nil?
     end
+
+    def validate_page_title_in_wiki
+      return unless model.wiki_page_title.present? && model.project&.wiki
+
+      errors.add :wiki_page_title, :inclusion unless model.project.wiki.find_page(model.wiki_page_title)
+    end
   end
 end

app/controllers/versions_controller.rb

@@ -82,14 +82,7 @@ class VersionsController < ApplicationController
            .new(user: current_user)
            .call(attributes)
 
-    @version = call.result
-
-    if call.success?
-      flash[:notice] = l(:notice_successful_create)
-      redirect_back_or_version_settings
-    else
-      render action: 'new'
-    end
+    render_cu(call, :notice_successful_create, 'new')
   end
 
   def edit; end
@@ -103,12 +96,7 @@ class VersionsController < ApplicationController
                 model: @version)
            .call(attributes)
 
-    if call.success?
-      flash[:notice] = l(:notice_successful_update)
-      redirect_back_or_version_settings
-    else
-      render action: 'edit'
-    end
+    render_cu(call, :notice_successful_update, 'edit')
   end
 
   def close_completed
@@ -154,4 +142,17 @@ class VersionsController < ApplicationController
       (default_types || selectable_types).map { |t| t.id.to_s }
     end
   end
+
+  def render_cu(call, success_message, failure_action)
+    @version = call.result
+
+    if call.success?
+      flash[:notice] = t(success_message)
+      redirect_back_or_version_settings
+    else
+      @errors = call.errors
+
+      render action: failure_action
+    end
+  end
 end

app/controllers/wiki_controller.rb

@@ -249,11 +249,13 @@ class WikiController < ApplicationController
 
   def edit_parent_page
     return render_403 unless editable?
+
     @parent_pages = @wiki.pages.includes(:parent) - @page.self_and_descendants
   end
 
   def update_parent_page
     return render_403 unless editable?
+
     @page.parent_id = params[:wiki_page][:parent_id]
     if @page.save
       flash[:notice] = l(:notice_successful_update)

app/helpers/error_message_helper.rb

@@ -51,7 +51,7 @@ module ErrorMessageHelper
       end
     end
 
-    render_error_messages_partial(error_messages, { object: object })
+    render_error_messages_partial(error_messages, object: object)
   end
 
   def extract_objects_from_params(params)
@@ -73,7 +73,7 @@ module ErrorMessageHelper
       render partial: 'common/validation_error',
              locals: { error_messages: messages,
                        classes: options[:classes],
-                       object_name:  options[:object].class.model_name.human }
+                       object_name: options[:object].class.model_name.human }
     end
   end
 end

app/helpers/wiki_helper.rb

@@ -28,21 +28,17 @@
 #++
 
 module WikiHelper
-  def wiki_page_options_for_select(pages, parent = nil, level = 0)
-    pages = pages.group_by(&:parent) unless pages.is_a?(Hash)
-    s = []
-    if level.zero?
-      s << ["-- #{t('label_no_parent_page')} --", '']
-    end
-    if pages.has_key?(parent)
-      pages[parent].each do |page|
-        indent = (level > 0) ? ('&nbsp;' * level * 2 + '&#187; ') : ''
+  def wiki_page_options_for_select(pages,
+                                   ids: true,
+                                   placeholder: true)
+    s = if placeholder
+          ["-- #{t('label_no_parent_page')} --", '']
+        else
+          []
+        end
 
-        s << [(indent + h(page.title)).html_safe, page.id]
-        s += wiki_page_options_for_select(pages, page, level + 1)
-      end
-    end
-    s
+    s + wiki_page_options_for_select_of_level(pages.group_by(&:parent),
+                                              ids: ids)
   end
 
   def breadcrumb_for_page(page, action = nil)
@@ -58,4 +54,25 @@ module WikiHelper
   def nl2br(content)
     content.gsub(/(?:\n\r?|\r\n?)/, '<br />').html_safe
   end
+
+  private
+
+  def wiki_page_options_for_select_of_level(pages,
+                                            parent: nil,
+                                            level: 0,
+                                            ids: true)
+    return [] unless pages[parent]
+
+    pages[parent].inject([]) do |s, page|
+      s << wiki_page_option(page, level, ids)
+      s += wiki_page_options_for_select_of_level(pages, parent: page, level: level + 1, ids: ids)
+      s
+    end
+  end
+
+  def wiki_page_option(page, level, ids)
+    indent = level.positive? ? ('&nbsp;' * level * 2 + '&#187; ') : ''
+    id = ids ? page.id : page.title
+    [(indent + h(page.title)).html_safe, id]
+  end
 end

app/views/versions/_form.html.erb

@@ -28,12 +28,12 @@ See docs/COPYRIGHT.rdoc for more details.
 ++#%>
 
 <%
-  # locals: f, project
+  # locals: f, project, errors
   version = f.object
   contract = version_contract(version)
 %>
 
-<%= error_messages_for 'version' %>
+<%= error_messages_for_contract version, errors %>
 <%= back_url_hidden_field_tag %>
 
 <div class="form--field -required">
@@ -51,10 +51,10 @@ See docs/COPYRIGHT.rdoc for more details.
 </div>
 
 <div class="form--field">
-  <%= f.text_field :wiki_page_title,
-                   label: :label_wiki_page,
-                   disabled: project.wiki.nil?,
-                   container_class: '-wide' %>
+  <%= f.select :wiki_page_title,
+               wiki_page_options_for_select(contract.assignable_wiki_pages.includes(:parent), placeholder: false, ids: false),
+               { label: :label_wiki_page, include_blank: true },
+               { container_class: "-wide", disabled: contract.assignable_wiki_pages.none? } %>
 </div>
 
 <div class="form--field">
@@ -68,12 +68,12 @@ See docs/COPYRIGHT.rdoc for more details.
 <div class="form--field">
   <%= f.select :sharing,
                contract.assignable_sharings.map { |v| [format_version_sharing(v), v] },
-               container_class: '-slim' %>
+               container_class: '-middle' %>
 </div>
 
-<% if @project.enabled_modules.map(&:name).include?("backlogs") %>
+<% if project.enabled_modules.map(&:name).include?("backlogs") %>
 
-  <%= version_settings_fields(version, @project) %>
+  <%= version_settings_fields(version, project) %>
 
 <% end %>
 

app/views/versions/edit.html.erb

@@ -26,14 +26,15 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 See docs/COPYRIGHT.rdoc for more details.
 
 ++#%>
-<% html_title l("label_roadmap_edit", name: @version.name) %>
+
+<% html_title t("label_roadmap_edit", name: @version.name) %>
 <%= toolbar title: Version.model_name.human %>
 
 <%= labelled_tabular_form_for @version do |f| %>
 
   <%= render partial: 'form', locals: { f: f,
-                                              project: @project } %>
-
-  <%= styled_button_tag l(:button_save), class: '-highlight -with-icon icon-checkmark' %>
+                                        project: @project,
+                                        errors: @errors } %>
 
+  <%= styled_button_tag t(:button_save), class: '-highlight -with-icon icon-checkmark' %>
 <% end %>

app/views/versions/new.html.erb

@@ -26,15 +26,15 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 See docs/COPYRIGHT.rdoc for more details.
 
 ++#%>
-<% html_title l("label_version_new") %>
+<% html_title t("label_version_new") %>
 
-<%= toolbar title: l(:label_version_new) %>
+<%= toolbar title: t(:label_version_new) %>
 
 <%= labelled_tabular_form_for [@project, @version] do |f| %>
 
   <%= render partial: 'versions/form', locals: { f: f,
-                                                       project: @project } %>
-
-  <%= styled_button_tag l(:button_create), class: '-highlight -with-icon icon-checkmark' %>
+                                                 project: @project,
+                                                 errors: @errors } %>
 
+  <%= styled_button_tag t(:button_create), class: '-highlight -with-icon icon-checkmark' %>
 <% end %>

modules/backlogs/features/version_settings.feature

@@ -88,8 +88,8 @@ Feature: Version Settings
     And the project uses the following modules:
         | backlogs |
     And the project has the following sprints:
-        | name       | start_date | effective_date | sharing       | wiki_page_title |
-        | shared     | 2010-01-01        | 2010-01-31     | system        | blubs    |
+        | name       | start_date      | effective_date | sharing     |
+        | shared     | 2010-01-01      | 2010-01-31     | system      |
     And I am working in project "ecookbook"
 
     When I go to the settings/versions page of the project called "ecookbook"

spec/contracts/versions/shared_contract_examples.rb

@@ -40,12 +40,27 @@ shared_examples_for 'version contract' do
   end
   let(:root_project) { FactoryBot.build_stubbed(:project) }
   let(:version_project) do
-    FactoryBot.build_stubbed(:project).tap do |p|
+    FactoryBot.build_stubbed(:project, wiki: project_wiki).tap do |p|
       allow(p)
         .to receive(:root)
         .and_return(root_project)
     end
   end
+  let(:project_wiki) do
+    FactoryBot.build_stubbed(:wiki, pages: wiki_pages).tap do |wiki|
+      allow(wiki)
+        .to receive(:find_page)
+        .with(version_wiki_page_title)
+        .and_return(find_page_result)
+    end
+  end
+  let(:find_page_result) do
+    double('page found')
+  end
+  let(:wiki_pages) do
+    [FactoryBot.build_stubbed(:wiki_page),
+     FactoryBot.build_stubbed(:wiki_page)]
+  end
   let(:version_name) { 'Version name' }
   let(:version_description) { 'Version description' }
   let(:version_start_date) { Date.current - 5.days }
@@ -56,172 +71,223 @@ shared_examples_for 'version contract' do
   let(:permissions) { [:manage_versions] }
   let(:root_permissions) { [:manage_versions] }
 
-  def expect_valid(valid, symbols = {})
-    expect(contract.validate).to eq(valid)
+  context 'validations' do
+    def expect_valid(valid, symbols = {})
+      expect(contract.validate).to eq(valid)
 
-    symbols.each do |key, arr|
-      expect(contract.errors.symbols_for(key)).to match_array arr
+      symbols.each do |key, arr|
+        expect(contract.errors.symbols_for(key)).to match_array arr
+      end
     end
-  end
 
-  shared_examples 'is valid' do
-    it 'is valid' do
-      expect_valid(true)
+    shared_examples 'is valid' do
+      it 'is valid' do
+        expect_valid(true)
+      end
     end
-  end
 
-  it_behaves_like 'is valid'
+    it_behaves_like 'is valid'
 
-  context 'if the project is nil' do
-    let(:version_project) { nil }
+    context 'if the project is nil' do
+      let(:version_project) { nil }
 
-    it 'is invalid' do
-      expect_valid(false, project_id: %i(blank))
+      it 'is invalid' do
+        expect_valid(false, project_id: %i(blank))
+      end
     end
-  end
 
-  context 'if the name is nil' do
-    let(:version_name) { nil }
+    context 'if the name is nil' do
+      let(:version_name) { nil }
 
-    it 'is invalid' do
-      expect_valid(false, name: %i(blank))
+      it 'is invalid' do
+        expect_valid(false, name: %i(blank))
+      end
     end
-  end
 
-  context 'if the description is nil' do
-    let(:version_description) { nil }
+    context 'if the description is nil' do
+      let(:version_description) { nil }
 
-    it_behaves_like 'is valid'
-  end
+      it_behaves_like 'is valid'
+    end
 
-  context 'if the start_date is nil' do
-    let(:version_start_date) { nil }
+    context 'if the start_date is nil' do
+      let(:version_start_date) { nil }
 
-    it_behaves_like 'is valid'
-  end
+      it_behaves_like 'is valid'
+    end
 
-  context 'if the end_date is nil' do
-    let(:version_due_date) { nil }
+    context 'if the end_date is nil' do
+      let(:version_due_date) { nil }
 
-    it_behaves_like 'is valid'
-  end
+      it_behaves_like 'is valid'
+    end
 
-  context 'if the status is nil' do
-    let(:version_status) { nil }
+    context 'if the status is nil' do
+      let(:version_status) { nil }
 
-    it 'is invalid' do
-      expect_valid(false, status: %i(inclusion))
+      it 'is invalid' do
+        expect_valid(false, status: %i(inclusion))
+      end
     end
-  end
 
-  context 'if the status is something other than the allowed values' do
-    let(:version_status) { 'other_status' }
+    context 'if the status is something other than the allowed values' do
+      let(:version_status) { 'other_status' }
 
-    it 'is invalid' do
-      expect_valid(false, status: %i(inclusion))
+      it 'is invalid' do
+        expect_valid(false, status: %i(inclusion))
+      end
     end
-  end
 
-  context 'if sharing is nil' do
-    before do
-      version.sharing = 'nil'
-    end
+    context 'if sharing is nil' do
+      before do
+        version.sharing = 'nil'
+      end
 
-    it 'is invalid' do
-      expect_valid(false, sharing: %i(inclusion))
+      it 'is invalid' do
+        expect_valid(false, sharing: %i(inclusion))
+      end
     end
-  end
 
-  context 'if sharing is bogus' do
-    before do
-      version.sharing = 'bogus'
-    end
+    context 'if sharing is bogus' do
+      before do
+        version.sharing = 'bogus'
+      end
 
-    it 'is invalid' do
-      expect_valid(false, sharing: %i(inclusion))
+      it 'is invalid' do
+        expect_valid(false, sharing: %i(inclusion))
+      end
     end
-  end
 
-  context 'if sharing is system and the user an admin' do
-    let(:current_user) { FactoryBot.build_stubbed(:admin) }
+    context 'if sharing is system and the user an admin' do
+      let(:current_user) { FactoryBot.build_stubbed(:admin) }
+
+      before do
+        version.sharing = 'system'
+      end
 
-    before do
-      version.sharing = 'system'
+      it_behaves_like 'is valid'
     end
 
-    it_behaves_like 'is valid'
-  end
+    context 'if sharing is system and the user no admin' do
+      before do
+        version.sharing = 'system'
+      end
 
-  context 'if sharing is system and the user no admin' do
-    before do
-      version.sharing = 'system'
+      it 'is invalid' do
+        expect_valid(false, sharing: %i(inclusion))
+      end
     end
 
-    it 'is invalid' do
-      expect_valid(false, sharing: %i(inclusion))
+    context 'if sharing is descendants' do
+      before do
+        version.sharing = 'descendants'
+      end
+
+      it_behaves_like 'is valid'
     end
-  end
 
-  context 'if sharing is descendants' do
-    before do
-      version.sharing = 'descendants'
+    context 'if sharing is tree and the user has manage permission on the root project' do
+      before do
+        version.sharing = 'tree'
+      end
+
+      it_behaves_like 'is valid'
     end
 
-    it_behaves_like 'is valid'
-  end
+    context 'if sharing is tree and the user has no manage permission on the root project' do
+      let(:root_permissions) { [] }
+      before do
+        version.sharing = 'tree'
+      end
 
-  context 'if sharing is tree and the user has manage permission on the root project' do
-    before do
-      version.sharing = 'tree'
+      it 'is invalid' do
+        expect_valid(false, sharing: %i(inclusion))
+      end
     end
 
-    it_behaves_like 'is valid'
-  end
+    context 'if sharing is hierarchy and the user has manage permission on the root project' do
+      before do
+        version.sharing = 'hierarchy'
+      end
 
-  context 'if sharing is tree and the user has no manage permission on the root project' do
-    let(:root_permissions) { [] }
-    before do
-      version.sharing = 'tree'
+      it_behaves_like 'is valid'
     end
 
-    it 'is invalid' do
-      expect_valid(false, sharing: %i(inclusion))
+    context 'if sharing is hierarchy and the user has no manage permission on the root project' do
+      let(:root_permissions) { [] }
+
+      before do
+        version.sharing = 'hierarchy'
+      end
+
+      it 'is invalid' do
+        expect_valid(false, sharing: %i(inclusion))
+      end
     end
-  end
 
-  context 'if sharing is hierarchy and the user has manage permission on the root project' do
-    before do
-      version.sharing = 'hierarchy'
+    context 'if the user lacks the manage_versions permission' do
+      let(:permissions) { [] }
+
+      it 'is invalid' do
+        expect_valid(false, base: %i(error_unauthorized))
+      end
     end
 
-    it_behaves_like 'is valid'
-  end
+    context 'if the start date is after the effective date' do
+      let(:version_start_date) { version_due_date + 1.day }
+
+      it 'is invalid' do
+        expect_valid(false, effective_date: %i(greater_than_start_date))
+      end
+    end
 
-  context 'if sharing is hierarchy and the user has no manage permission on the root project' do
-    let(:root_permissions) { [] }
+    context 'if wiki_page_title is nil' do
+      let(:version_wiki_page_title) { nil }
+      let(:find_page_result) do
+        nil
+      end
 
-    before do
-      version.sharing = 'hierarchy'
+      it_behaves_like 'is valid'
     end
 
-    it 'is invalid' do
-      expect_valid(false, sharing: %i(inclusion))
+    context 'if wiki_page_title is blank' do
+      let(:version_wiki_page_title) { '' }
+      let(:find_page_result) do
+        nil
+      end
+
+      it_behaves_like 'is valid'
     end
-  end
 
-  context 'if the user lacks the manage_versions permission' do
-    let(:permissions) { [] }
+    context 'if wiki_page_title contains a non existing page' do
+      let(:version_wiki_page_title) { 'http://some/url/i/made/up' }
+      let(:find_page_result) do
+        nil
+      end
 
-    it 'is invalid' do
-      expect_valid(false, base: %i(error_unauthorized))
+      it 'is invalid' do
+        expect_valid(false, wiki_page_title: %i(inclusion))
+      end
     end
   end
 
-  context 'if the start date is after the effective date' do
-    let(:version_start_date) { version_due_date + 1.day }
+  describe 'assignable values' do
+    describe 'assignable_wiki_pages' do
+      context 'with a wiki assigned' do
+        it 'returns the pages of the project`s wiki' do
+          expect(contract.assignable_wiki_pages)
+            .to match_array(wiki_pages)
+        end
+      end
+
+      context 'without a wiki assigned' do
+        let(:project_wiki) { nil }
 
-    it 'is invalid' do
-      expect_valid(false, effective_date: %i(greater_than_start_date))
+        it 'is empty' do
+          expect(contract.assignable_wiki_pages)
+            .to be_empty
+        end
+      end
     end
   end
 end

spec/controllers/versions_controller_spec.rb

@@ -236,7 +236,7 @@ describe VersionsController, type: :controller do
 
   describe '#update' do
     context 'with valid params' do
-      let(:params) {
+      let(:params) do
         {
           id: version1.id,
           version: {
@@ -244,7 +244,7 @@ describe VersionsController, type: :controller do
             effective_date: Date.today.strftime('%Y-%m-%d')
           }
         }
-      }
+      end
       before do
         login_as(user)
         patch :update, params: params
@@ -284,6 +284,7 @@ describe VersionsController, type: :controller do
 
       it { expect(response).to be_successful }
       it { expect(response).to render_template('edit') }
+      it { expect(assigns(:errors).symbols_for(:name)).to match_array([:blank]) }
     end
   end