From aed494e395c923dd1e3483f718f87f340e7d6158 Mon Sep 17 00:00:00 2001
From: Chris Huynh <chris.huynh333@gmail.com>
Date: Thu, 30 Jan 2025 11:51:49 -0600
Subject: [PATCH] avoid rendering pagination if only 1 page, add associated
 test

---
 .../nextflow/samplesheet_component.html.erb   | 90 ++++++++++---------
 .../nextflow/samplesheet_component.rb         |  3 +-
 .../nextflow/samplesheet_controller.js        |  6 +-
 .../nextflow_samplesheet_component_test.rb    |  7 +-
 .../nextflow_samplesheet_component_preview.rb | 15 +++-
 .../default.html.erb                          | 16 +---
 .../workflow_executions/submissions_test.rb   | 54 ++++++++++-
 7 files changed, 118 insertions(+), 73 deletions(-)

diff --git a/app/components/nextflow/samplesheet_component.html.erb b/app/components/nextflow/samplesheet_component.html.erb
index f43ef8fc79..2bf0d9886d 100644
--- a/app/components/nextflow/samplesheet_component.html.erb
+++ b/app/components/nextflow/samplesheet_component.html.erb
@@ -1,6 +1,6 @@
 <div class="sample-sheet">
   <div class="hidden" data-nextflow--samplesheet-target="workflowAttributes">
-    <%= samples_workflow_executions_attributes %>
+    <%= samples_workflow_executions_attributes.to_json %>
   </div>
   <div class="hidden" data-nextflow--samplesheet-target="samplesheetProperties">
     <%= @properties.to_json %>
@@ -59,49 +59,51 @@
       </div>
     </template>
   </div>
-  <div class="flex items-center justify-end">
-    <nav aria-label="<%= t("viral.pagy.pagination_component.aria-label") %>">
-      <ul class="inline-flex h-10 text-base">
-        <li><button
-            type="button"
-            class="
-              flex items-center justify-center px-4 h-10 ms-0 leading-tight border
-              border-slate-300 dark:border-slate-700 cursor-default text-slate-600 bg-slate-50
-              dark:bg-slate-700 dark:text-slate-400 border-e-0 rounded-s-lg
-            "
-            data-action="click->nextflow--samplesheet#previousPage"
-            data-nextflow--samplesheet-target="previousBtn"
-            disabled="disabled"
-          ><%= t(".previous") %></button>
-        </li>
-        <li><select
-            class="
-              flex items-center justify-center px-4 h-10 ms-0 leading-tight border
-              border-slate-300 dark:border-slate-700 text-slate-500 bg-white
-              hover:bg-slate-100 hover:text-slate-700 dark:bg-slate-800 dark:text-slate-400
-              dark:hover:bg-slate-700 dark:hover:text-white cursor-pointer border-transparent
-              focus:border-transparent focus:ring-0
-            "
-            data-nextflow--samplesheet-target="pageNum"
-            data-action="change->nextflow--samplesheet#pageSelected"
-            aria-label="<%= t(".page_selection.aria_label") %>"
-          ><option value="1" selected="selected">1</option></select>
-        </li>
-        <li><button
-            class="
-              flex items-center justify-center px-4 h-10 ms-0 leading-tight border
-              border-slate-300 dark:border-slate-700 text-slate-500 bg-white
-              hover:bg-slate-100 hover:text-slate-700 dark:bg-slate-800 dark:text-slate-400
-              dark:hover:bg-slate-700 dark:hover:text-white rounded-e-lg
-            "
-            type="button"
-            data-nextflow--samplesheet-target="nextBtn"
-            data-action="click->nextflow--samplesheet#nextPage"
-          ><%= t(".next") %></button>
-        </li>
-      </ul>
-    </nav>
-  </div>
+  <% if samples_workflow_executions_attributes.count > 5 %>
+    <div id="samplesheet_pagination" class="flex items-center justify-end">
+      <nav aria-label="<%= t("viral.pagy.pagination_component.aria-label") %>">
+        <ul class="inline-flex h-10 text-base">
+          <li><button
+              type="button"
+              class="
+                flex items-center justify-center px-4 h-10 ms-0 leading-tight border
+                border-slate-300 dark:border-slate-700 cursor-default text-slate-600 bg-slate-50
+                dark:bg-slate-700 dark:text-slate-400 border-e-0 rounded-s-lg
+              "
+              data-action="click->nextflow--samplesheet#previousPage"
+              data-nextflow--samplesheet-target="previousBtn"
+              disabled="disabled"
+            ><%= t(".previous") %></button>
+          </li>
+          <li><select
+              class="
+                flex items-center justify-center px-4 h-10 ms-0 leading-tight border
+                border-slate-300 dark:border-slate-700 text-slate-500 bg-white
+                hover:bg-slate-100 hover:text-slate-700 dark:bg-slate-800 dark:text-slate-400
+                dark:hover:bg-slate-700 dark:hover:text-white cursor-pointer border-transparent
+                focus:border-transparent focus:ring-0
+              "
+              data-nextflow--samplesheet-target="pageNum"
+              data-action="change->nextflow--samplesheet#pageSelected"
+              aria-label="<%= t(".page_selection.aria_label") %>"
+            ><option value="1" selected="selected">1</option></select>
+          </li>
+          <li><button
+              class="
+                flex items-center justify-center px-4 h-10 ms-0 leading-tight border
+                border-slate-300 dark:border-slate-700 text-slate-500 bg-white
+                hover:bg-slate-100 hover:text-slate-700 dark:bg-slate-800 dark:text-slate-400
+                dark:hover:bg-slate-700 dark:hover:text-white rounded-e-lg
+              "
+              type="button"
+              data-nextflow--samplesheet-target="nextBtn"
+              data-action="click->nextflow--samplesheet#nextPage"
+            ><%= t(".next") %></button>
+          </li>
+        </ul>
+      </nav>
+    </div>
+  <% end %>
 </div>
 
 <template data-nextflow--samplesheet-target="cellContainer">
