diff --git a/app/helpers/sort_helper.rb b/app/helpers/sort_helper.rb index bc7fe0078aba..5ceb668827ab 100644 --- a/app/helpers/sort_helper.rb +++ b/app/helpers/sort_helper.rb @@ -401,7 +401,7 @@ def action_menu(column, table_columns, caption, default_order, filter_column_map sort_actions(menu, column, default_order, content_args:, allowed_params:, **html_options) # Some columns do not offer a filter. Only show the option when filtering is possible. - filter_action(menu, filter, content_args:) if filter + filter_action(menu, column, filter, content_args:) if filter move_column_actions(menu, column, table_columns, content_args:, allowed_params:, **html_options) add_and_remove_column_actions(menu, column, table_columns, content_args:, allowed_params:, **html_options) @@ -421,22 +421,31 @@ def sort_actions(menu, column, default_order, content_args:, allowed_params: nil asc_sort_link = projects_path(sort_by_options(column, "asc", default_order, allowed_params:, **html_options)) menu.with_item(**menu_options(label: t(:label_sort_descending), - content_args:, + content_args: content_args.merge( + data: { + "test-selector" => "#{column}-sort-desc" + } + ), href: desc_sort_link)) do |item| item.with_leading_visual_icon(icon: :"sort-desc") end menu.with_item(**menu_options(label: t(:label_sort_ascending), - content_args:, + content_args: content_args.merge( + data: { + "test-selector" => "#{column}-sort-asc" + } + ), href: asc_sort_link)) do |item| item.with_leading_visual_icon(icon: :"sort-asc") end end - def filter_action(menu, filter, content_args:) + def filter_action(menu, column, filter, content_args:) menu.with_divider menu.with_item(**menu_options(label: t(:label_filter_by), content_args: content_args.merge( data: { + "test-selector" => "#{column}-filter-by", action: "table-action-menu#filterBy", filter_name: filter } @@ -446,22 +455,41 @@ def filter_action(menu, filter, content_args:) end def move_column_actions(menu, column, selected_columns, content_args:, allowed_params: nil, **html_options) - left_shift = shift_element(selected_columns, column) - shift_left_link = build_columns_link(left_shift, allowed_params:, **html_options) - - right_shift = shift_element(selected_columns, column, :right) - shift_right_link = build_columns_link(right_shift, allowed_params:, **html_options) + column_pos = selected_columns.index(column) + return unless column_pos menu.with_divider - menu.with_item(**menu_options(label: t(:label_move_column_left), - content_args:, - href: shift_left_link)) do |item| - item.with_leading_visual_icon(icon: :"op-columns-left") + + # Only offer the option of moving the column left if it is not already the leftmost column. + if column_pos > 0 + left_shift = shift_element(selected_columns, column) + shift_left_link = build_columns_link(left_shift, allowed_params:, **html_options) + + menu.with_item(**menu_options(label: t(:label_move_column_left), + content_args: content_args.merge( + data: { + "test-selector" => "#{column}-move-col-left" + } + ), + href: shift_left_link)) do |item| + item.with_leading_visual_icon(icon: :"op-columns-left") + end end - menu.with_item(**menu_options(label: t(:label_move_column_right), - content_args:, - href: shift_right_link)) do |item| - item.with_leading_visual_icon(icon: :"op-columns-right") + + # Only offer the option of moving the column right if it is not already the rightmost column. + if column_pos < selected_columns.length - 1 + right_shift = shift_element(selected_columns, column, :right) + shift_right_link = build_columns_link(right_shift, allowed_params:, **html_options) + + menu.with_item(**menu_options(label: t(:label_move_column_right), + content_args: content_args.merge( + data: { + "test-selector" => "#{column}-move-col-right" + } + ), + href: shift_right_link)) do |item| + item.with_leading_visual_icon(icon: :"op-columns-right") + end end end @@ -473,14 +501,21 @@ def add_and_remove_column_actions(menu, column, selected_columns, content_args:, menu.with_item(**menu_options(label: t(:label_add_column), content_args: content_args.merge( - data: { controller: "async-dialog" } + data: { + controller: "async-dialog", + "test-selector" => "#{column}-add-column" + } ), href: config_view_modal_link)) do |item| item.with_leading_visual_icon(icon: :columns) end menu.with_divider menu.with_item(**menu_options(label: t(:label_remove_column), - content_args:, + content_args: content_args.merge( + data: { + "test-selector" => "#{column}-remove-column" + } + ), scheme: :danger, href: rm_column_link)) do |item| item.with_leading_visual_icon(icon: :trash) diff --git a/spec/helpers/sort_helper_spec.rb b/spec/helpers/sort_helper_spec.rb index 7d692a604028..5dc96b8c22a3 100644 --- a/spec/helpers/sort_helper_spec.rb +++ b/spec/helpers/sort_helper_spec.rb @@ -270,4 +270,138 @@ def session; @session ||= {}; end end end end + + describe "#sort_header_with_action_menu" do + subject(:output) do + helper.sort_header_with_action_menu("id", + %w[name id description], {}, **options) + end + + let(:options) { { param: :json } } + let(:sort_criteria) { SortHelper::SortCriteria.new } + + let(:action_menu) do + # The resulting HTML is too big to assert in detail. We will only check some key parts to ensure it is + # an action menu with the expected content. + Nokogiri::HTML(output).at_css("th .generic-table--sort-header action-menu") + end + + before do + # helper relies on this instance var + @sort_criteria = sort_criteria + + # fake having called '/projects' + allow(helper) + .to receive(:url_options) + .and_return(url_options.merge(controller: "projects", action: "index")) + end + + it "renders an action-menu button as column header" do + expect(action_menu.at_css("button#menu-id-button .Button-content .Button-label").text).to eq("Id") + end + + it "shows sorting actions in the action-menu" do + sort_desc = action_menu.at_css("action-list .ActionListItem a[data-test-selector='id-sort-desc']") + expect(sort_desc.at_css(".ActionListItem-label").text.strip).to eq("Sort descending") + expect(sort_desc["href"]).to eq("/projects?sortBy=%5B%5B%22id%22%2C%22desc%22%5D%5D") + + sort_asc = action_menu.at_css("action-list .ActionListItem a[data-test-selector='id-sort-asc']") + expect(sort_asc.at_css(".ActionListItem-label").text.strip).to eq("Sort ascending") + expect(sort_asc["href"]).to eq("/projects?sortBy=%5B%5B%22id%22%2C%22asc%22%5D%5D") + end + + it "shows an action to move columns left and right" do + move_left = action_menu.at_css("action-list .ActionListItem a[data-test-selector='id-move-col-left']") + expect(move_left.at_css(".ActionListItem-label").text.strip).to eq("Move column left") + # The id column moved one place to the left and is now the first column instead of the second. + expect(move_left["href"]).to eq("/projects?columns=id+name+description") + + move_right = action_menu.at_css("action-list .ActionListItem a[data-test-selector='id-move-col-right']") + expect(move_right.at_css(".ActionListItem-label").text.strip).to eq("Move column right") + # The id column moved one place to the right and is now the last one. + expect(move_right["href"]).to eq("/projects?columns=name+description+id") + end + + context "with the current column being the leftmost one" do + subject(:output) do + helper.sort_header_with_action_menu("id", + %w[id name description], {}, **options) + end + + it "does not offer a 'move left' option" do + move_left = action_menu.at_css("action-list .ActionListItem a[data-test-selector='id-move-col-left']") + expect(move_left).to be_nil + + # But it offers a 'move right' option + move_right = action_menu.at_css("action-list .ActionListItem a[data-test-selector='id-move-col-right']") + expect(move_right).not_to be_nil + end + end + + context "with the current column being the rightmost one" do + subject(:output) do + helper.sort_header_with_action_menu("id", + %w[name description id], {}, **options) + end + + it "does not offer a 'move right' option" do + move_right = action_menu.at_css("action-list .ActionListItem a[data-test-selector='id-move-col-right']") + expect(move_right).to be_nil + + # But it offers a 'move left' option + move_left = action_menu.at_css("action-list .ActionListItem a[data-test-selector='id-move-col-left']") + expect(move_left).not_to be_nil + end + end + + it "shows an action to add columns" do + add_col = action_menu.at_css("action-list .ActionListItem a[data-test-selector='id-add-column']") + expect(add_col.at_css(".ActionListItem-label").text.strip).to eq("Add column") + # Check that the 'ConfigureViewModal' is opened on link click. This is where adding columns happens. + expect(add_col["href"]).to eq("/project_queries/configure_view_modal") + end + + it "shows an action to remove a column" do + remove_col = action_menu.at_css("action-list .ActionListItem a[data-test-selector='id-remove-column']") + expect(remove_col.at_css(".ActionListItem-label").text.strip).to eq("Remove column") + # The current column is removed from the columns-query: + expect(remove_col["href"]).to eq("/projects?columns=name+description") + end + + it "shows a 'filter by' action" do + filter_by = action_menu.at_css("action-list .ActionListItem button[data-test-selector='id-filter-by']") + expect(filter_by.at_css(".ActionListItem-label").text.strip).to eq("Filter by") + # Check that the correct Stimulus controller with the correct data is referenced: + expect(filter_by["data-action"]).to eq("table-action-menu#filterBy") + expect(filter_by["data-filter-name"]).to eq("id") + end + + context "with a filter mapping for the column" do + subject(:output) do + helper.sort_header_with_action_menu("id", + %w[name id description], { "id" => "id_code" }, **options) + end + + it "shows a 'filter by' action with the mapped filter" do + filter_by = action_menu.at_css("action-list .ActionListItem button[data-test-selector='id-filter-by']") + expect(filter_by.at_css(".ActionListItem-label").text.strip).to eq("Filter by") + expect(filter_by["data-action"]).to eq("table-action-menu#filterBy") + # With a column mapping, the filter name is changed accordingly: + expect(filter_by["data-filter-name"]).to eq("id_code") + end + end + + context "with the filter mapping specifying there is no filter for the column" do + subject(:output) do + # With the filter name mapped to nil, we expect no filter action to be present. + helper.sort_header_with_action_menu("id", + %w[name id description], { "id" => nil }, **options) + end + + it "does not show a 'filter by' action" do + filter_by = action_menu.at_css("action-list .ActionListItem button[data-test-selector='id-filter-by']") + expect(filter_by).to be_nil + end + end + end end