From f89b970cc43e101dfb8e59864a2388821bdd8817 Mon Sep 17 00:00:00 2001 From: Phil Date: Thu, 22 Jan 2026 21:49:46 +0000 Subject: [PATCH 1/4] Added embedded_block edit mode URL support - fixes #325 --- .github/copilot-instructions.md | 1 + app/assets/javascripts/app/_fpa.js | 4 +- .../app/_fpa_ajax_processors_reports.js | 17 ++ .../reports_common_result_cell.rb | 15 +- .../reports_common_result_cell_spec.rb | 170 +++++++++++ spec/system/report_embedded_block_spec.rb | 288 ++++++++++++++++++ 6 files changed, 490 insertions(+), 5 deletions(-) create mode 100644 spec/helpers/report_results/reports_common_result_cell_spec.rb create mode 100644 spec/system/report_embedded_block_spec.rb diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 09c323cac4..a9b71cd838 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -10,6 +10,7 @@ 5. **If requirements are not clear, ask for clarification before proceeding** 6. **Never commit directly to `up-develop` or `develop` branches** - always create feature branches and pull requests 7. **Focus on configuration over code** - most features should be achievable through admin panel settings rather than new Ruby code +8. **Create new files and edit directly in the editor**; avoid using command line file operations unless absolutely necessary ### Critical Rules for Running Terminal Commands diff --git a/app/assets/javascripts/app/_fpa.js b/app/assets/javascripts/app/_fpa.js index 558f84cd4f..a20bf98639 100644 --- a/app/assets/javascripts/app/_fpa.js +++ b/app/assets/javascripts/app/_fpa.js @@ -1316,8 +1316,8 @@ _fpa = { }) if (modal_index) { - // Hide a previously shown modal back - $('.modal.in').removeClass('in').addClass('was-in'); + // Hide a previously shown modal back, but not the one we're currently showing + $('.modal.in').not(pm).removeClass('in').addClass('was-in'); pm.on('click.dismiss.bs.modal', `[data-dismiss="modal${modal_index}"]`, function () { _fpa.hide_modal(modal_index); diff --git a/app/assets/javascripts/app/_fpa_ajax_processors_reports.js b/app/assets/javascripts/app/_fpa_ajax_processors_reports.js index 2af2853de4..cc87505909 100644 --- a/app/assets/javascripts/app/_fpa_ajax_processors_reports.js +++ b/app/assets/javascripts/app/_fpa_ajax_processors_reports.js @@ -41,6 +41,7 @@ _fpa.postprocessors_reports = { var hyph_name = us_name.hyphenate() var id = block.attr('data-id') var master_id = block.attr('data-master-id') + var edit_mode = block.attr('data-edit-mode') === 'true' var target_block = "report-result-embedded-block" var html = $(`
`) if ($(block).contents().length == 0) { @@ -61,6 +62,22 @@ _fpa.postprocessors_reports = { sv_opt.allow_actions = _fpa.state.user_can; _fpa.secure_view.setup_links($target_block, 'a.redcap-file-use-secure-view', sv_opt); + + // If edit mode, click the edit button to open the form directly + if (edit_mode) { + var edit_btn = $target_block.find('.edit-entity.glyphicon-pencil').first(); + if (edit_btn.length) { + edit_btn.click(); + } + + // Set up handler to close modal on successful save + $target_block.on('ajax:success', 'form', function (event) { + // Close the modal after a successful save + window.setTimeout(function () { + _fpa.hide_modal(1); + }, 300); + }); + } }, 500); }, 500); }, diff --git a/app/helpers/report_results/reports_common_result_cell.rb b/app/helpers/report_results/reports_common_result_cell.rb index acd12c63de..ea911cce70 100644 --- a/app/helpers/report_results/reports_common_result_cell.rb +++ b/app/helpers/report_results/reports_common_result_cell.rb @@ -192,16 +192,25 @@ def cell_content_for_embedded_block icon = 'glyphicon glyphicon-tasks' end - split_url = url.split('/') + # Detect if this is an edit mode URL (ends with /edit) + edit_mode = url.end_with?('/edit') + split_url = url.split('/') split_url = split_url.reject(&:blank?) + + # If edit mode, remove 'edit' from the end before extracting id + split_url.pop if edit_mode + id = split_url.last master_id = split_url[1] if split_url.first == 'masters' hyph_name = split_url[-3..-2]&.join('__')&.hyphenate&.singularize || '' + # Add edit-mode attribute if this is an edit URL + edit_mode_attr = edit_mode ? ' data-edit-mode="true"' : '' + html = <<~END_HTML - #{a_text} -
+ #{a_text} +
END_HTML html.html_safe diff --git a/spec/helpers/report_results/reports_common_result_cell_spec.rb b/spec/helpers/report_results/reports_common_result_cell_spec.rb new file mode 100644 index 0000000000..e5f3fc05bf --- /dev/null +++ b/spec/helpers/report_results/reports_common_result_cell_spec.rb @@ -0,0 +1,170 @@ +# frozen_string_literal: true + +# ReportsCommonResultCell Spec +# +# Tests the cell content rendering for report table cells, specifically focusing on +# the embedded_block feature that displays dynamic model and activity log records +# in a modal dialog when clicked. +# +# Test Coverage: +# - #cell_content_for_embedded_block: Generates HTML for opening records in a modal +# - Handles plain URLs like /dynamic_model/table_name/123 +# - Handles markdown format links like [Label](/dynamic_model/table_name/123) +# - Handles activity log URLs like /activity_log/log_type/456 +# - NEW: Handles edit mode URLs ending with /edit (GitHub #325) +# - /dynamic_model/table_name/123/edit should include data-edit-mode="true" +# - Modal should open directly in edit mode +# - Save should close the modal dialog + +require 'rails_helper' + +RSpec.describe ReportResults::ReportsCommonResultCell do + let(:table_name) { 'dynamic_model__test_items' } + let(:col_name) { 'edit_link' } + let(:col_tag) { nil } + let(:col_show_as) { 'embedded_block' } + let(:selection_options) { double('selection_options') } + + def build_cell(content) + described_class.new(table_name, content, col_name, col_tag, col_show_as, selection_options) + end + + describe '#cell_content_for_embedded_block' do + context 'with dynamic model show URLs (existing behavior)' do + it 'returns the content unchanged if blank' do + cell = build_cell('') + expect(cell.cell_content_for_embedded_block).to eq('') + end + + it 'returns nil if content is nil' do + cell = build_cell(nil) + expect(cell.cell_content_for_embedded_block).to be_nil + end + + it 'parses plain URL and generates correct HTML with show mode attributes' do + url = '/dynamic_model/datadic_variables/123' + cell = build_cell(url) + html = cell.cell_content_for_embedded_block + + # Should contain the link + expect(html).to include('href="/dynamic_model/datadic_variables/123"') + expect(html).to include('data-remote="true"') + expect(html).to include('class="report-embedded-block-link glyphicon glyphicon-tasks"') + + # Should contain the hidden div for the modal content + expect(html).to include('id="report-result-embedded-block--123"') + expect(html).to include('data-preprocessor="report_embed_dynamic_block"') + expect(html).to include('data-model-name="dynamic_model__datadic_variable"') + expect(html).to include('data-id="123"') + + # Should NOT contain edit-mode attribute (show mode) + expect(html).not_to include('data-edit-mode') + end + + it 'parses markdown format link correctly' do + url = '[Edit Item](/dynamic_model/datadic_variables/456)' + cell = build_cell(url) + html = cell.cell_content_for_embedded_block + + # Should contain the link text + expect(html).to include('>Edit Item') + expect(html).to include('href="/dynamic_model/datadic_variables/456"') + + # Should have correct id + expect(html).to include('data-id="456"') + expect(html).to include('id="report-result-embedded-block--456"') + + # Should NOT contain edit-mode attribute + expect(html).not_to include('data-edit-mode') + end + + it 'extracts master_id from URL when present' do + url = '/masters/789/dynamic_model/test_items/123' + cell = build_cell(url) + html = cell.cell_content_for_embedded_block + + expect(html).to include('data-master-id="789"') + expect(html).to include('data-id="123"') + end + + it 'generates correct hyphenated model name for dynamic models' do + url = '/dynamic_model/datadic_variables/123' + cell = build_cell(url) + html = cell.cell_content_for_embedded_block + + # dynamic_model + datadic_variables -> dynamic_model__datadic_variable (singularized, hyphenated) + expect(html).to include('data-model-name="dynamic_model__datadic_variable"') + expect(html).to include('data-dynamic-model--datadic-variable-id="123"') + end + end + + context 'with activity log show URLs (existing behavior)' do + it 'parses activity log URL and generates correct HTML' do + url = '/activity_log/test_processes/456' + cell = build_cell(url) + html = cell.cell_content_for_embedded_block + + expect(html).to include('href="/activity_log/test_processes/456"') + expect(html).to include('data-id="456"') + expect(html).to include('data-model-name="activity_log__test_process"') + + # Should NOT contain edit-mode attribute + expect(html).not_to include('data-edit-mode') + end + end + + context 'with edit mode URLs ending in /edit (GitHub #325)' do + it 'parses dynamic model edit URL and includes edit-mode attribute' do + url = '/dynamic_model/datadic_variables/123/edit' + cell = build_cell(url) + html = cell.cell_content_for_embedded_block + + # Should extract the correct id (not "edit") + expect(html).to include('data-id="123"') + expect(html).not_to include('data-id="edit"') + + # Should include edit-mode attribute + expect(html).to include('data-edit-mode="true"') + + # Should still have correct model name + expect(html).to include('data-model-name="dynamic_model__datadic_variable"') + + # Should have the full URL including /edit + expect(html).to include('href="/dynamic_model/datadic_variables/123/edit"') + end + + it 'parses markdown format edit URL correctly' do + url = '[Edit Now](/dynamic_model/datadic_variables/789/edit)' + cell = build_cell(url) + html = cell.cell_content_for_embedded_block + + expect(html).to include('>Edit Now') + expect(html).to include('data-id="789"') + expect(html).to include('data-edit-mode="true"') + expect(html).not_to include('data-id="edit"') + end + + it 'parses activity log edit URL and includes edit-mode attribute' do + url = '/activity_log/test_processes/456/edit' + cell = build_cell(url) + html = cell.cell_content_for_embedded_block + + expect(html).to include('data-id="456"') + expect(html).not_to include('data-id="edit"') + expect(html).to include('data-edit-mode="true"') + expect(html).to include('data-model-name="activity_log__test_process"') + end + + it 'handles master_id correctly with edit URLs' do + url = '/masters/999/dynamic_model/test_items/123/edit' + cell = build_cell(url) + html = cell.cell_content_for_embedded_block + + expect(html).to include('data-master-id="999"') + expect(html).to include('data-id="123"') + expect(html).to include('data-edit-mode="true"') + expect(html).not_to include('data-id="edit"') + end + end + end +end diff --git a/spec/system/report_embedded_block_spec.rb b/spec/system/report_embedded_block_spec.rb new file mode 100644 index 0000000000..376017b44a --- /dev/null +++ b/spec/system/report_embedded_block_spec.rb @@ -0,0 +1,288 @@ +# frozen_string_literal: true + +# Report Embedded Block System Spec +# +# Tests the embedded_block feature in reports that displays dynamic model and activity log +# records in a modal dialog when clicked. This is a UI test that verifies the full interaction. +# +# Test Coverage: +# - Clicking embedded_block link with show URL opens modal in show mode (existing behavior) +# - Clicking embedded_block link with edit URL (ending in /edit) opens modal in edit mode (GitHub #325) +# - When edit mode modal is saved, the modal should close automatically +# - Works for both dynamic models and activity logs + +require 'rails_helper' + +describe 'report embedded_block', js: true, driver: $browser_driver do + include ModelSupport + include MasterDataSupport + include FeatureSupport + include TestFieldsDmSupport + + before(:all) do + change_setting('TwoFactorAuthDisabledForUser', true) + change_setting('TwoFactorAuthDisabledForAdmin', true) + SetupHelper.feature_setup + + @admin, = create_admin + seed_database + create_data_set_outside_tx + + # Must create user BEFORE setup_fields_dm since it requires @user + setup_embedded_block_test_user + + setup_fields_dm # Sets up dynamic_model__test_with_id_recs + + # Reload routes after setting up dynamic model + DynamicModel.routes_reload + Rails.application.routes_reloader.reload! + + setup_embedded_block_test_data + setup_embedded_block_reports + end + + def setup_embedded_block_test_user + @user, @good_password = create_user + @good_email = @user.email + + Admin::UserAccessControl.create!( + app_type_id: @user.app_type_id, + access: :read, + resource_type: :general, + resource_name: :view_reports, + current_admin: @admin, + user: @user + ) + + setup_access :dynamic_model__test_with_id_recs, user: @user + + expect(@user.can?(:view_reports)).to be_truthy + end + + def setup_embedded_block_test_data + # Create a test record we can display in embedded_block + @master = Master.create!(current_user: @user) + @test_record = DynamicModel::TestWithIdRec.create!( + master: @master, + name: 'Embedded Block Test Record', + value: 'Test Value 123', + current_user: @user + ) + expect(@test_record).to be_persisted + end + + def setup_embedded_block_reports + # Report with embedded_block using show URL (existing behavior) + # Dynamic model routes require master_id: /masters/{master_id}/dynamic_model/table_name/{id} + # Use subquery to get the latest record dynamically + sql_show = <<~SQL + SELECT + '/masters/' || master_id || '/dynamic_model/test_with_id_recs/' || id AS show_link, + name + FROM dynamic_test.test_with_id_recs + ORDER BY id DESC + LIMIT 1 + SQL + + options_show = <<~YAML + column_options: + show_as: + show_link: embedded_block + YAML + + @report_show = Report.create!( + current_admin: @admin, + name: "Embedded Block Show Mode #{SecureRandom.hex(4)}", + description: 'Test embedded_block in show mode', + sql: sql_show, + options: options_show, + disabled: false, + report_type: 'regular_report', + auto: false, + searchable: false + ) + + Admin::UserAccessControl.create!( + app_type: @user.app_type, + access: :read, + resource_type: :report, + resource_name: @report_show.alt_resource_name, + current_admin: @admin + ) + + # Report with embedded_block using edit URL (new GitHub #325 feature) + sql_edit = <<~SQL + SELECT + '/masters/' || master_id || '/dynamic_model/test_with_id_recs/' || id || '/edit' AS edit_link, + name + FROM dynamic_test.test_with_id_recs + ORDER BY id DESC + LIMIT 1 + SQL + + options_edit = <<~YAML + column_options: + show_as: + edit_link: embedded_block + YAML + + @report_edit = Report.create!( + current_admin: @admin, + name: "Embedded Block Edit Mode #{SecureRandom.hex(4)}", + description: 'Test embedded_block in edit mode (GitHub #325)', + sql: sql_edit, + options: options_edit, + disabled: false, + report_type: 'regular_report', + auto: false, + searchable: false + ) + + Admin::UserAccessControl.create!( + app_type: @user.app_type, + access: :read, + resource_type: :report, + resource_name: @report_edit.alt_resource_name, + current_admin: @admin + ) + end + + before(:each) do + login + # Make sure no modal is open from a previous test + visit '/reports' + finish_page_loading + end + + def navigate_to_report(report) + # Navigate fresh each time + visit '/reports' + finish_page_loading + + expect(page).to have_css(".data-results table.tablesorter tr[data-report-id='#{report.id}']") + + within ".data-results table.tablesorter tr[data-report-id='#{report.id}']" do + click_link report.name + end + + expect(page).to have_css('.report-criteria') + finish_page_loading + end + + def run_report + within '#report_query_form' do + click_button 'table' + end + + expect(page).to have_css('.search-status-done') + expect(page).to have_css('.report-results-block table') + finish_page_loading + end + + def click_embedded_block_link + # Clear any flash notices that might obscure the link + dismiss_all_alerts + + # Find and click the embedded_block link (glyphicon-tasks icon) + link = find('.report-embedded-block-link') + scroll_into_view(link) + sleep 0.5 + + # Ensure the link is clickable (not obscured) + expect(link).to be_visible + + link.click + finish_page_loading + end + + def wait_for_modal + # Allow time for AJAX and modal animation + # Sometimes the modal takes a while to appear, so we'll wait longer + modal_appeared = false + 5.times do |i| + sleep 2 + if page.has_css?('#primary-modal1.fade.in', visible: true) + modal_appeared = true + break + end + end + + unless modal_appeared + # Debug: save state to understand why modal didn't appear + puts_debug 'Modal did not appear - checking for errors' + puts_debug "Alert danger: #{find('.alert-danger').text}" if page.has_css?('.alert-danger') + puts_debug 'Modal exists but not .fade.in' if page.has_css?('#primary-modal1', visible: true) + save_html_snapshot('/tmp/modal_debug.html') + end + + expect(modal_appeared).to be(true), 'Modal did not appear after multiple retries' + finish_page_loading + end + + context 'with show mode URL (existing behavior)' do + it 'opens the modal in show mode when clicking embedded_block link' do + navigate_to_report(@report_show) + run_report + + # Verify the embedded_block link is present + expect(page).to have_css('.report-embedded-block-link') + + click_embedded_block_link + wait_for_modal + + # In show mode, we should see the record displayed (not a form) + within '#primary-modal1.fade.in' do + # Should contain the record name (case insensitive as Rails downcases user data) + expect(page).to have_content(/embedded block test record/i) + # Should NOT have an active edit form initially (no save button visible) + expect(page).not_to have_css('form.edit_dynamic_model_test_with_id_rec') + end + end + end + + context 'with edit mode URL ending in /edit (GitHub #325)' do + it 'opens the modal in edit mode when clicking embedded_block link with /edit URL' do + navigate_to_report(@report_edit) + run_report + + # Verify the embedded_block link is present + expect(page).to have_css('.report-embedded-block-link') + + click_embedded_block_link + wait_for_modal + + # In edit mode, we should see an editable form + within '#primary-modal1.fade.in' do + # Should have an edit form + expect(page).to have_css('form.edit_dynamic_model_test_with_id_rec', wait: 10) + # Form should be visible and editable + expect(page).to have_field('dynamic_model_test_with_id_rec[name]') + expect(page).to have_button('Save') + end + end + + it 'closes the modal when saving in edit mode' do + navigate_to_report(@report_edit) + run_report + + click_embedded_block_link + wait_for_modal + + within '#primary-modal1.fade.in' do + # Wait for form to load + expect(page).to have_css('form.edit_dynamic_model_test_with_id_rec', wait: 10) + + # Update a field + fill_in 'dynamic_model_test_with_id_rec[name]', with: 'Updated Record Name' + click_button 'Save' + end + + # Modal should close after save + expect(page).not_to have_css('#primary-modal1.fade.in', wait: 10) + + # Verify the update was saved (Rails downcases user data) + @test_record.reload + expect(@test_record.name).to eq('updated record name') + end + end +end From 1c11dadc318eed2119fb49630538b3619b32f341 Mon Sep 17 00:00:00 2001 From: Phil Date: Fri, 23 Jan 2026 10:59:36 +0000 Subject: [PATCH 2/4] Added activity log edit mode test for embedded_block - fixes #325 --- spec/system/report_embedded_block_spec.rb | 106 ++++++++++++++++++++-- 1 file changed, 100 insertions(+), 6 deletions(-) diff --git a/spec/system/report_embedded_block_spec.rb b/spec/system/report_embedded_block_spec.rb index 376017b44a..bd32ad56b3 100644 --- a/spec/system/report_embedded_block_spec.rb +++ b/spec/system/report_embedded_block_spec.rb @@ -6,10 +6,13 @@ # records in a modal dialog when clicked. This is a UI test that verifies the full interaction. # # Test Coverage: -# - Clicking embedded_block link with show URL opens modal in show mode (existing behavior) -# - Clicking embedded_block link with edit URL (ending in /edit) opens modal in edit mode (GitHub #325) -# - When edit mode modal is saved, the modal should close automatically -# - Works for both dynamic models and activity logs +# - Dynamic Models: +# - Show URL opens modal in show mode (uses JSON + client-side Handlebars templates) +# - Edit URL (ending in /edit) opens modal in edit mode (GitHub #325) +# - When edit mode modal is saved, the modal closes automatically +# - Activity Logs: +# - Edit URL (ending in /edit) opens modal in edit mode (GitHub #325) +# - Note: Show mode not tested for activity logs as Handlebars templates aren't loaded on report pages require 'rails_helper' @@ -39,6 +42,10 @@ setup_embedded_block_test_data setup_embedded_block_reports + + # Activity log setup + setup_activity_log_test_data + setup_activity_log_reports end def setup_embedded_block_test_user @@ -55,6 +62,9 @@ def setup_embedded_block_test_user ) setup_access :dynamic_model__test_with_id_recs, user: @user + setup_access :activity_log__player_contact_phones, user: @user + setup_access :activity_log__player_contact_phone__primary, resource_type: :activity_log_type, user: @user + setup_access :activity_log__player_contact_phone__blank_log, resource_type: :activity_log_type, user: @user expect(@user.can?(:view_reports)).to be_truthy end @@ -147,6 +157,69 @@ def setup_embedded_block_reports ) end + def setup_activity_log_test_data + # Create a player contact first (required for activity log) + @player_contact = PlayerContact.create!( + master: @master, + data: '(617)555-1234', + rec_type: 'phone', + rank: 10, + current_user: @user + ) + expect(@player_contact).to be_persisted + + # Create an activity log record + @activity_log = ActivityLog::PlayerContactPhone.create!( + master: @master, + player_contact: @player_contact, + select_call_direction: 'from player', + select_who: 'user', + extra_log_type: 'primary', + current_user: @user + ) + expect(@activity_log).to be_persisted + end + + def setup_activity_log_reports + # Report with embedded_block using edit URL for activity log (GitHub #325) + # Note: Show mode is not tested for activity logs because the Handlebars templates + # required for rendering JSON responses are not loaded on report pages. + sql_al_edit = <<~SQL + SELECT + '/masters/' || master_id || '/activity_log/player_contact_phones/' || id || '/edit' AS al_edit_link, + extra_log_type + FROM activity_log_player_contact_phones + ORDER BY id DESC + LIMIT 1 + SQL + + options_al_edit = <<~YAML + column_options: + show_as: + al_edit_link: embedded_block + YAML + + @report_al_edit = Report.create!( + current_admin: @admin, + name: "Activity Log Edit Mode #{SecureRandom.hex(4)}", + description: 'Test embedded_block with activity log in edit mode (GitHub #325)', + sql: sql_al_edit, + options: options_al_edit, + disabled: false, + report_type: 'regular_report', + auto: false, + searchable: false + ) + + Admin::UserAccessControl.create!( + app_type: @user.app_type, + access: :read, + resource_type: :report, + resource_name: @report_al_edit.alt_resource_name, + current_admin: @admin + ) + end + before(:each) do login # Make sure no modal is open from a previous test @@ -199,7 +272,7 @@ def wait_for_modal # Allow time for AJAX and modal animation # Sometimes the modal takes a while to appear, so we'll wait longer modal_appeared = false - 5.times do |i| + 5.times do |_i| sleep 2 if page.has_css?('#primary-modal1.fade.in', visible: true) modal_appeared = true @@ -211,7 +284,7 @@ def wait_for_modal # Debug: save state to understand why modal didn't appear puts_debug 'Modal did not appear - checking for errors' puts_debug "Alert danger: #{find('.alert-danger').text}" if page.has_css?('.alert-danger') - puts_debug 'Modal exists but not .fade.in' if page.has_css?('#primary-modal1', visible: true) + puts_debug 'Modal exists but not .fade.in' if page.has_css?('#primary-modal1', visible: :all) save_html_snapshot('/tmp/modal_debug.html') end @@ -285,4 +358,25 @@ def wait_for_modal expect(@test_record.name).to eq('updated record name') end end + + context 'with activity log edit URL ending in /edit (GitHub #325)' do + it 'opens the activity log modal in edit mode when clicking embedded_block link with /edit URL' do + navigate_to_report(@report_al_edit) + run_report + + # Verify the embedded_block link is present + expect(page).to have_css('.report-embedded-block-link') + + click_embedded_block_link + wait_for_modal + + # In edit mode, we should see an editable form + within '#primary-modal1.fade.in' do + # Should have an edit form for activity log + expect(page).to have_css('form.edit_activity_log_player_contact_phone', wait: 10) + # Form should have typical activity log fields + expect(page).to have_button('Save') + end + end + end end From 814948f7f929a65c2d6064b4ac20a941c6c8eeab Mon Sep 17 00:00:00 2001 From: Phil Date: Fri, 23 Jan 2026 12:24:03 +0000 Subject: [PATCH 3/4] Refactored embedded_block edit mode implementation and tests - Ruby: Extracted parse_embedded_block_url and build_embedded_block_html private methods - JavaScript: Added JSDoc comments and extracted setup_edit_mode function - Unit tests: Added shared examples for URL parsing tests - System tests: Added helper methods and shared examples for cleaner test organization --- .../app/_fpa_ajax_processors_reports.js | 86 +++-- .../reports_common_result_cell.rb | 82 +++-- .../reports_common_result_cell_spec.rb | 169 ++++----- spec/system/report_embedded_block_spec.rb | 333 +++++++----------- 4 files changed, 300 insertions(+), 370 deletions(-) diff --git a/app/assets/javascripts/app/_fpa_ajax_processors_reports.js b/app/assets/javascripts/app/_fpa_ajax_processors_reports.js index cc87505909..bc17c9ffac 100644 --- a/app/assets/javascripts/app/_fpa_ajax_processors_reports.js +++ b/app/assets/javascripts/app/_fpa_ajax_processors_reports.js @@ -36,52 +36,74 @@ _fpa.postprocessors_reports = { $('#modal_results_block').prop('id', 'modal_results_block_1') }, + /** + * Postprocessor for embedded dynamic block links in reports. + * Opens a modal dialog displaying a dynamic model or activity log record. + * + * Supports two modes: + * - Show mode: Displays the record in read-only view + * - Edit mode: Opens the edit form directly (GitHub #325) + * When data-edit-mode="true", clicks the edit button and closes modal on save + */ report_embed_dynamic_block: function (block, data) { - var us_name = block.attr('data-model-name') - var hyph_name = us_name.hyphenate() - var id = block.attr('data-id') - var master_id = block.attr('data-master-id') - var edit_mode = block.attr('data-edit-mode') === 'true' - var target_block = "report-result-embedded-block" - var html = $(`
`) - if ($(block).contents().length == 0) { + var modelName = block.attr('data-model-name'); + var hyphenatedName = modelName.hyphenate(); + var recordId = block.attr('data-id'); + var masterId = block.attr('data-master-id'); + var isEditMode = block.attr('data-edit-mode') === 'true'; + var targetBlockId = 'report-result-embedded-block'; + + // Build the modal content container + var modalContent = $(`
`); + + // If no content was loaded via AJAX, hide the modal and return + if ($(block).contents().length === 0) { _fpa.hide_modal(1); return; } - block.removeClass('sv-added-setup-links') - _fpa.show_modal(html, null, true, 'embedded-dynamic-block', 1) + + block.removeClass('sv-added-setup-links'); + _fpa.show_modal(modalContent, null, true, 'embedded-dynamic-block', 1); + + // Move content into modal and set up handlers window.setTimeout(function () { - $(block).contents().appendTo(`#${target_block}`) + $(block).contents().appendTo(`#${targetBlockId}`); $(block).html(''); - window.setTimeout(function () { - const $target_block = $(`#${target_block}`); - _fpa.form_utils.resize_labels($target_block, null, true) - // Ensure that the viewer is set up with the user's capabilities to view and download - var sv_opt = { allow_actions: null }; - sv_opt.allow_actions = _fpa.state.user_can; - - _fpa.secure_view.setup_links($target_block, 'a.redcap-file-use-secure-view', sv_opt); + window.setTimeout(function () { + const $targetBlock = $(`#${targetBlockId}`); + _fpa.form_utils.resize_labels($targetBlock, null, true); - // If edit mode, click the edit button to open the form directly - if (edit_mode) { - var edit_btn = $target_block.find('.edit-entity.glyphicon-pencil').first(); - if (edit_btn.length) { - edit_btn.click(); - } + // Set up secure view links with user capabilities + var secureViewOptions = { allow_actions: _fpa.state.user_can }; + _fpa.secure_view.setup_links($targetBlock, 'a.redcap-file-use-secure-view', secureViewOptions); - // Set up handler to close modal on successful save - $target_block.on('ajax:success', 'form', function (event) { - // Close the modal after a successful save - window.setTimeout(function () { - _fpa.hide_modal(1); - }, 300); - }); + // Handle edit mode: click edit button and set up save handler + if (isEditMode) { + _fpa.postprocessors_reports.setup_edit_mode($targetBlock); } }, 500); }, 500); }, + /** + * Sets up edit mode behavior for an embedded block. + * Clicks the edit button to open the form and closes modal on successful save. + */ + setup_edit_mode: function ($targetBlock) { + var editButton = $targetBlock.find('.edit-entity.glyphicon-pencil').first(); + if (editButton.length) { + editButton.click(); + } + + // Close modal after successful form save + $targetBlock.on('ajax:success', 'form', function () { + window.setTimeout(function () { + _fpa.hide_modal(1); + }, 300); + }); + }, + reports_form: function (block, data) { _fpa.report_criteria.reports_form(block, data); }, diff --git a/app/helpers/report_results/reports_common_result_cell.rb b/app/helpers/report_results/reports_common_result_cell.rb index ea911cce70..097d9a54c9 100644 --- a/app/helpers/report_results/reports_common_result_cell.rb +++ b/app/helpers/report_results/reports_common_result_cell.rb @@ -177,45 +177,81 @@ def cell_content_for_url html.html_safe end + # + # Generate HTML for opening a dynamic model or activity log record in a modal dialog. + # Supports both show and edit modes: + # - Show mode: URL like /masters/123/dynamic_model/table_name/456 + # - Edit mode: URL ending with /edit (GitHub #325) + # + # The content can be either a plain URL or a markdown format link [label](url) def cell_content_for_embedded_block return cell_content unless cell_content.present? - url = cell_content + parsed = parse_embedded_block_url(cell_content) + build_embedded_block_html(parsed) + end - if url.start_with? '[' - # This is a markdown format link - col_url_parts = url&.scan(/^\[(.+)\]\((.+)\)$/) - a_text = html_escape col_url_parts&.first&.first - url = html_escape col_url_parts&.first&.last + private + + # + # Parse the URL from embedded_block content. + # Returns a hash with parsed components including edit_mode flag. + def parse_embedded_block_url(content) + url = content + link_text = nil + icon_class = nil + + if content.start_with?('[') + # Markdown format link: [Label](/url/path) + url_parts = content.scan(/^\[(.+)\]\((.+)\)$/) + link_text = html_escape(url_parts&.first&.first) + url = html_escape(url_parts&.first&.last) else - # This is a plain URL - icon = 'glyphicon glyphicon-tasks' + # Plain URL - show icon + icon_class = 'glyphicon glyphicon-tasks' end - # Detect if this is an edit mode URL (ends with /edit) edit_mode = url.end_with?('/edit') + url_segments = url.split('/').reject(&:blank?) + + # Remove 'edit' suffix before extracting record id + url_segments.pop if edit_mode + + record_id = url_segments.last + master_id = url_segments[1] if url_segments.first == 'masters' + # Join the last two path segments to form model name (e.g., dynamic_model__table_name) + # rubocop:disable Style/SafeNavigationChainLength + model_name_hyphenated = url_segments[-3..-2]&.join('__')&.hyphenate&.singularize || '' + # rubocop:enable Style/SafeNavigationChainLength + + { + url:, + link_text:, + icon_class:, + edit_mode:, + record_id:, + master_id:, + model_name_hyphenated: + } + end - split_url = url.split('/') - split_url = split_url.reject(&:blank?) - - # If edit mode, remove 'edit' from the end before extracting id - split_url.pop if edit_mode - - id = split_url.last - master_id = split_url[1] if split_url.first == 'masters' - hyph_name = split_url[-3..-2]&.join('__')&.hyphenate&.singularize || '' - - # Add edit-mode attribute if this is an edit URL - edit_mode_attr = edit_mode ? ' data-edit-mode="true"' : '' + # + # Build the HTML for an embedded_block link and target div + def build_embedded_block_html(parsed) + hyph_name = parsed[:model_name_hyphenated] + record_id = parsed[:record_id] + edit_mode_attr = parsed[:edit_mode] ? ' data-edit-mode="true"' : '' html = <<~END_HTML - #{a_text} -
+ #{parsed[:link_text]} +
END_HTML html.html_safe end + public + def cell_content_for_embedded_report return cell_content unless cell_content.present? diff --git a/spec/helpers/report_results/reports_common_result_cell_spec.rb b/spec/helpers/report_results/reports_common_result_cell_spec.rb index e5f3fc05bf..82593dd466 100644 --- a/spec/helpers/report_results/reports_common_result_cell_spec.rb +++ b/spec/helpers/report_results/reports_common_result_cell_spec.rb @@ -11,10 +11,7 @@ # - Handles plain URLs like /dynamic_model/table_name/123 # - Handles markdown format links like [Label](/dynamic_model/table_name/123) # - Handles activity log URLs like /activity_log/log_type/456 -# - NEW: Handles edit mode URLs ending with /edit (GitHub #325) -# - /dynamic_model/table_name/123/edit should include data-edit-mode="true" -# - Modal should open directly in edit mode -# - Save should close the modal dialog +# - Handles edit mode URLs ending with /edit (GitHub #325) require 'rails_helper' @@ -30,140 +27,92 @@ def build_cell(content) end describe '#cell_content_for_embedded_block' do - context 'with dynamic model show URLs (existing behavior)' do - it 'returns the content unchanged if blank' do - cell = build_cell('') - expect(cell.cell_content_for_embedded_block).to eq('') + describe 'edge cases' do + it 'returns blank content unchanged' do + expect(build_cell('').cell_content_for_embedded_block).to eq('') end - it 'returns nil if content is nil' do - cell = build_cell(nil) - expect(cell.cell_content_for_embedded_block).to be_nil - end - - it 'parses plain URL and generates correct HTML with show mode attributes' do - url = '/dynamic_model/datadic_variables/123' - cell = build_cell(url) - html = cell.cell_content_for_embedded_block - - # Should contain the link - expect(html).to include('href="/dynamic_model/datadic_variables/123"') - expect(html).to include('data-remote="true"') - expect(html).to include('class="report-embedded-block-link glyphicon glyphicon-tasks"') - - # Should contain the hidden div for the modal content - expect(html).to include('id="report-result-embedded-block--123"') - expect(html).to include('data-preprocessor="report_embed_dynamic_block"') - expect(html).to include('data-model-name="dynamic_model__datadic_variable"') - expect(html).to include('data-id="123"') - - # Should NOT contain edit-mode attribute (show mode) - expect(html).not_to include('data-edit-mode') + it 'returns nil content as nil' do + expect(build_cell(nil).cell_content_for_embedded_block).to be_nil end + end - it 'parses markdown format link correctly' do - url = '[Edit Item](/dynamic_model/datadic_variables/456)' - cell = build_cell(url) - html = cell.cell_content_for_embedded_block + describe 'URL parsing' do + shared_examples 'parses URL correctly' do |url, expected| + it "extracts correct attributes from #{url}" do + html = build_cell(url).cell_content_for_embedded_block - # Should contain the link text - expect(html).to include('>Edit Item') - expect(html).to include('href="/dynamic_model/datadic_variables/456"') + expect(html).to include("data-id=\"#{expected[:id]}\"") + expect(html).not_to include('data-id="edit"') + expect(html).to include("data-model-name=\"#{expected[:model_name]}\"") - # Should have correct id - expect(html).to include('data-id="456"') - expect(html).to include('id="report-result-embedded-block--456"') + expect(html).to include("data-master-id=\"#{expected[:master_id]}\"") if expected[:master_id] - # Should NOT contain edit-mode attribute - expect(html).not_to include('data-edit-mode') + if expected[:edit_mode] + expect(html).to include('data-edit-mode="true"') + else + expect(html).not_to include('data-edit-mode') + end + end end - it 'extracts master_id from URL when present' do - url = '/masters/789/dynamic_model/test_items/123' - cell = build_cell(url) - html = cell.cell_content_for_embedded_block + context 'with dynamic model URLs' do + include_examples 'parses URL correctly', + '/dynamic_model/datadic_variables/123', + { id: '123', model_name: 'dynamic_model__datadic_variable', edit_mode: false } - expect(html).to include('data-master-id="789"') - expect(html).to include('data-id="123"') - end + include_examples 'parses URL correctly', + '/masters/789/dynamic_model/test_items/456', + { id: '456', model_name: 'dynamic_model__test_item', master_id: '789', edit_mode: false } - it 'generates correct hyphenated model name for dynamic models' do - url = '/dynamic_model/datadic_variables/123' - cell = build_cell(url) - html = cell.cell_content_for_embedded_block + include_examples 'parses URL correctly', + '/dynamic_model/datadic_variables/123/edit', + { id: '123', model_name: 'dynamic_model__datadic_variable', edit_mode: true } - # dynamic_model + datadic_variables -> dynamic_model__datadic_variable (singularized, hyphenated) - expect(html).to include('data-model-name="dynamic_model__datadic_variable"') - expect(html).to include('data-dynamic-model--datadic-variable-id="123"') + include_examples 'parses URL correctly', + '/masters/999/dynamic_model/test_items/123/edit', + { id: '123', model_name: 'dynamic_model__test_item', master_id: '999', edit_mode: true } end - end - - context 'with activity log show URLs (existing behavior)' do - it 'parses activity log URL and generates correct HTML' do - url = '/activity_log/test_processes/456' - cell = build_cell(url) - html = cell.cell_content_for_embedded_block - expect(html).to include('href="/activity_log/test_processes/456"') - expect(html).to include('data-id="456"') - expect(html).to include('data-model-name="activity_log__test_process"') + context 'with activity log URLs' do + include_examples 'parses URL correctly', + '/activity_log/test_processes/456', + { id: '456', model_name: 'activity_log__test_process', edit_mode: false } - # Should NOT contain edit-mode attribute - expect(html).not_to include('data-edit-mode') + include_examples 'parses URL correctly', + '/activity_log/test_processes/456/edit', + { id: '456', model_name: 'activity_log__test_process', edit_mode: true } end end - context 'with edit mode URLs ending in /edit (GitHub #325)' do - it 'parses dynamic model edit URL and includes edit-mode attribute' do - url = '/dynamic_model/datadic_variables/123/edit' - cell = build_cell(url) - html = cell.cell_content_for_embedded_block - - # Should extract the correct id (not "edit") - expect(html).to include('data-id="123"') - expect(html).not_to include('data-id="edit"') - - # Should include edit-mode attribute - expect(html).to include('data-edit-mode="true"') - - # Should still have correct model name - expect(html).to include('data-model-name="dynamic_model__datadic_variable"') + describe 'HTML generation' do + it 'generates link with correct attributes for plain URL' do + html = build_cell('/dynamic_model/datadic_variables/123').cell_content_for_embedded_block - # Should have the full URL including /edit - expect(html).to include('href="/dynamic_model/datadic_variables/123/edit"') + expect(html).to include('href="/dynamic_model/datadic_variables/123"') + expect(html).to include('data-remote="true"') + expect(html).to include('class="report-embedded-block-link glyphicon glyphicon-tasks"') + expect(html).to include('data-preprocessor="report_embed_dynamic_block"') end - it 'parses markdown format edit URL correctly' do - url = '[Edit Now](/dynamic_model/datadic_variables/789/edit)' - cell = build_cell(url) - html = cell.cell_content_for_embedded_block + it 'generates link with label for markdown format' do + html = build_cell('[Edit Item](/dynamic_model/datadic_variables/456)').cell_content_for_embedded_block - expect(html).to include('>Edit Now') - expect(html).to include('data-id="789"') - expect(html).to include('data-edit-mode="true"') - expect(html).not_to include('data-id="edit"') + expect(html).to include('>Edit Item') + expect(html).to include('href="/dynamic_model/datadic_variables/456"') end - it 'parses activity log edit URL and includes edit-mode attribute' do - url = '/activity_log/test_processes/456/edit' - cell = build_cell(url) - html = cell.cell_content_for_embedded_block + it 'generates target div with correct id' do + html = build_cell('/dynamic_model/test/789').cell_content_for_embedded_block - expect(html).to include('data-id="456"') - expect(html).not_to include('data-id="edit"') - expect(html).to include('data-edit-mode="true"') - expect(html).to include('data-model-name="activity_log__test_process"') + expect(html).to include('id="report-result-embedded-block--789"') + expect(html).to include('class="report-temp-embedded-block"') end - it 'handles master_id correctly with edit URLs' do - url = '/masters/999/dynamic_model/test_items/123/edit' - cell = build_cell(url) - html = cell.cell_content_for_embedded_block + it 'preserves full URL including /edit for edit mode' do + html = build_cell('/dynamic_model/test/123/edit').cell_content_for_embedded_block - expect(html).to include('data-master-id="999"') - expect(html).to include('data-id="123"') - expect(html).to include('data-edit-mode="true"') - expect(html).not_to include('data-id="edit"') + expect(html).to include('href="/dynamic_model/test/123/edit"') end end end diff --git a/spec/system/report_embedded_block_spec.rb b/spec/system/report_embedded_block_spec.rb index bd32ad56b3..68b95ff899 100644 --- a/spec/system/report_embedded_block_spec.rb +++ b/spec/system/report_embedded_block_spec.rb @@ -31,27 +31,24 @@ seed_database create_data_set_outside_tx - # Must create user BEFORE setup_fields_dm since it requires @user - setup_embedded_block_test_user - - setup_fields_dm # Sets up dynamic_model__test_with_id_recs - - # Reload routes after setting up dynamic model - DynamicModel.routes_reload - Rails.application.routes_reloader.reload! - - setup_embedded_block_test_data - setup_embedded_block_reports - - # Activity log setup - setup_activity_log_test_data - setup_activity_log_reports + setup_test_user + setup_dynamic_model + setup_test_data + setup_reports end - def setup_embedded_block_test_user + # + # Setup helpers + # + def setup_test_user @user, @good_password = create_user @good_email = @user.email + grant_report_access + grant_model_access + end + + def grant_report_access Admin::UserAccessControl.create!( app_type_id: @user.app_type_id, access: :read, @@ -60,105 +57,31 @@ def setup_embedded_block_test_user current_admin: @admin, user: @user ) + end + def grant_model_access setup_access :dynamic_model__test_with_id_recs, user: @user setup_access :activity_log__player_contact_phones, user: @user setup_access :activity_log__player_contact_phone__primary, resource_type: :activity_log_type, user: @user setup_access :activity_log__player_contact_phone__blank_log, resource_type: :activity_log_type, user: @user + end - expect(@user.can?(:view_reports)).to be_truthy + def setup_dynamic_model + setup_fields_dm + DynamicModel.routes_reload + Rails.application.routes_reloader.reload! end - def setup_embedded_block_test_data - # Create a test record we can display in embedded_block + def setup_test_data @master = Master.create!(current_user: @user) + @test_record = DynamicModel::TestWithIdRec.create!( master: @master, name: 'Embedded Block Test Record', value: 'Test Value 123', current_user: @user ) - expect(@test_record).to be_persisted - end - - def setup_embedded_block_reports - # Report with embedded_block using show URL (existing behavior) - # Dynamic model routes require master_id: /masters/{master_id}/dynamic_model/table_name/{id} - # Use subquery to get the latest record dynamically - sql_show = <<~SQL - SELECT - '/masters/' || master_id || '/dynamic_model/test_with_id_recs/' || id AS show_link, - name - FROM dynamic_test.test_with_id_recs - ORDER BY id DESC - LIMIT 1 - SQL - - options_show = <<~YAML - column_options: - show_as: - show_link: embedded_block - YAML - @report_show = Report.create!( - current_admin: @admin, - name: "Embedded Block Show Mode #{SecureRandom.hex(4)}", - description: 'Test embedded_block in show mode', - sql: sql_show, - options: options_show, - disabled: false, - report_type: 'regular_report', - auto: false, - searchable: false - ) - - Admin::UserAccessControl.create!( - app_type: @user.app_type, - access: :read, - resource_type: :report, - resource_name: @report_show.alt_resource_name, - current_admin: @admin - ) - - # Report with embedded_block using edit URL (new GitHub #325 feature) - sql_edit = <<~SQL - SELECT - '/masters/' || master_id || '/dynamic_model/test_with_id_recs/' || id || '/edit' AS edit_link, - name - FROM dynamic_test.test_with_id_recs - ORDER BY id DESC - LIMIT 1 - SQL - - options_edit = <<~YAML - column_options: - show_as: - edit_link: embedded_block - YAML - - @report_edit = Report.create!( - current_admin: @admin, - name: "Embedded Block Edit Mode #{SecureRandom.hex(4)}", - description: 'Test embedded_block in edit mode (GitHub #325)', - sql: sql_edit, - options: options_edit, - disabled: false, - report_type: 'regular_report', - auto: false, - searchable: false - ) - - Admin::UserAccessControl.create!( - app_type: @user.app_type, - access: :read, - resource_type: :report, - resource_name: @report_edit.alt_resource_name, - current_admin: @admin - ) - end - - def setup_activity_log_test_data - # Create a player contact first (required for activity log) @player_contact = PlayerContact.create!( master: @master, data: '(617)555-1234', @@ -166,9 +89,7 @@ def setup_activity_log_test_data rank: 10, current_user: @user ) - expect(@player_contact).to be_persisted - # Create an activity log record @activity_log = ActivityLog::PlayerContactPhone.create!( master: @master, player_contact: @player_contact, @@ -177,34 +98,35 @@ def setup_activity_log_test_data extra_log_type: 'primary', current_user: @user ) - expect(@activity_log).to be_persisted end - def setup_activity_log_reports - # Report with embedded_block using edit URL for activity log (GitHub #325) - # Note: Show mode is not tested for activity logs because the Handlebars templates - # required for rendering JSON responses are not loaded on report pages. - sql_al_edit = <<~SQL - SELECT - '/masters/' || master_id || '/activity_log/player_contact_phones/' || id || '/edit' AS al_edit_link, - extra_log_type - FROM activity_log_player_contact_phones - ORDER BY id DESC - LIMIT 1 - SQL + def setup_reports + @report_dm_show = create_embedded_block_report( + name: 'DM Show Mode', + sql: dynamic_model_show_sql, + link_column: 'show_link' + ) - options_al_edit = <<~YAML - column_options: - show_as: - al_edit_link: embedded_block - YAML + @report_dm_edit = create_embedded_block_report( + name: 'DM Edit Mode', + sql: dynamic_model_edit_sql, + link_column: 'edit_link' + ) - @report_al_edit = Report.create!( + @report_al_edit = create_embedded_block_report( + name: 'AL Edit Mode', + sql: activity_log_edit_sql, + link_column: 'al_edit_link' + ) + end + + def create_embedded_block_report(name:, sql:, link_column:) + report = Report.create!( current_admin: @admin, - name: "Activity Log Edit Mode #{SecureRandom.hex(4)}", - description: 'Test embedded_block with activity log in edit mode (GitHub #325)', - sql: sql_al_edit, - options: options_al_edit, + name: "#{name} #{SecureRandom.hex(4)}", + description: "Test #{name}", + sql: sql, + options: "column_options:\n show_as:\n #{link_column}: embedded_block", disabled: false, report_type: 'regular_report', auto: false, @@ -215,20 +137,47 @@ def setup_activity_log_reports app_type: @user.app_type, access: :read, resource_type: :report, - resource_name: @report_al_edit.alt_resource_name, + resource_name: report.alt_resource_name, current_admin: @admin ) + + report end + # + # SQL generators for reports + # + def dynamic_model_show_sql + <<~SQL + SELECT '/masters/' || master_id || '/dynamic_model/test_with_id_recs/' || id AS show_link, name + FROM dynamic_test.test_with_id_recs ORDER BY id DESC LIMIT 1 + SQL + end + + def dynamic_model_edit_sql + <<~SQL + SELECT '/masters/' || master_id || '/dynamic_model/test_with_id_recs/' || id || '/edit' AS edit_link, name + FROM dynamic_test.test_with_id_recs ORDER BY id DESC LIMIT 1 + SQL + end + + def activity_log_edit_sql + <<~SQL + SELECT '/masters/' || master_id || '/activity_log/player_contact_phones/' || id || '/edit' AS al_edit_link, extra_log_type + FROM activity_log_player_contact_phones ORDER BY id DESC LIMIT 1 + SQL + end + + # + # Navigation and interaction helpers + # before(:each) do login - # Make sure no modal is open from a previous test visit '/reports' finish_page_loading end def navigate_to_report(report) - # Navigate fresh each time visit '/reports' finish_page_loading @@ -242,37 +191,23 @@ def navigate_to_report(report) finish_page_loading end - def run_report - within '#report_query_form' do - click_button 'table' - end - + def run_report_and_click_embedded_link + within('#report_query_form') { click_button 'table' } expect(page).to have_css('.search-status-done') expect(page).to have_css('.report-results-block table') finish_page_loading - end - def click_embedded_block_link - # Clear any flash notices that might obscure the link dismiss_all_alerts - - # Find and click the embedded_block link (glyphicon-tasks icon) link = find('.report-embedded-block-link') scroll_into_view(link) sleep 0.5 - - # Ensure the link is clickable (not obscured) - expect(link).to be_visible - link.click finish_page_loading end - def wait_for_modal - # Allow time for AJAX and modal animation - # Sometimes the modal takes a while to appear, so we'll wait longer + def wait_for_modal_to_appear modal_appeared = false - 5.times do |_i| + 5.times do sleep 2 if page.has_css?('#primary-modal1.fade.in', visible: true) modal_appeared = true @@ -281,10 +216,8 @@ def wait_for_modal end unless modal_appeared - # Debug: save state to understand why modal didn't appear puts_debug 'Modal did not appear - checking for errors' - puts_debug "Alert danger: #{find('.alert-danger').text}" if page.has_css?('.alert-danger') - puts_debug 'Modal exists but not .fade.in' if page.has_css?('#primary-modal1', visible: :all) + puts_debug "Alert: #{find('.alert-danger').text}" if page.has_css?('.alert-danger') save_html_snapshot('/tmp/modal_debug.html') end @@ -292,91 +225,81 @@ def wait_for_modal finish_page_loading end - context 'with show mode URL (existing behavior)' do - it 'opens the modal in show mode when clicking embedded_block link' do - navigate_to_report(@report_show) - run_report - - # Verify the embedded_block link is present - expect(page).to have_css('.report-embedded-block-link') + # + # Shared examples for common test patterns + # + shared_examples 'opens modal with embedded content' do + it 'displays the modal when clicking embedded_block link' do + navigate_to_report(report) + run_report_and_click_embedded_link + wait_for_modal_to_appear + expect(page).to have_css('#primary-modal1.fade.in') + end + end - click_embedded_block_link - wait_for_modal + shared_examples 'opens in edit mode' do + it 'shows the edit form directly' do + navigate_to_report(report) + run_report_and_click_embedded_link + wait_for_modal_to_appear - # In show mode, we should see the record displayed (not a form) within '#primary-modal1.fade.in' do - # Should contain the record name (case insensitive as Rails downcases user data) - expect(page).to have_content(/embedded block test record/i) - # Should NOT have an active edit form initially (no save button visible) - expect(page).not_to have_css('form.edit_dynamic_model_test_with_id_rec') + expect(page).to have_css(edit_form_selector, wait: 10) + expect(page).to have_button('Save') end end end - context 'with edit mode URL ending in /edit (GitHub #325)' do - it 'opens the modal in edit mode when clicking embedded_block link with /edit URL' do - navigate_to_report(@report_edit) - run_report + # + # Test contexts + # + context 'with dynamic model show URL (existing behavior)' do + let(:report) { @report_dm_show } - # Verify the embedded_block link is present - expect(page).to have_css('.report-embedded-block-link') + include_examples 'opens modal with embedded content' - click_embedded_block_link - wait_for_modal + it 'displays record in read-only mode without edit form' do + navigate_to_report(report) + run_report_and_click_embedded_link + wait_for_modal_to_appear - # In edit mode, we should see an editable form within '#primary-modal1.fade.in' do - # Should have an edit form - expect(page).to have_css('form.edit_dynamic_model_test_with_id_rec', wait: 10) - # Form should be visible and editable - expect(page).to have_field('dynamic_model_test_with_id_rec[name]') - expect(page).to have_button('Save') + expect(page).to have_content(/embedded block test record/i) + expect(page).not_to have_css('form.edit_dynamic_model_test_with_id_rec') end end + end - it 'closes the modal when saving in edit mode' do - navigate_to_report(@report_edit) - run_report + context 'with dynamic model edit URL (GitHub #325)' do + let(:report) { @report_dm_edit } + let(:edit_form_selector) { 'form.edit_dynamic_model_test_with_id_rec' } - click_embedded_block_link - wait_for_modal + include_examples 'opens modal with embedded content' + include_examples 'opens in edit mode' - within '#primary-modal1.fade.in' do - # Wait for form to load - expect(page).to have_css('form.edit_dynamic_model_test_with_id_rec', wait: 10) + it 'allows editing and closes modal on save' do + navigate_to_report(report) + run_report_and_click_embedded_link + wait_for_modal_to_appear - # Update a field + within '#primary-modal1.fade.in' do + expect(page).to have_css(edit_form_selector, wait: 10) fill_in 'dynamic_model_test_with_id_rec[name]', with: 'Updated Record Name' click_button 'Save' end - # Modal should close after save expect(page).not_to have_css('#primary-modal1.fade.in', wait: 10) - # Verify the update was saved (Rails downcases user data) @test_record.reload expect(@test_record.name).to eq('updated record name') end end - context 'with activity log edit URL ending in /edit (GitHub #325)' do - it 'opens the activity log modal in edit mode when clicking embedded_block link with /edit URL' do - navigate_to_report(@report_al_edit) - run_report - - # Verify the embedded_block link is present - expect(page).to have_css('.report-embedded-block-link') + context 'with activity log edit URL (GitHub #325)' do + let(:report) { @report_al_edit } + let(:edit_form_selector) { 'form.edit_activity_log_player_contact_phone' } - click_embedded_block_link - wait_for_modal - - # In edit mode, we should see an editable form - within '#primary-modal1.fade.in' do - # Should have an edit form for activity log - expect(page).to have_css('form.edit_activity_log_player_contact_phone', wait: 10) - # Form should have typical activity log fields - expect(page).to have_button('Save') - end - end + include_examples 'opens modal with embedded content' + include_examples 'opens in edit mode' end end From ba483d9826f2c869b2ddd2e50b73bd5298087a56 Mon Sep 17 00:00:00 2001 From: Phil Date: Fri, 23 Jan 2026 12:31:11 +0000 Subject: [PATCH 4/4] Added activity log save-and-close test for complete coverage --- spec/system/report_embedded_block_spec.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/spec/system/report_embedded_block_spec.rb b/spec/system/report_embedded_block_spec.rb index 68b95ff899..48b63a56d3 100644 --- a/spec/system/report_embedded_block_spec.rb +++ b/spec/system/report_embedded_block_spec.rb @@ -301,5 +301,19 @@ def wait_for_modal_to_appear include_examples 'opens modal with embedded content' include_examples 'opens in edit mode' + + it 'allows editing and closes modal on save' do + navigate_to_report(report) + run_report_and_click_embedded_link + wait_for_modal_to_appear + + within '#primary-modal1.fade.in' do + expect(page).to have_css(edit_form_selector, wait: 10) + # Form is already valid from setup, just save it + click_button 'Save' + end + + expect(page).not_to have_css('#primary-modal1.fade.in', wait: 10) + end end end