diff --git a/app/components/nextflow/samplesheet_component.rb b/app/components/nextflow/samplesheet_component.rb
index c884ca6a44..01f5f74520 100644
--- a/app/components/nextflow/samplesheet_component.rb
+++ b/app/components/nextflow/samplesheet_component.rb
@@ -17,10 +17,9 @@ def initialize(schema:, samples:, fields:, namespace_id:, workflow_params:)
     end
 
     def samples_workflow_executions_attributes
-      attributes = samples.each_with_index.to_h do |sample, index|
+      samples.each_with_index.to_h do |sample, index|
         [index, samples_workflow_execution_attributes(sample)]
       end
-      attributes.to_json
     end
 
     private
diff --git a/app/javascript/controllers/nextflow/samplesheet_controller.js b/app/javascript/controllers/nextflow/samplesheet_controller.js
index 4c720f0f1c..4fb36b9e9f 100644
--- a/app/javascript/controllers/nextflow/samplesheet_controller.js
+++ b/app/javascript/controllers/nextflow/samplesheet_controller.js
@@ -83,6 +83,7 @@ export default class extends Controller {
     this.#samplesheetAttributes = JSON.parse(
       this.workflowAttributesTarget.innerText,
     );
+
     // clear the now unnecessary DOM element
     this.workflowAttributesTarget.remove();
     this.#columnNames = Object.keys(this.#samplesheetProperties);
@@ -106,10 +107,7 @@ export default class extends Controller {
     );
 
     // disable dropdown and next button if only 1 page of samples, otherwise create the dropdown page options
