From f4083a8d64dc5bdf930dab6e8ffeb8fc6398c651 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 18 Sep 2023 10:58:47 +0200 Subject: [PATCH 1/5] Fix keyword arguments for Ruby 3 REDMINE-19438 --- .../scrolled/lib/pageflow_scrolled/additional_seed_data.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/entry_types/scrolled/lib/pageflow_scrolled/additional_seed_data.rb b/entry_types/scrolled/lib/pageflow_scrolled/additional_seed_data.rb index 181beec37e..eb25b77d87 100644 --- a/entry_types/scrolled/lib/pageflow_scrolled/additional_seed_data.rb +++ b/entry_types/scrolled/lib/pageflow_scrolled/additional_seed_data.rb @@ -32,7 +32,7 @@ def items_for_entry(entry, options) entry.revision, content_element_type_names ), - options + **options ) end From e05162bb0839a92a4b5d5c0542b3f55cc0be4931 Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 18 Sep 2023 10:59:53 +0200 Subject: [PATCH 2/5] Pass entry type param in editor controller specs Required to find matching route. Also closer to real request. Entry type of fixture entry needs to match passed entry type to prevent `Pageflow::EditorController` from responding with bad reqest. REDMINE-19438 --- .../editor/chapters_controller_spec.rb | 47 +++++++---- .../content_elements_controller_spec.rb | 80 ++++++++++++------- .../editor/sections_controller_spec.rb | 39 +++++---- 3 files changed, 108 insertions(+), 58 deletions(-) diff --git a/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/chapters_controller_spec.rb b/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/chapters_controller_spec.rb index c527bfadf5..9bfad0fbd2 100644 --- a/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/chapters_controller_spec.rb +++ b/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/chapters_controller_spec.rb @@ -9,10 +9,11 @@ module PageflowScrolled describe '#create' do it 'requires authentication' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') post(:create, params: { + entry_type: 'scrolled', entry_id: entry, chapter: attributes_for(:scrolled_chapter) }, format: 'json') @@ -21,11 +22,12 @@ module PageflowScrolled end it 'succeeds for authorized user' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') authorize_for_editor_controller(entry) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, chapter: attributes_for(:scrolled_chapter) }, format: 'json') @@ -34,11 +36,12 @@ module PageflowScrolled end it 'allows setting the chapters configuration hash' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') authorize_for_editor_controller(entry) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, chapter: { configuration: {title: 'A chapter title'} @@ -50,11 +53,15 @@ module PageflowScrolled end it 'renders attributes as camel case' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') authorize_for_editor_controller(entry) post(:create, - params: {entry_id: entry, chapter: attributes_for(:scrolled_chapter)}, + params: { + entry_type: 'scrolled', + entry_id: entry, + chapter: attributes_for(:scrolled_chapter) + }, format: 'json') expect(json_response(path: [:permaId])).to be_present end @@ -62,12 +69,13 @@ module PageflowScrolled describe '#update' do it 'allows updating the chapters configuration hash' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapter = create(:scrolled_chapter, revision: entry.draft) authorize_for_editor_controller(entry) patch(:update, params: { + entry_type: 'scrolled', entry_id: entry, id: chapter, chapter: { @@ -80,14 +88,15 @@ module PageflowScrolled end it 'does not allow updating a chapter from a different entry' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') create(:scrolled_chapter, revision: entry.draft) - other_entry = create(:entry) + other_entry = create(:entry, type_name: 'scrolled') chapter_in_other_entry = create(:scrolled_chapter, revision: other_entry.draft) authorize_for_editor_controller(entry) patch(:update, params: { + entry_type: 'scrolled', entry_id: entry, id: chapter_in_other_entry, chapter: { @@ -101,13 +110,14 @@ module PageflowScrolled describe '#order' do it 'updates position of chapters according to given params order' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapters = create_list(:scrolled_chapter, 2, revision: entry.draft) storyline = chapters.first.storyline authorize_for_editor_controller(entry) put(:order, params: { + entry_type: 'scrolled', entry_id: entry, storyline_id: storyline, ids: [chapters.last.id, chapters.first.id] @@ -118,12 +128,13 @@ module PageflowScrolled end it 'uses first storyline by default' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapters = create_list(:scrolled_chapter, 2, revision: entry.draft) authorize_for_editor_controller(entry) put(:order, params: { + entry_type: 'scrolled', entry_id: entry, ids: [chapters.last.id, chapters.first.id] }, format: 'json') @@ -133,13 +144,14 @@ module PageflowScrolled end it 'allows moving a chapter from one storyline to another within the same entry' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapter = create(:scrolled_chapter, revision: entry.draft) other_storyline = create(:scrolled_storyline, revision: entry.draft) authorize_for_editor_controller(entry) put(:order, params: { + entry_type: 'scrolled', entry_id: entry, storyline_id: other_storyline, ids: [chapter.id] @@ -149,14 +161,15 @@ module PageflowScrolled end it 'does not allow moving a chapter to a storyline of different entry' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapter = create(:scrolled_chapter, revision: entry.draft) - other_entry = create(:entry) + other_entry = create(:entry, type_name: 'scrolled') storyline_in_other_entry = create(:scrolled_storyline, revision: other_entry.draft) authorize_for_editor_controller(entry) put(:order, params: { + entry_type: 'scrolled', entry_id: entry, storyline_id: storyline_in_other_entry, ids: [chapter.id] @@ -168,13 +181,14 @@ module PageflowScrolled describe '#destroy' do it 'deletes the chapter' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapter = create(:scrolled_chapter, revision: entry.draft) storyline = chapter.storyline authorize_for_editor_controller(entry) delete(:destroy, params: { + entry_type: 'scrolled', entry_id: entry, id: chapter }, format: 'json') @@ -184,14 +198,15 @@ module PageflowScrolled end it 'does not allow deleting a chapter from a different entry' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') create(:scrolled_chapter, revision: entry.draft) - other_entry = create(:entry) + other_entry = create(:entry, type_name: 'scrolled') chapter_in_other_entry = create(:scrolled_chapter, revision: other_entry.draft) authorize_for_editor_controller(entry) delete(:destroy, params: { + entry_type: 'scrolled', entry_id: entry, id: chapter_in_other_entry }, format: 'json') diff --git a/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/content_elements_controller_spec.rb b/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/content_elements_controller_spec.rb index d81e5f898d..7f00742f63 100644 --- a/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/content_elements_controller_spec.rb +++ b/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/content_elements_controller_spec.rb @@ -9,11 +9,12 @@ module PageflowScrolled describe '#batch' do it 'requires authentication' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: section.id, content_elements: [] @@ -23,13 +24,14 @@ module PageflowScrolled end it 'allows ordering content elements referenced by id' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) content_elements = create_list(:content_element, 2, section: section) authorize_for_editor_controller(entry) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: section.id, content_elements: [ @@ -43,7 +45,7 @@ module PageflowScrolled end it 'allows moving content elements to different section' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) other_section = create(:section, revision: entry.draft) content_element = create(:content_element, :text_block, section: section) @@ -51,6 +53,7 @@ module PageflowScrolled authorize_for_editor_controller(entry) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: other_section, content_elements: [ @@ -62,15 +65,16 @@ module PageflowScrolled end it 'does not allow moving content elements to different entry' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) - other_entry = create(:entry) + other_entry = create(:entry, type_name: 'scrolled') other_section = create(:section, revision: other_entry.draft) content_element = create(:content_element, section: section) authorize_for_editor_controller(entry) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: other_section, content_elements: [ @@ -82,13 +86,14 @@ module PageflowScrolled end it 'allows setting content element configuration hashes' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) content_element = create(:content_element, section: section) authorize_for_editor_controller(entry) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: section.id, content_elements: [ @@ -100,7 +105,7 @@ module PageflowScrolled end it 'does not change configuration hash if not passed' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) content_element = create(:content_element, section: section, @@ -109,6 +114,7 @@ module PageflowScrolled authorize_for_editor_controller(entry) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: section.id, content_elements: [ @@ -120,12 +126,13 @@ module PageflowScrolled end it 'allows creating content elements' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) authorize_for_editor_controller(entry) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: section.id, content_elements: [ @@ -139,13 +146,14 @@ module PageflowScrolled end it 'deletes content elements marked by _delete flag' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) content_elements = create_list(:content_element, 2, section: section) authorize_for_editor_controller(entry) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: section.id, content_elements: [ @@ -158,12 +166,13 @@ module PageflowScrolled end it 'responds with array of objects containging content element ids and perma ids' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) authorize_for_editor_controller(entry) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: section.id, content_elements: [ @@ -180,12 +189,13 @@ module PageflowScrolled end it 'does not create content elements if id is unknown' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) authorize_for_editor_controller(entry) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: section.id, content_elements: [ @@ -197,12 +207,13 @@ module PageflowScrolled end it 'rolls back other changes if one id is unknown' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) authorize_for_editor_controller(entry) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: section.id, content_elements: [ @@ -215,7 +226,7 @@ module PageflowScrolled end it 'allows performing a mix of update, create and delete operations' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) content_elements = create_list(:content_element, 3, @@ -225,6 +236,7 @@ module PageflowScrolled authorize_for_editor_controller(entry) post(:batch, params: { + entry_type: 'scrolled', entry_id: entry.id, section_id: section.id, content_elements: [ @@ -263,11 +275,12 @@ module PageflowScrolled describe '#create' do it 'requires authentication' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, section_id: section, content_element: attributes_for(:content_element, :text_block) @@ -277,12 +290,13 @@ module PageflowScrolled end it 'succeeds for authorized user' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) authorize_for_editor_controller(entry) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, section_id: section, content_element: attributes_for(:content_element, :text_block) @@ -292,12 +306,13 @@ module PageflowScrolled end it 'allows setting the content elements configuration hash' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) authorize_for_editor_controller(entry) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, section_id: section, content_element: { @@ -309,12 +324,13 @@ module PageflowScrolled end it 'allows setting the content elements type name' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) authorize_for_editor_controller(entry) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, section_id: section, content_element: { @@ -326,12 +342,13 @@ module PageflowScrolled end it 'can handle camel case attributes' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) authorize_for_editor_controller(entry) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, section_id: section, content_element: { @@ -343,12 +360,13 @@ module PageflowScrolled end it 'renders attributes as camel case' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) authorize_for_editor_controller(entry) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, section_id: section, content_element: attributes_for(:content_element, :text_block) @@ -359,12 +377,13 @@ module PageflowScrolled describe '#update' do it 'allows updating the content elements configuration hash' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') content_element = create(:content_element, :text_block, revision: entry.draft) authorize_for_editor_controller(entry) patch(:update, params: { + entry_type: 'scrolled', entry_id: entry, id: content_element, content_element: { @@ -377,14 +396,15 @@ module PageflowScrolled end it 'does not allow updating a content element from a different entry' do - entry = create(:entry) - other_entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') + other_entry = create(:entry, type_name: 'scrolled') content_element_in_other_entry = create(:content_element, revision: other_entry.draft) authorize_for_editor_controller(entry) patch(:update, params: { + entry_type: 'scrolled', entry_id: entry, id: content_element_in_other_entry, content_element: { @@ -398,7 +418,7 @@ module PageflowScrolled describe '#order' do it 'updates position of content elements according to given params order' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) content_elements = create_list(:content_element, 2, :text_block, section: section) @@ -406,6 +426,7 @@ module PageflowScrolled put(:order, params: { + entry_type: 'scrolled', entry_id: entry, section_id: section, ids: [content_elements.first.id, content_elements.last.id] @@ -416,14 +437,15 @@ module PageflowScrolled end it 'does not allow moving a content element to a section of a different entry' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') content_element = create(:content_element, revision: entry.draft) - other_entry = create(:entry) + other_entry = create(:entry, type_name: 'scrolled') section_in_other_entry = create(:section, revision: other_entry.draft) authorize_for_editor_controller(entry) put(:order, params: { + entry_type: 'scrolled', entry_id: entry, section_id: section_in_other_entry, ids: [content_element.id] @@ -435,13 +457,14 @@ module PageflowScrolled describe '#destroy' do it 'deletes the content element' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) content_element = create(:content_element, :text_block, section: section) authorize_for_editor_controller(entry) delete(:destroy, params: { + entry_type: 'scrolled', entry_id: entry, id: content_element }, format: 'json') @@ -451,13 +474,14 @@ module PageflowScrolled end it 'does not allow deleting a content element from a different entry' do - entry = create(:entry) - other_entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') + other_entry = create(:entry, type_name: 'scrolled') content_element_in_other_entry = create(:content_element, revision: other_entry.draft) authorize_for_editor_controller(entry) delete(:destroy, params: { + entry_type: 'scrolled', entry_id: entry, id: content_element_in_other_entry }, format: 'json') diff --git a/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/sections_controller_spec.rb b/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/sections_controller_spec.rb index b845d8d1ac..da1a3d817f 100644 --- a/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/sections_controller_spec.rb +++ b/entry_types/scrolled/spec/controllers/pageflow_scrolled/editor/sections_controller_spec.rb @@ -9,11 +9,12 @@ module PageflowScrolled describe '#create' do it 'requires authentication' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapter = create(:scrolled_chapter, revision: entry.draft) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, chapter_id: chapter, section: attributes_for(:section) @@ -23,12 +24,13 @@ module PageflowScrolled end it 'succeeds for authorized user' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapter = create(:scrolled_chapter, revision: entry.draft) authorize_for_editor_controller(entry) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, chapter_id: chapter, section: attributes_for(:section) @@ -38,12 +40,13 @@ module PageflowScrolled end it 'allows setting the sections configuration hash' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapter = create(:scrolled_chapter, revision: entry.draft) authorize_for_editor_controller(entry) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, chapter_id: chapter, section: { @@ -55,12 +58,13 @@ module PageflowScrolled end it 'renders attributes as camel case' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapter = create(:scrolled_chapter, revision: entry.draft) authorize_for_editor_controller(entry) post(:create, params: { + entry_type: 'scrolled', entry_id: entry, chapter_id: chapter, section: attributes_for(:section) @@ -71,12 +75,13 @@ module PageflowScrolled describe '#update' do it 'allows updating the sections configuration hash' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) authorize_for_editor_controller(entry) patch(:update, params: { + entry_type: 'scrolled', entry_id: entry, id: section, section: { @@ -89,14 +94,15 @@ module PageflowScrolled end it 'does not allow updating a section from a different entry' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') create(:section, revision: entry.draft) - other_entry = create(:entry) + other_entry = create(:entry, type_name: 'scrolled') section_in_other_entry = create(:section, revision: other_entry.draft) authorize_for_editor_controller(entry) patch(:update, params: { + entry_type: 'scrolled', entry_id: entry, id: section_in_other_entry, chapter: { @@ -110,13 +116,14 @@ module PageflowScrolled describe '#order' do it 'updates position of sections according to given params order' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapter = create(:scrolled_chapter, revision: entry.draft) sections = create_list(:section, 2, chapter: chapter) authorize_for_editor_controller(entry) put(:order, params: { + entry_type: 'scrolled', entry_id: entry, chapter_id: chapter, ids: [sections.first.id, sections.last.id] @@ -127,7 +134,7 @@ module PageflowScrolled end it 'allows moving a section from one chapter to another within the same entry' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') revision = entry.draft chapter = create(:scrolled_chapter, revision: revision) section = create(:section, chapter: chapter) @@ -136,6 +143,7 @@ module PageflowScrolled authorize_for_editor_controller(entry) put(:order, params: { + entry_type: 'scrolled', entry_id: entry, chapter_id: other_chapter, ids: [section.id] @@ -145,15 +153,16 @@ module PageflowScrolled end it 'does not allow moving a section to a chapter of another entry' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') chapter = create(:scrolled_chapter, revision: entry.draft) section = create(:section, chapter: chapter) - other_entry = create(:entry) + other_entry = create(:entry, type_name: 'scrolled') chapter_in_other_entry = create(:scrolled_chapter, revision: other_entry.draft) authorize_for_editor_controller(entry) put(:order, params: { + entry_type: 'scrolled', entry_id: entry, chapter_id: chapter_in_other_entry, ids: [section.id] @@ -165,13 +174,14 @@ module PageflowScrolled describe '#destroy' do it 'deletes the section' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') section = create(:section, revision: entry.draft) chapter = section.chapter authorize_for_editor_controller(entry) delete(:destroy, params: { + entry_type: 'scrolled', entry_id: entry, id: section }, format: 'json') @@ -181,14 +191,15 @@ module PageflowScrolled end it 'does not allow deleting a section from a different entry' do - entry = create(:entry) + entry = create(:entry, type_name: 'scrolled') create(:section, revision: entry.draft) - other_entry = create(:entry) + other_entry = create(:entry, type_name: 'scrolled') section_in_other_entry = create(:section, revision: other_entry.draft) authorize_for_editor_controller(entry) delete(:destroy, params: { + entry_type: 'scrolled', entry_id: entry, id: section_in_other_entry }, format: 'json') From c22ad25eab0c021615ef68dfcaca07dd5696fb1b Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 18 Sep 2023 11:14:39 +0200 Subject: [PATCH 3/5] Make file fixtures work in Rails 6 REDMINE-19438 --- entry_types/scrolled/spec/spec_helper.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/entry_types/scrolled/spec/spec_helper.rb b/entry_types/scrolled/spec/spec_helper.rb index bfda4936ee..c9689a6f87 100644 --- a/entry_types/scrolled/spec/spec_helper.rb +++ b/entry_types/scrolled/spec/spec_helper.rb @@ -22,7 +22,11 @@ # Enable flags like --only-failures and --next-failure config.example_status_persistence_file_path = '.rspec_status' - config.fixture_path = './spec/fixtures' + if Pageflow::RailsVersion.experimental? + config.file_fixture_path = './spec/fixtures' + else + config.fixture_path = './spec/fixtures' + end # Disable RSpec exposing methods globally on `Module` and `main` config.disable_monkey_patching! From 2e2e095b024d3fe662fc02c904efbcfff1b3a88c Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 18 Sep 2023 11:22:18 +0200 Subject: [PATCH 4/5] Use Shakapacker instead of Webpacker with Rails 6 REDMINE-19438 --- .github/workflows/tests.yml | 2 +- Gemfile | 18 ++++-- .../helpers/pageflow_scrolled/packs_helper.rb | 41 +++++++++--- .../pageflow_scrolled/themes_helper.rb | 4 +- .../editor/entries/_head.html.erb | 7 ++- .../install/install_generator.rb | 62 ++++++++++++++++++- .../scrolled/spec/support/config/webpacker.rb | 14 +++-- package/config/webpack5.js | 14 +++++ pageflow.gemspec | 6 +- spec/support/pageflow/dummy/app.rb | 4 +- spec/support/pageflow/dummy/rails_template.rb | 6 +- 11 files changed, 150 insertions(+), 28 deletions(-) create mode 100644 package/config/webpack5.js diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 70135cbde8..6cc786922b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -152,7 +152,7 @@ jobs: - name: Precompile working-directory: ${{ matrix.engine-directory }} run: | - WEBPACKER_PRECOMPILE=false bin/rake app:assets:precompile + WEBPACKER_PRECOMPILE=false SHAKAPACKER_PRECOMPILE=false bin/rake app:assets:precompile # Test diff --git a/Gemfile b/Gemfile index e24e4993f5..f58fa80db5 100644 --- a/Gemfile +++ b/Gemfile @@ -23,11 +23,19 @@ end # Required for XML serialization in Active Admin gem 'activemodel-serializers-xml' -# Make webpacker available in specs. Host applications that want to -# use webpacker need to add it to their Gemfile themselves. Requiring -# webpacker in an engine file (like we normally do) would force all -# host application to install webpacker. -gem 'webpacker' +if Pageflow::RailsVersion.experimental? + # Make shakapacker available in specs. Host applications that want to + # use shakapacker need to add it to their Gemfile themselves. Requiring + # shakapacker in an engine file (like we normally do) would force all + # host application to install shakapacker. + gem 'shakapacker' +else + # Make webpacker available in specs. Host applications that want to + # use webpacker need to add it to their Gemfile themselves. Requiring + # webpacker in an engine file (like we normally do) would force all + # host application to install webpacker. + gem 'webpacker' +end # Make tests fail on JS errors gem 'capybara-chromedriver-logger', git: 'https://github.com/codevise/capybara-chromedriver-logger', branch: 'fix-selenium-4-deprecation', require: false diff --git a/entry_types/scrolled/app/helpers/pageflow_scrolled/packs_helper.rb b/entry_types/scrolled/app/helpers/pageflow_scrolled/packs_helper.rb index eb0a4f44b3..f7fa6f4c11 100644 --- a/entry_types/scrolled/app/helpers/pageflow_scrolled/packs_helper.rb +++ b/entry_types/scrolled/app/helpers/pageflow_scrolled/packs_helper.rb @@ -2,22 +2,43 @@ module PageflowScrolled # @api private module PacksHelper def scrolled_frontend_javascript_packs_tag(entry, options) - javascript_packs_with_chunks_tag( - *scrolled_frontend_packs(entry, options) - ) + if Pageflow::RailsVersion.experimental? + javascript_pack_tag( + *scrolled_frontend_packs(entry, **options), + defer: false + ) + else + javascript_packs_with_chunks_tag( + *scrolled_frontend_packs(entry, **options) + ) + end end def scrolled_frontend_stylesheet_packs_tag(entry, options) - stylesheet_packs_with_chunks_tag( - *scrolled_frontend_packs(entry, options), - media: 'all' - ) + if Pageflow::RailsVersion.experimental? + stylesheet_pack_tag( + *scrolled_frontend_packs(entry, **options), + media: 'all' + ) + else + stylesheet_packs_with_chunks_tag( + *scrolled_frontend_packs(entry, **options), + media: 'all' + ) + end end def scrolled_editor_javascript_packs_tag(entry) - javascript_packs_with_chunks_tag( - *scrolled_editor_packs(entry) - ) + if Pageflow::RailsVersion.experimental? + javascript_pack_tag( + *scrolled_editor_packs(entry), + defer: false + ) + else + javascript_packs_with_chunks_tag( + *scrolled_editor_packs(entry) + ) + end end def scrolled_frontend_packs(entry, widget_scope:) diff --git a/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb b/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb index d241cf0285..97ea282d64 100644 --- a/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb +++ b/entry_types/scrolled/app/helpers/pageflow_scrolled/themes_helper.rb @@ -6,9 +6,11 @@ def scrolled_theme_asset_path(theme, theme_file_role: nil, theme_file_style: :resized, relative_url: false) + prefix = Pageflow::RailsVersion.experimental? ? 'static' : 'media' + path = theme.files.dig(theme_file_role, theme_file_style) || - asset_pack_path("media/pageflow-scrolled/themes/#{theme.name}/#{path}") + asset_pack_path("#{prefix}/pageflow-scrolled/themes/#{theme.name}/#{path}") if relative_url URI.parse(path).path diff --git a/entry_types/scrolled/app/views/pageflow_scrolled/editor/entries/_head.html.erb b/entry_types/scrolled/app/views/pageflow_scrolled/editor/entries/_head.html.erb index fc79c9bfbc..4631436ea9 100644 --- a/entry_types/scrolled/app/views/pageflow_scrolled/editor/entries/_head.html.erb +++ b/entry_types/scrolled/app/views/pageflow_scrolled/editor/entries/_head.html.erb @@ -1,5 +1,10 @@ <%= stylesheet_link_tag 'pageflow_paged/editor', media: 'all' %> -<%= stylesheet_packs_with_chunks_tag 'pageflow-scrolled-frontend' %> + +<% if Pageflow::RailsVersion.experimental? %> + <%= stylesheet_pack_tag 'pageflow-scrolled-frontend' %> +<% else %> + <%= stylesheet_packs_with_chunks_tag 'pageflow-scrolled-frontend' %> +<% end %> <%= scrolled_theme_properties_style_tag(entry.theme) %> <%= scrolled_theme_stylesheet_pack_tags(entry.theme) %> diff --git a/entry_types/scrolled/lib/generators/pageflow_scrolled/install/install_generator.rb b/entry_types/scrolled/lib/generators/pageflow_scrolled/install/install_generator.rb index 25d032fa3d..f7d5f16ad6 100644 --- a/entry_types/scrolled/lib/generators/pageflow_scrolled/install/install_generator.rb +++ b/entry_types/scrolled/lib/generators/pageflow_scrolled/install/install_generator.rb @@ -30,10 +30,66 @@ def theme_plugin end def install_packages - run 'yarn add postcss-url@^8.0.0 @fontsource/source-sans-pro' + if Pageflow::RailsVersion.experimental? + run 'yarn add css-loader style-loader' \ + ' mini-css-extract-plugin css-minimizer-webpack-plugin' \ + ' postcss postcss-loader postcss-url' \ + ' @fontsource/source-sans-pro' + else + run 'yarn add postcss-url@^8.0.0 @fontsource/source-sans-pro' + end + end + + def webpack_config + return unless Pageflow::RailsVersion.experimental? + + gsub_file( + 'config/webpack/webpack.config.js', + "const { generateWebpackConfig } = require('shakapacker')", + "const { generateWebpackConfig, merge, mergeWithRules } = require('shakapacker')" + ) + + gsub_file( + 'config/webpack/webpack.config.js', + 'const webpackConfig = generateWebpackConfig()', + <<~JS + const webpackConfig = merge( + generateWebpackConfig(), + require('pageflow/config/webpack5'), + require('pageflow-scrolled/config/webpack') + ) + JS + ) + + gsub_file( + 'config/webpack/webpack.config.js', + 'module.exports = webpackConfig', + <<~JS + // Extend file rule to include mp3 extension + module.exports = mergeWithRules({ + module: { + rules: { + test: 'replace', + type: 'match' + }, + }, + })(webpackConfig, { + module: { + rules: [ + { + test: /\.(bmp|gif|jpe?g|png|tiff|ico|avif|webp|eot|otf|ttf|woff|woff2|svg|mp3)$/, + type: 'asset/resource' + } + ] + } + }) + JS + ) end def webpack_environment + return if Pageflow::RailsVersion.experimental? + inject_into_file('config/webpack/environment.js', before: "module.exports = environment\n") do "environment.config.merge(require('pageflow/config/webpack'))\n" \ @@ -53,6 +109,8 @@ def webpack_environment end def webpacker_yml + return if Pageflow::RailsVersion.experimental? + gsub_file('config/webpacker.yml', 'extract_css: false', 'extract_css: true') @@ -64,6 +122,8 @@ def webpacker_yml end def postcss_config + return if Pageflow::RailsVersion.experimental? + inject_into_file('postcss.config.js', after: "require('postcss-import'),\n") do " // Make relative urls in fontsource packages work\n" \ diff --git a/entry_types/scrolled/spec/support/config/webpacker.rb b/entry_types/scrolled/spec/support/config/webpacker.rb index 767d45e098..86cb285367 100644 --- a/entry_types/scrolled/spec/support/config/webpacker.rb +++ b/entry_types/scrolled/spec/support/config/webpacker.rb @@ -1,8 +1,10 @@ -# Make sure packs are recompiled for Capybara specs when Rollup -# outputs change +unless Pageflow::RailsVersion.experimental? + # Make sure packs are recompiled for Capybara specs when Rollup + # outputs change -Webpacker::Compiler.watched_paths << Pageflow::Engine.root.join('package/*.js') -Webpacker::Compiler.watched_paths << PageflowScrolled::Engine.root.join('package/*.js') + Webpacker::Compiler.watched_paths << Pageflow::Engine.root.join('package/*.js') + Webpacker::Compiler.watched_paths << PageflowScrolled::Engine.root.join('package/*.js') -# see https://github.com/rails/webpacker/issues/2410 -Webpacker::Compiler.watched_paths << Rails.root.join('app/javascript/**/*.js') + # see https://github.com/rails/webpacker/issues/2410 + Webpacker::Compiler.watched_paths << Rails.root.join('app/javascript/**/*.js') +end diff --git a/package/config/webpack5.js b/package/config/webpack5.js new file mode 100644 index 0000000000..3c82a2511f --- /dev/null +++ b/package/config/webpack5.js @@ -0,0 +1,14 @@ +module.exports = { + externals: { + 'backbone': 'Backbone', + 'backbone.babysitter': 'Backbone.ChildViewContainer', + 'cocktail': 'Cocktail', + 'jquery': 'jQuery', + 'jquery-ui': 'jQuery', + 'jquery.minicolors': 'jQuery', + 'underscore': '_', + 'backbone.marionette': 'Backbone.Marionette', + 'iscroll': 'IScroll', + 'wysihtml5': 'wysihtml5' + } +}; diff --git a/pageflow.gemspec b/pageflow.gemspec index d58ffa1f81..a34d479a1f 100644 --- a/pageflow.gemspec +++ b/pageflow.gemspec @@ -129,7 +129,11 @@ Gem::Specification.new do |s| s.add_dependency 'sprockets', '< 4' # Used for Webpack build in host application - s.add_dependency 'webpacker', '~> 4.2' + if Pageflow::RailsVersion.experimental? + s.add_dependency 'shakapacker', '~> 7.0' + else + s.add_dependency 'webpacker', '~> 4.2' + end # Using translations from rails locales in javascript code. s.add_dependency 'i18n-js', '~> 2.1' diff --git a/spec/support/pageflow/dummy/app.rb b/spec/support/pageflow/dummy/app.rb index cae3c8fdef..a606619393 100644 --- a/spec/support/pageflow/dummy/app.rb +++ b/spec/support/pageflow/dummy/app.rb @@ -29,7 +29,9 @@ def template_path end def rails_new_options - '--skip-test-unit --skip-bundle --database=mysql' + result = '--skip-test-unit --skip-bundle --database=mysql' + result << ' --skip-javascript' if Pageflow::RailsVersion.experimental? + result end end end diff --git a/spec/support/pageflow/dummy/rails_template.rb b/spec/support/pageflow/dummy/rails_template.rb index 5dd092b1b4..84eb7acf5e 100644 --- a/spec/support/pageflow/dummy/rails_template.rb +++ b/spec/support/pageflow/dummy/rails_template.rb @@ -42,7 +42,11 @@ def source_paths # Install Webpacker -rake 'webpacker:install' if ENV['PAGEFLOW_INSTALL_WEBPACKER'] == 'true' +if Pageflow::RailsVersion.experimental? + rake 'shakapacker:install' if ENV['PAGEFLOW_INSTALL_WEBPACKER'] == 'true' +else + rake 'webpacker:install' if ENV['PAGEFLOW_INSTALL_WEBPACKER'] == 'true' +end # Install pageflow and the tested engine via their generators. From b994fb70c036d60916e303c79075b0e80513efce Mon Sep 17 00:00:00 2001 From: Tim Fischbach Date: Mon, 18 Sep 2023 11:24:11 +0200 Subject: [PATCH 5/5] Run Pageflow Scrolled specs against rails 6 REDMINE-19438 --- .github/workflows/tests.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6cc786922b..2be4affd00 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -49,12 +49,26 @@ jobs: plugin-name: pageflow_scrolled rspec-command: bin/rspec --tag ~js + - engine-name: pageflow_scrolled + ruby-version: 3.2 + rails-version: "~> 6.1" + engine-directory: entry_types/scrolled + plugin-name: pageflow_scrolled + rspec-command: bin/rspec --tag ~js + - engine-name: pageflow_scrolled ruby-version: 2.6 engine-directory: entry_types/scrolled plugin-name: pageflow_scrolled rspec-command: bin/rspec-with-retry-on-timeout --tag js + - engine-name: pageflow_scrolled + ruby-version: 3.2 + rails-version: "~> 6.1" + engine-directory: entry_types/scrolled + plugin-name: pageflow_scrolled + rspec-command: bin/rspec-with-retry-on-timeout --tag js + env: PAGEFLOW_RAILS_VERSION: ${{ matrix.rails-version }}