avoid nil for wiki page slugs as they would be invalid (#9081)

* avoid nil for wiki page slugs as they would be invalid

Also explicitly enables having "!" for a wiki title but fails hard for all cases, which are not handled by default or custom.

* move legacy specs over

app/models/wiki_page.rb

@@ -54,7 +54,13 @@ class WikiPage < ApplicationRecord
 
   attr_accessor :redirect_existing_links
 
-  validates_presence_of :title
+  validates :title, presence: true
+  validates :slug,
+            presence: {
+              message: ->(object, _) {
+                I18n.t('activerecord.errors.models.wiki_page.attributes.slug.undeducible', title: object.title)
+              }
+            }
   validates_associated :content
 
   validate :validate_consistency_of_parent_title

config/locales/en.yml

@@ -697,6 +697,10 @@ en:
           attributes:
             hours:
               day_limit: "is too high as a maximum of 24 hours can be logged per date."
+        wiki_page:
+          attributes:
+            slug:
+              undeducible: "cannot be deduced from the title '%{title}'."
         work_package:
           is_not_a_valid_target_for_time_entries: "Work package #%{id} is not a valid target for reassigning the time entries."
           attributes:

lib/open_project/acts_as_url/adapter/op_active_record.rb

@@ -42,9 +42,18 @@ module OpenProject
         def modify_base_url
           super
 
-          if base_url.empty? && instance.send(settings.attribute_to_urlify).to_s == '.'
+          modify_base_url_custom_rules if base_url.empty?
-            self.base_url = 'dot'
+        end
-          end
+
+        def modify_base_url_custom_rules
+          replacement = case instance.send(settings.attribute_to_urlify).to_s
+                        when '.'
+                          'dot'
+                        when '!'
+                          'bang'
+                        end
+
+          self.base_url = replacement if replacement
         end
       end
     end

spec/models/wiki_page_spec.rb

@@ -31,7 +31,9 @@ require 'spec_helper'
 describe WikiPage, type: :model do
   let(:project) { FactoryBot.create(:project).reload } # a wiki is created for project, but the object doesn't know of it (FIXME?)
   let(:wiki) { project.wiki }
-  let(:wiki_page) { FactoryBot.create(:wiki_page, wiki: wiki, title: wiki.wiki_menu_items.first.title) }
+  let(:title) { wiki.wiki_menu_items.first.title }
+  let(:wiki_page) { FactoryBot.create(:wiki_page, wiki: wiki, title: title) }
+  let(:new_wiki_page) { FactoryBot.build(:wiki_page, wiki: wiki, title: title) }
 
   it_behaves_like 'acts_as_watchable included' do
     let(:model_instance) { FactoryBot.create(:wiki_page) }
@@ -66,6 +68,23 @@ describe WikiPage, type: :model do
         expect(wiki_page.slug).to eq('dot')
       end
     end
+
+    context 'when only having a ! for the title' do
+      let(:wiki_page) { FactoryBot.create(:wiki_page, wiki: wiki, title: '!') }
+
+      it 'creates a non empty slug' do
+        expect(wiki_page.slug).to eq('bang')
+      end
+    end
+
+    context 'when only having a { for the title' do
+      let(:wiki_page) { FactoryBot.create(:wiki_page, wiki: wiki, title: '{') }
+
+      it 'fails to create' do
+        expect { wiki_page }
+          .to raise_error(ActiveRecord::RecordInvalid)
+      end
+    end
   end
 
   describe '#nearest_main_item' do
@@ -83,7 +102,7 @@ describe WikiPage, type: :model do
 
   describe '#destroy' do
     context 'when the only wiki page is destroyed' do
-      before :each do
+      before do
         wiki_page.destroy
       end
 
@@ -94,7 +113,7 @@ describe WikiPage, type: :model do
     end
 
     context 'when one of two wiki pages is destroyed' do
-      before :each do
+      before do
         FactoryBot.create(:wiki_page, wiki: wiki)
         wiki_page.destroy
       end
@@ -106,6 +125,26 @@ describe WikiPage, type: :model do
     end
   end
 
+  describe '#title' do
+    context 'when it is blank' do
+      let(:title) { nil }
+
+      it 'is invalid' do
+        new_wiki_page.valid?
+
+        expect(new_wiki_page.errors.symbols_for(:title))
+          .to match_array [:blank]
+      end
+    end
+  end
+
+  describe '#protected?' do
+    it 'is false by default' do
+      expect(wiki_page.reload)
+        .not_to be_protected
+    end
+  end
+
   describe '#project' do
     it 'is the same as the project on wiki' do
       expect(wiki_page.project).to eql(wiki.project)

spec_legacy/unit/wiki_page_spec.rb

@@ -37,20 +37,6 @@ describe WikiPage, type: :model do
     @page = @wiki.pages.first
   end
 
-  it 'should create' do
-    page = WikiPage.new(wiki: @wiki)
-    assert !page.save
-    assert_equal 1, page.errors.count
-
-    page.title = 'Page'
-    assert page.save
-    page.reload
-    assert !page.protected?
-
-    @wiki.reload
-    assert @wiki.pages.include?(page)
-  end
-
   it 'should find or new page' do
     page = @wiki.find_or_new_page('CookBook documentation')
     assert_kind_of WikiPage, page