-    if (this.#lastPage == 1) {
-      this.#disablePaginationButton(this.nextBtnTarget);
-      this.#disablePaginationButton(this.pageNumTarget);
-    } else {
+    if (this.#lastPage > 1) {
       this.#generatePageNumberDropdown();
     }
     // render samplesheet table
diff --git a/test/components/nextflow_samplesheet_component_test.rb b/test/components/nextflow_samplesheet_component_test.rb
index 5b496a8942..6e2ebc2cd6 100644
--- a/test/components/nextflow_samplesheet_component_test.rb
+++ b/test/components/nextflow_samplesheet_component_test.rb
@@ -11,15 +11,14 @@ class NextflowSamplesheetComponentTest < ApplicationSystemTestCase
     @sample2 = samples(:sample44)
   end
   test 'default' do
-    sample = Sample.first
-    visit('/rails/view_components/nextflow_component/default')
+    visit("/rails/view_components/nextflow_samplesheet_component/default?sample_ids[]=#{@sample1.id}&sample_ids[]=#{@sample2.id}")
 
     assert_selector '.samplesheet-table' do |table|
       table.assert_selector '.table-header', count: 5
       table.assert_selector '.table-column:last-of-type .table-header', text: 'STRANDEDNESS (REQUIRED)'
       table.assert_selector '.table-column:first-of-type .table-td', count: 2
-      table.assert_selector '.table-column:first-of-type .table-td:first-of-type', text: sample.puid
-      table.assert_selector '.table-column:nth-of-type(2) .table-td:first-of-type', text: sample.name
+      table.assert_selector '.table-column:first-of-type .table-td:first-of-type', text: @sample1.puid
+      table.assert_selector '.table-column:nth-of-type(2) .table-td:first-of-type', text: @sample1.name
       table.assert_selector '.table-column:last-of-type .table-td:first-of-type select option', count: 4
       table.assert_selector '.table-column:last-of-type .table-td:first-of-type select option:nth-of-type(2)',
                             text: 'forward'
diff --git a/test/components/previews/nextflow_samplesheet_component_preview.rb b/test/components/previews/nextflow_samplesheet_component_preview.rb
index 186d71bdfa..c5a957c083 100644
--- a/test/components/previews/nextflow_samplesheet_component_preview.rb
+++ b/test/components/previews/nextflow_samplesheet_component_preview.rb
@@ -2,13 +2,22 @@
 
 class NextflowSamplesheetComponentPreview < ViewComponent::Preview
   # @param schema_file select :schema_file_options
-  def default(schema_file: 'samplesheet_schema.json', sample_ids: [Sample.first.id, Sample.second.id])
+  def default(schema_file: 'nextflow_schema.json', sample_ids: [Sample.first.id, Sample.second.id])
     samples = Sample.where(id: sample_ids)
 
+    entry = {
+      name: 'phac-nml/iridanextexample',
+      description: 'IRIDA Next Example Pipeline',
+      url: 'https://github.com/phac-nml/iridanextexample'
+    }.with_indifferent_access
+
+    workflow = Irida::Pipeline.new(entry, '1.0.1',
+                                   Rails.root.join('test/fixtures/files/nextflow/', schema_file),
+                                   Rails.root.join('test/fixtures/files/nextflow/samplesheet_schema.json'))
+
     render_with_template(locals: {
-                           schema_file:,
                            samples:,
-                           fields: %w[insdc_accession country metadata_3]
+                           workflow:
                          })
   end
 
diff --git a/test/components/previews/nextflow_samplesheet_component_preview/default.html.erb b/test/components/previews/nextflow_samplesheet_component_preview/default.html.erb
index a81121e439..1eac9b9dd8 100644
--- a/test/components/previews/nextflow_samplesheet_component_preview/default.html.erb
+++ b/test/components/previews/nextflow_samplesheet_component_preview/default.html.erb
@@ -1,15 +1,7 @@
-<%= render Nextflow::SamplesheetComponent.new(
-  schema:
-    JSON.parse(
-      File.read(
-        Rails.root.join("test", "fixtures", "files", "nextflow", schema_file),
-      ),
-    ),
+<%= render NextflowComponent.new(
+  workflow:,
+  url: "no_where",
   samples:,
-  fields:,
+  fields: [],
   namespace_id: "SDFSDFSDF",
-  workflow_params: {
-    name: "workflow-name",
-    version: "workflow-version",
-  },
 ) %>
diff --git a/test/system/workflow_executions/submissions_test.rb b/test/system/workflow_executions/submissions_test.rb
index 19c4132d9d..90fd2f7b9f 100644
--- a/test/system/workflow_executions/submissions_test.rb
+++ b/test/system/workflow_executions/submissions_test.rb
@@ -10,6 +10,7 @@ class SubmissionsTest < ApplicationSystemTestCase
       @sample22 = samples(:sample22)
       @sample43 = samples(:sample43)
       @sample44 = samples(:sample44)
+      @sample46 = samples(:sample46)
       @project2 = projects(:project2)
       @project = projects(:project37)
       @group1 = groups(:group_one)
@@ -669,7 +670,6 @@ class SubmissionsTest < ApplicationSystemTestCase
       login_as user
       fwd_attachment = attachments(:attachmentPEFWD43)
       rev_attachment = attachments(:attachmentPEREV43)
-      sample46 = samples(:sample46)
       visit namespace_project_samples_url(namespace_id: @namespace.path, project_id: @project.path)
       # verify samples table loaded
       assert_text strip_tags(I18n.t(:'viral.pagy.limit_component.summary', from: 1, to: 3, count: 3,
@@ -701,9 +701,9 @@ class SubmissionsTest < ApplicationSystemTestCase
         assert_selector "div[id='#{@sample44.id}_fastq_2']",
                         text: I18n.t('nextflow.samplesheet.file_cell_component.no_selected_file')
 
-        assert_selector "div[id='#{sample46.id}_fastq_1']",
+        assert_selector "div[id='#{@sample46.id}_fastq_1']",
                         text: I18n.t('nextflow.samplesheet.file_cell_component.no_selected_file')
-        assert_selector "div[id='#{sample46.id}_fastq_2']",
+        assert_selector "div[id='#{@sample46.id}_fastq_2']",
                         text: I18n.t('nextflow.samplesheet.file_cell_component.no_selected_file')
         # verify error msg has not rendered
         assert_no_text I18n.t('nextflow.samplesheet_component.data_missing_error')
@@ -716,7 +716,7 @@ class SubmissionsTest < ApplicationSystemTestCase
         within('div[data-nextflow--samplesheet-target="error"]') do
           assert_text "#{I18n.t('nextflow_component.data_missing_error')}"
           assert_text "- #{@sample44.puid}: fastq_1"
-          assert_text "- #{sample46.puid}: fastq_1"
+          assert_text "- #{@sample46.puid}: fastq_1"
         end
         ### VERIFY END ###
       end
@@ -858,5 +858,51 @@ class SubmissionsTest < ApplicationSystemTestCase
       end
       ### VERIFY END ###
     end
+
+    test 'pagination does not render if only one page of samples' do
+      ### SETUP START ###
+      user = users(:john_doe)
+      login_as user
+      visit namespace_project_samples_url(namespace_id: @namespace.path, project_id: @project.path)
+      # verify samples table loaded
+      assert_text strip_tags(I18n.t(:'viral.pagy.limit_component.summary', from: 1, to: 3, count: 3,
+                                                                           locale: user.locale))
+      # select samples
+      click_button I18n.t(:'projects.samples.index.select_all_button')
+      # launch workflow execution dialog
+      click_on I18n.t(:'projects.samples.index.workflows.button_sr')
+
+      assert_selector '#dialog'
+      within %(turbo-frame[id="samples_dialog"]) do
+        assert_selector '.dialog--header', text: I18n.t(:'workflow_executions.submissions.pipeline_selection.title')
+        assert_button text: 'phac-nml/iridanextexample', count: 3
+        first('button', text: 'phac-nml/iridanextexample').click
+      end
+      ### SETUP END ###
+
+      ### ACTIONS START ###
+      within '#dialog' do
+        # verify samples samplesheet loaded
+        assert_selector 'div.sample-sheet'
+        within('#metadata-sample-column') do
+          assert_text @sample43.puid
+          assert_text @sample44.puid
+          assert_text @sample46.puid
+        end
+        ### ACTIONS END ###
+
+        ### VERIFY START ###
+        # verify no pagination buttons rendered
+        assert_no_selector '#samplesheet_pagination'
+        assert_no_selector 'button[data-action="click->nextflow--samplesheet#previousPage"][disabled]',
+                           text: I18n.t('nextflow.samplesheet_component.previous')
+        assert_no_selector 'select[data-action="change->nextflow--samplesheet#pageSelected"]', text: '1'
+
+        assert_no_selector 'button[data-action="click->nextflow--samplesheet#nextPage"]',
+                           text: I18n.t('nextflow.samplesheet_component.next')
+
+        ### VERIFY END ###
+      end
+    end
   end
 end