From f1e84573f746477fd9445e3856e8709ecbe2be60 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Tue, 10 Dec 2024 19:25:53 -0600 Subject: [PATCH 01/29] feat: :sparkles: Initial work on editing line list --- app/components/samples/table_component.html.erb | 13 ++++++++++--- app/components/samples/table_component.rb | 2 ++ app/controllers/projects/samples_controller.rb | 17 +++++++++++++++++ .../projects/samples/_edit_field_form.html.erb | 1 + app/views/projects/samples/_table.html.erb | 5 +++++ 5 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 app/views/projects/samples/_edit_field_form.html.erb diff --git a/app/components/samples/table_component.html.erb b/app/components/samples/table_component.html.erb index af09179b27..66323b1f36 100644 --- a/app/components/samples/table_component.html.erb +++ b/app/components/samples/table_component.html.erb @@ -135,9 +135,16 @@ <% @metadata_fields.each do |field| %> <%= render_cell( tag: 'td', - classes: class_names('px-3 py-3') + id: dom_id(sample, field), + + ) do %> - <%= sample.metadata[field] %> + <%= form_with(url: @check_editable_url, method: :get) do |form| %> + <%= form.hidden_field :id, value: sample.id %> + <%= form.hidden_field :field, value: field %> + <%= form.submit sample.metadata[field], class: "cursor-pointer p-4" %> + + <% end %> <% end %> <% end %> <% if @renders_row_actions %> @@ -199,7 +206,7 @@ >0 - + <% end %> diff --git a/app/components/samples/table_component.rb b/app/components/samples/table_component.rb index ea42e0bde0..b908fa2b90 100644 --- a/app/components/samples/table_component.rb +++ b/app/components/samples/table_component.rb @@ -18,6 +18,7 @@ def initialize( search_params: {}, row_actions: {}, empty: {}, + check_editable_url: nil, **system_arguments ) @samples = samples @@ -29,6 +30,7 @@ def initialize( @search_params = search_params @row_actions = row_actions @empty = empty + @check_editable_url = check_editable_url @renders_row_actions = @row_actions.select { |_key, value| value }.count.positive? @system_arguments = system_arguments diff --git a/app/controllers/projects/samples_controller.rb b/app/controllers/projects/samples_controller.rb index b7cb6abc06..d00c8c3783 100644 --- a/app/controllers/projects/samples_controller.rb +++ b/app/controllers/projects/samples_controller.rb @@ -91,6 +91,23 @@ def select end end + def check_editable + @sample = Sample.find(params[:id]) + @field = params[:field] + + if @sample.field_editable?(@field) + render turbo_stream: turbo_stream.replace( + helpers.dom_id(@sample, @field), + partial: 'samples/edit_field_form', + locals: { sample: @sample, field: @field } + ) + else + render turbo_stream: turbo_stream.append('flash', + partial: 'shared/alert', + locals: { message: 'This field is not editable' }) + end + end + private def sample diff --git a/app/views/projects/samples/_edit_field_form.html.erb b/app/views/projects/samples/_edit_field_form.html.erb new file mode 100644 index 0000000000..4d848e0d25 --- /dev/null +++ b/app/views/projects/samples/_edit_field_form.html.erb @@ -0,0 +1 @@ +
FOOBAR
diff --git a/app/views/projects/samples/_table.html.erb b/app/views/projects/samples/_table.html.erb index 1dc9438313..c071b6f662 100644 --- a/app/views/projects/samples/_table.html.erb +++ b/app/views/projects/samples/_table.html.erb @@ -21,4 +21,9 @@ title: t(:"projects.samples.index.no_samples"), description: t(:"projects.samples.index.no_associated_samples"), }, + check_editable_url: + check_editable_namespace_project_samples_path( + project.namespace.parent, + project, + ), ) %> From 0fd806937f05f13268305d2e00f851d627ad99e6 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Tue, 10 Dec 2024 20:38:27 -0600 Subject: [PATCH 02/29] feat: Add check_editable route to project for enhanced editing capabilities --- config/routes/project.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/config/routes/project.rb b/config/routes/project.rb index d329342ed2..d4d507191f 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -59,6 +59,7 @@ get :select post :list post :search + get :check_editable end resources :attachments, module: :samples, only: %i[new create destroy] do scope module: :attachments, as: :attachments do From 75c712e7387328e39e94d0c0113a9bd210d8bf80 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Tue, 10 Dec 2024 21:27:45 -0600 Subject: [PATCH 03/29] feat: Enhance sample editing functionality with update_field action and form improvements - Added a new `update_field` action in the SamplesController to handle field updates. - Updated the `check_editable` method to pass the current value to the edit form. - Modified the `_edit_field_form` partial to include hidden fields for original value and format. - Updated routes to include the new `update_field` patch route for samples. - Improved the HTML structure of the edit field form for better integration with Turbo Streams. --- app/components/samples/table_component.html.erb | 3 ++- app/controllers/projects/samples_controller.rb | 14 ++++++++++++-- .../projects/samples/_edit_field_form.html.erb | 11 ++++++++++- config/routes/project.rb | 1 + 4 files changed, 25 insertions(+), 4 deletions(-) diff --git a/app/components/samples/table_component.html.erb b/app/components/samples/table_component.html.erb index 66323b1f36..5f23f6f46b 100644 --- a/app/components/samples/table_component.html.erb +++ b/app/components/samples/table_component.html.erb @@ -142,8 +142,9 @@ <%= form_with(url: @check_editable_url, method: :get) do |form| %> <%= form.hidden_field :id, value: sample.id %> <%= form.hidden_field :field, value: field %> + <%= form.hidden_field :value, value: sample.metadata[field] %> + <%= form.hidden_field :format, value: "turbo_stream" %> <%= form.submit sample.metadata[field], class: "cursor-pointer p-4" %> - <% end %> <% end %> <% end %> diff --git a/app/controllers/projects/samples_controller.rb b/app/controllers/projects/samples_controller.rb index d00c8c3783..860a2105fd 100644 --- a/app/controllers/projects/samples_controller.rb +++ b/app/controllers/projects/samples_controller.rb @@ -94,12 +94,13 @@ def select def check_editable @sample = Sample.find(params[:id]) @field = params[:field] + @value = params[:value] if @sample.field_editable?(@field) render turbo_stream: turbo_stream.replace( helpers.dom_id(@sample, @field), - partial: 'samples/edit_field_form', - locals: { sample: @sample, field: @field } + partial: 'edit_field_form', + locals: { sample: @sample, field: @field, value: @value } ) else render turbo_stream: turbo_stream.append('flash', @@ -108,6 +109,15 @@ def check_editable end end + def update_field + @sample = Sample.find(params[:id]) + @field = params[:field] + @value = params[:value] + @original_value = params[:original_value] + + @sample.update(metadata: { @field => @value }) if @value != @original_value + end + private def sample diff --git a/app/views/projects/samples/_edit_field_form.html.erb b/app/views/projects/samples/_edit_field_form.html.erb index 4d848e0d25..5cd360feee 100644 --- a/app/views/projects/samples/_edit_field_form.html.erb +++ b/app/views/projects/samples/_edit_field_form.html.erb @@ -1 +1,10 @@ -
FOOBAR
+ + <%= form_with(url: update_field_namespace_project_samples_path(@project.namespace.parent, + @project), method: :patch) do |f| %> + <%= f.hidden_field :id, value: @sample.id %> + <%= f.hidden_field :field, value: @field %> + <%= f.hidden_field :original_value, value: @value %> + <%= f.hidden_field :format, value: "turbo_stream" %> + <%= f.text_field :value, value: @value, autofocus: true, class: "w-full m-0" %> + <% end %> + diff --git a/config/routes/project.rb b/config/routes/project.rb index d4d507191f..beaac6b946 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -60,6 +60,7 @@ post :list post :search get :check_editable + patch :update_field end resources :attachments, module: :samples, only: %i[new create destroy] do scope module: :attachments, as: :attachments do From 196d732cfa7f41bc14b1060c127ba34628976d97 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Wed, 11 Dec 2024 07:14:02 -0600 Subject: [PATCH 04/29] feat: Implement editable cell component for enhanced inline editing of sample fields - Added `EditableCell` component to streamline the rendering of editable table cells. - Refactored `table_component.html.erb` to utilize the new `EditableCell` for cleaner code and improved maintainability. - Updated `samples_controller.rb` to render Turbo Stream updates for editable fields. - Introduced `inline_edit_controller.js` to manage focus and submission of inline edit forms. - Enhanced `_edit_field_form.html.erb` to integrate with the new inline editing functionality. - Created `_editable_table_field.html.erb` partial to encapsulate editable cell rendering logic. --- app/components/samples/editable_cell.rb | 27 +++++++++++++++++++ .../samples/table_component.html.erb | 21 ++++----------- .../projects/samples_controller.rb | 6 +++++ .../controllers/inline_edit_controller.js | 14 ++++++++++ .../samples/_edit_field_form.html.erb | 10 +++++-- .../samples/_editable_table_field.html.erb | 9 +++++++ 6 files changed, 69 insertions(+), 18 deletions(-) create mode 100644 app/components/samples/editable_cell.rb create mode 100644 app/javascript/controllers/inline_edit_controller.js create mode 100644 app/views/projects/samples/_editable_table_field.html.erb diff --git a/app/components/samples/editable_cell.rb b/app/components/samples/editable_cell.rb new file mode 100644 index 0000000000..a64aa79f3c --- /dev/null +++ b/app/components/samples/editable_cell.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Samples + # Component for rendering an editable cell + class EditableCell < Component + with_collection_parameter :field + + erb_template <<~ERB + + <%= form_with(url: @check_editable_url, method: :get) do |form| %> + <%= form.hidden_field :id, value: @sample.id %> + <%= form.hidden_field :field, value: @field %> + <%= form.hidden_field :value, value: @value %> + <%= form.hidden_field :format, value: "turbo_stream" %> + <%= form.submit @value, class: "cursor-pointer p-4 hover:bg-primary-50 w-full text-left" %> + <% end %> + + ERB + + def initialize(field:, sample:, check_editable_url:) + @sample = sample + @field = field + @value = sample.metadata[field] + @check_editable_url = check_editable_url + end + end +end diff --git a/app/components/samples/table_component.html.erb b/app/components/samples/table_component.html.erb index 5f23f6f46b..72878b3077 100644 --- a/app/components/samples/table_component.html.erb +++ b/app/components/samples/table_component.html.erb @@ -132,22 +132,11 @@ <% end %> <% end %> <% end %> - <% @metadata_fields.each do |field| %> - <%= render_cell( - tag: 'td', - id: dom_id(sample, field), - - - ) do %> - <%= form_with(url: @check_editable_url, method: :get) do |form| %> - <%= form.hidden_field :id, value: sample.id %> - <%= form.hidden_field :field, value: field %> - <%= form.hidden_field :value, value: sample.metadata[field] %> - <%= form.hidden_field :format, value: "turbo_stream" %> - <%= form.submit sample.metadata[field], class: "cursor-pointer p-4" %> - <% end %> - <% end %> - <% end %> + <%= render Samples::EditableCell.with_collection( + @metadata_fields, + sample: sample, + check_editable_url: @check_editable_url, + ) %> <% if @renders_row_actions %> <%= render_cell( tag: 'td', diff --git a/app/controllers/projects/samples_controller.rb b/app/controllers/projects/samples_controller.rb index 860a2105fd..c874f8bf33 100644 --- a/app/controllers/projects/samples_controller.rb +++ b/app/controllers/projects/samples_controller.rb @@ -116,6 +116,12 @@ def update_field @original_value = params[:original_value] @sample.update(metadata: { @field => @value }) if @value != @original_value + + render turbo_stream: turbo_stream.replace( + helpers.dom_id(@sample, @field), + partial: 'editable_table_field', + locals: { sample: @sample, field: @field, value: @value } + ) end private diff --git a/app/javascript/controllers/inline_edit_controller.js b/app/javascript/controllers/inline_edit_controller.js new file mode 100644 index 0000000000..a054c6ce55 --- /dev/null +++ b/app/javascript/controllers/inline_edit_controller.js @@ -0,0 +1,14 @@ +import { Controller } from "@hotwired/stimulus"; + +export default class extends Controller { + static targets = ["input"]; + + connect() { + this.inputTarget.focus(); + this.inputTarget.select(); + } + + submit() { + this.element.requestSubmit(); + } +} diff --git a/app/views/projects/samples/_edit_field_form.html.erb b/app/views/projects/samples/_edit_field_form.html.erb index 5cd360feee..d4873c5b3c 100644 --- a/app/views/projects/samples/_edit_field_form.html.erb +++ b/app/views/projects/samples/_edit_field_form.html.erb @@ -1,10 +1,16 @@ <%= form_with(url: update_field_namespace_project_samples_path(@project.namespace.parent, - @project), method: :patch) do |f| %> + @project), method: :patch, data: { controller: "inline-edit" }) do |f| %> <%= f.hidden_field :id, value: @sample.id %> <%= f.hidden_field :field, value: @field %> <%= f.hidden_field :original_value, value: @value %> <%= f.hidden_field :format, value: "turbo_stream" %> - <%= f.text_field :value, value: @value, autofocus: true, class: "w-full m-0" %> + <%= f.text_field :value, + value: @value, + class: "w-full m-0", + data: { + "inline-edit-target": "input", + action: "blur->inline-edit#submit", + } %> <% end %> diff --git a/app/views/projects/samples/_editable_table_field.html.erb b/app/views/projects/samples/_editable_table_field.html.erb new file mode 100644 index 0000000000..52addea5d8 --- /dev/null +++ b/app/views/projects/samples/_editable_table_field.html.erb @@ -0,0 +1,9 @@ +<%= render Samples::EditableCell.new( + field: @field, + sample: @sample, + check_editable_url: + check_editable_namespace_project_samples_path( + @sample.project.namespace.parent, + @sample.project, + ), +) %> From 54ecb45c45412634b5b3ed5fafc72dc6cdccacb5 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Wed, 11 Dec 2024 07:56:52 -0600 Subject: [PATCH 05/29] feat: Refactor sample metadata update logic in SamplesController - Replaced direct update call with a new service object for handling metadata updates. - Enhanced the update process to better manage field updates and original values. - Improved code readability and maintainability by encapsulating update logic within a dedicated service. --- app/controllers/projects/samples_controller.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/app/controllers/projects/samples_controller.rb b/app/controllers/projects/samples_controller.rb index c874f8bf33..29c5b8f393 100644 --- a/app/controllers/projects/samples_controller.rb +++ b/app/controllers/projects/samples_controller.rb @@ -115,7 +115,17 @@ def update_field @value = params[:value] @original_value = params[:original_value] - @sample.update(metadata: { @field => @value }) if @value != @original_value + updated_fields = ::Samples::Metadata::Fields::UpdateService.new(@project, @sample, current_user, + { + 'update_field' => { + 'key' => { + @field => @field + }, + 'value' => { + @original_value => @value + } + } + }).execute render turbo_stream: turbo_stream.replace( helpers.dom_id(@sample, @field), From 10dc64b8533551951ced943590d794e946ab366b Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Wed, 11 Dec 2024 09:48:09 -0600 Subject: [PATCH 06/29] feat: Refactor update_field action in SamplesController for improved error handling and code organization - Introduced private methods to encapsulate logic for setting field variables and updating sample fields. - Enhanced error handling by rendering appropriate responses based on sample validation errors. - Improved code readability and maintainability by organizing the update process into dedicated methods. --- .../projects/samples_controller.rb | 50 +++++++++++++++---- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/app/controllers/projects/samples_controller.rb b/app/controllers/projects/samples_controller.rb index 29c5b8f393..5d1387e294 100644 --- a/app/controllers/projects/samples_controller.rb +++ b/app/controllers/projects/samples_controller.rb @@ -110,23 +110,51 @@ def check_editable end def update_field + set_field_variables + update_sample_field + + if @sample.errors.any? + render_error_response + else + render_success_response + end + end + + private + + def set_field_variables @sample = Sample.find(params[:id]) @field = params[:field] @value = params[:value] @original_value = params[:original_value] + end + + def update_sample_field + ::Samples::Metadata::Fields::UpdateService.new( + @project, + @sample, + current_user, + update_field_params + ).execute + end - updated_fields = ::Samples::Metadata::Fields::UpdateService.new(@project, @sample, current_user, - { - 'update_field' => { - 'key' => { - @field => @field - }, - 'value' => { - @original_value => @value - } - } - }).execute + def update_field_params + { + 'update_field' => { + 'key' => { @field => @field }, + 'value' => { @original_value => @value } + } + } + end + + def render_error_response + render status: :unprocessable_entity, locals: { + type: 'error', + message: @sample.errors.full_messages.first + } + end + def render_success_response render turbo_stream: turbo_stream.replace( helpers.dom_id(@sample, @field), partial: 'editable_table_field', From 4a5b71845f8118640b250172cd0d38c7e760dc68 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Wed, 11 Dec 2024 10:41:05 -0600 Subject: [PATCH 07/29] feat: Enhance editable cell functionality and refactor update_field action - Updated the styling of the submit button in the EditableCell component for improved visual consistency. - Refactored the update_field action in SamplesController to streamline the update process using a single service call. - Improved the HTML structure of the edit field form to enhance integration with Turbo Streams and maintain a consistent user experience. --- app/components/samples/editable_cell.rb | 2 +- .../projects/samples_controller.rb | 50 ++++--------------- .../samples/_edit_field_form.html.erb | 3 +- 3 files changed, 14 insertions(+), 41 deletions(-) diff --git a/app/components/samples/editable_cell.rb b/app/components/samples/editable_cell.rb index a64aa79f3c..b602e591b8 100644 --- a/app/components/samples/editable_cell.rb +++ b/app/components/samples/editable_cell.rb @@ -12,7 +12,7 @@ class EditableCell < Component <%= form.hidden_field :field, value: @field %> <%= form.hidden_field :value, value: @value %> <%= form.hidden_field :format, value: "turbo_stream" %> - <%= form.submit @value, class: "cursor-pointer p-4 hover:bg-primary-50 w-full text-left" %> + <%= form.submit @value, class: "cursor-pointer p-4 hover:bg-slate-50 dark:hover:bg-slate-600 w-full text-left" %> <% end %> ERB diff --git a/app/controllers/projects/samples_controller.rb b/app/controllers/projects/samples_controller.rb index 5d1387e294..29c5b8f393 100644 --- a/app/controllers/projects/samples_controller.rb +++ b/app/controllers/projects/samples_controller.rb @@ -110,51 +110,23 @@ def check_editable end def update_field - set_field_variables - update_sample_field - - if @sample.errors.any? - render_error_response - else - render_success_response - end - end - - private - - def set_field_variables @sample = Sample.find(params[:id]) @field = params[:field] @value = params[:value] @original_value = params[:original_value] - end - - def update_sample_field - ::Samples::Metadata::Fields::UpdateService.new( - @project, - @sample, - current_user, - update_field_params - ).execute - end - def update_field_params - { - 'update_field' => { - 'key' => { @field => @field }, - 'value' => { @original_value => @value } - } - } - end - - def render_error_response - render status: :unprocessable_entity, locals: { - type: 'error', - message: @sample.errors.full_messages.first - } - end + updated_fields = ::Samples::Metadata::Fields::UpdateService.new(@project, @sample, current_user, + { + 'update_field' => { + 'key' => { + @field => @field + }, + 'value' => { + @original_value => @value + } + } + }).execute - def render_success_response render turbo_stream: turbo_stream.replace( helpers.dom_id(@sample, @field), partial: 'editable_table_field', diff --git a/app/views/projects/samples/_edit_field_form.html.erb b/app/views/projects/samples/_edit_field_form.html.erb index d4873c5b3c..b52ebabff5 100644 --- a/app/views/projects/samples/_edit_field_form.html.erb +++ b/app/views/projects/samples/_edit_field_form.html.erb @@ -7,7 +7,8 @@ <%= f.hidden_field :format, value: "turbo_stream" %> <%= f.text_field :value, value: @value, - class: "w-full m-0", + class: + "w-full m-0 border-slate-300 text-slate-900 text-sm focus:ring-primary-500 focus:border-primary-500 block w-full p-2.5 dark:bg-slate-700 dark:border-slate-600 dark:text-white dark:focus:ring-primary-500 dark:focus:border-primary-500", data: { "inline-edit-target": "input", action: "blur->inline-edit#submit", From 910864212ef4bc291adb10239c3d5fd248a94f47 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Wed, 11 Dec 2024 13:39:19 -0600 Subject: [PATCH 08/29] feat: Add test for editing sample fields in samples_test.rb - Introduced a new test case to verify the functionality of editing a sample field. - The test navigates to the sample's edit page, interacts with the metadata toggle, and checks for the correct display of the sample's metadata. - Ensures that the edit field button is functional and properly integrated with the existing sample editing features. --- test/system/projects/samples_test.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/system/projects/samples_test.rb b/test/system/projects/samples_test.rb index 86d3cff973..cb063c9d6e 100644 --- a/test/system/projects/samples_test.rb +++ b/test/system/projects/samples_test.rb @@ -1548,6 +1548,16 @@ def retrieve_puids end end + test 'can edit a sample field' do + visit namespace_project_samples_url(@namespace, @project) + find('label', text: I18n.t('projects.samples.shared.metadata_toggle.label')).click + + within 'tbody' do + assert_text @sample1.metadata[@field] + click_link I18n.t('projects.samples.show.edit_field_button', field: @field) + end + end + def long_filter_text text = (1..500).map { |n| "sample#{n}" }.join(', ') "#{text}, #{@sample1.name}" # Need to comma to force the tag to be created From 5e7e46f321c74c67fd9266475393d7402707188f Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Thu, 12 Dec 2024 08:39:53 -0600 Subject: [PATCH 09/29] feat: Implement inline editing for sample fields with check_editable and update_field actions - Added `check_editable` method in SamplesController to verify field editability and render appropriate forms. - Introduced `update_field` action to handle updates for sample fields, utilizing a dedicated service for processing. - Created `_edit_field_form.html.erb` partial for inline editing, including necessary hidden fields for seamless updates. - Added `_editable_table_field.html.erb` partial to encapsulate editable cell rendering logic. - Updated routes to include new paths for `check_editable` and `update_field` actions. - Enhanced `_table.html.erb` to integrate the new editable functionality, improving user experience with Turbo Streams. --- app/controllers/groups/samples_controller.rb | 44 +++++++++++++++++++ .../groups/samples/_edit_field_form.html.erb | 17 +++++++ .../samples/_editable_table_field.html.erb | 5 +++ app/views/groups/samples/_table.html.erb | 1 + config/routes/group.rb | 2 + 5 files changed, 69 insertions(+) create mode 100644 app/views/groups/samples/_edit_field_form.html.erb create mode 100644 app/views/groups/samples/_editable_table_field.html.erb diff --git a/app/controllers/groups/samples_controller.rb b/app/controllers/groups/samples_controller.rb index a827b35a4b..d067ef071b 100644 --- a/app/controllers/groups/samples_controller.rb +++ b/app/controllers/groups/samples_controller.rb @@ -34,6 +34,50 @@ def select end end + def check_editable + @sample = Sample.find(params[:id]) + @field = params[:field] + @value = params[:value] + + if @sample.field_editable?(@field) + render turbo_stream: turbo_stream.replace( + helpers.dom_id(@sample, @field), + partial: 'edit_field_form', + locals: { sample: @sample, field: @field, value: @value } + ) + else + render turbo_stream: turbo_stream.append('flash', + partial: 'shared/alert', + locals: { message: 'This field is not editable' }) + end + end + + def update_field + @sample = Sample.find(params[:id]) + @project = Project.find(params[:project_id]) + @field = params[:field] + @value = params[:value] + @original_value = params[:original_value] + + updated_fields = ::Samples::Metadata::Fields::UpdateService.new(@project, @sample, current_user, + { + 'update_field' => { + 'key' => { + @field => @field + }, + 'value' => { + @original_value => @value + } + } + }).execute + + render turbo_stream: turbo_stream.replace( + helpers.dom_id(@sample, @field), + partial: 'editable_table_field', + locals: { sample: @sample, field: @field, value: @value } + ) + end + private def group diff --git a/app/views/groups/samples/_edit_field_form.html.erb b/app/views/groups/samples/_edit_field_form.html.erb new file mode 100644 index 0000000000..03a09c121a --- /dev/null +++ b/app/views/groups/samples/_edit_field_form.html.erb @@ -0,0 +1,17 @@ + + <%= form_with(url: update_field_group_samples_path(@group), method: :patch, data: { controller: "inline-edit" }) do |f| %> + <%= f.hidden_field :id, value: @sample.id %> + <%= f.hidden_field :project_id, value: @sample.project.id %> + <%= f.hidden_field :field, value: @field %> + <%= f.hidden_field :original_value, value: @value %> + <%= f.hidden_field :format, value: "turbo_stream" %> + <%= f.text_field :value, + value: @value, + class: + "w-full m-0 border-slate-300 text-slate-900 text-sm focus:ring-primary-500 focus:border-primary-500 block w-full p-2.5 dark:bg-slate-700 dark:border-slate-600 dark:text-white dark:focus:ring-primary-500 dark:focus:border-primary-500", + data: { + "inline-edit-target": "input", + action: "blur->inline-edit#submit", + } %> + <% end %> + diff --git a/app/views/groups/samples/_editable_table_field.html.erb b/app/views/groups/samples/_editable_table_field.html.erb new file mode 100644 index 0000000000..26fce3ecde --- /dev/null +++ b/app/views/groups/samples/_editable_table_field.html.erb @@ -0,0 +1,5 @@ +<%= render Samples::EditableCell.new( + field: @field, + sample: @sample, + check_editable_url: check_editable_group_samples_path(@group), +) %> diff --git a/app/views/groups/samples/_table.html.erb b/app/views/groups/samples/_table.html.erb index d97389a312..0591d0e058 100644 --- a/app/views/groups/samples/_table.html.erb +++ b/app/views/groups/samples/_table.html.erb @@ -15,4 +15,5 @@ title: t(:"groups.samples.table.no_samples"), description: t(:"groups.samples.table.no_associated_samples"), }, + check_editable_url: check_editable_group_samples_path(group), ) %> diff --git a/config/routes/group.rb b/config/routes/group.rb index dfa60aab2b..008ab03b17 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -38,6 +38,8 @@ collection do get :select post :search + get :check_editable + patch :update_field end end resources :subgroups, only: %i[index] From 2fbd0e65c65551e32d7e21d1a7bda922023808b4 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Thu, 12 Dec 2024 10:33:43 -0600 Subject: [PATCH 10/29] fix: Update sample field test to assert correct button interaction and field visibility - Modified the test in `samples_test.rb` to assert the presence of the sample's metadata value directly. - Changed the interaction from clicking an edit field button to clicking a button that directly represents the sample's value, enhancing the test's accuracy and clarity. - Added an assertion to verify that the field is correctly displayed after the interaction, ensuring the test covers the expected behavior of the inline editing feature. --- test/system/projects/samples_test.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/system/projects/samples_test.rb b/test/system/projects/samples_test.rb index cb063c9d6e..7887ab64f0 100644 --- a/test/system/projects/samples_test.rb +++ b/test/system/projects/samples_test.rb @@ -1553,8 +1553,9 @@ def retrieve_puids find('label', text: I18n.t('projects.samples.shared.metadata_toggle.label')).click within 'tbody' do - assert_text @sample1.metadata[@field] - click_link I18n.t('projects.samples.show.edit_field_button', field: @field) + assert_text 'value1' + click_button 'value1' + assert_field 'value1' end end From bb00771acc8f5ebab03e3d32444014635bf925c3 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Thu, 12 Dec 2024 10:57:07 -0600 Subject: [PATCH 11/29] feat: Enhance inline editing functionality with cancel support and original value handling - Added `original` value support in `inline_edit_controller.js` to store the initial input value. - Implemented `cancel` method to revert changes on Escape key press, improving user experience. - Updated `_edit_field_form.html.erb` and `_edit_field_form.html.erb` to pass original value to the inline edit controller. - Modified input field actions to include cancel functionality, allowing users to easily discard edits. --- app/javascript/controllers/inline_edit_controller.js | 12 +++++++++++- app/views/groups/samples/_edit_field_form.html.erb | 5 +++-- app/views/projects/samples/_edit_field_form.html.erb | 4 ++-- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/app/javascript/controllers/inline_edit_controller.js b/app/javascript/controllers/inline_edit_controller.js index a054c6ce55..b88b55a990 100644 --- a/app/javascript/controllers/inline_edit_controller.js +++ b/app/javascript/controllers/inline_edit_controller.js @@ -2,13 +2,23 @@ import { Controller } from "@hotwired/stimulus"; export default class extends Controller { static targets = ["input"]; + static values = { + original: String, + }; connect() { this.inputTarget.focus(); this.inputTarget.select(); } - submit() { + submit(event) { this.element.requestSubmit(); } + + cancel(event) { + if (event.key === "Escape") { + this.inputTarget.value = this.originalValue; + this.inputTarget.blur(); + } + } } diff --git a/app/views/groups/samples/_edit_field_form.html.erb b/app/views/groups/samples/_edit_field_form.html.erb index 03a09c121a..2d88014630 100644 --- a/app/views/groups/samples/_edit_field_form.html.erb +++ b/app/views/groups/samples/_edit_field_form.html.erb @@ -1,5 +1,6 @@ - <%= form_with(url: update_field_group_samples_path(@group), method: :patch, data: { controller: "inline-edit" }) do |f| %> + <%= form_with(url: update_field_group_samples_path(@group), method: :patch, + data: { controller: "inline-edit", "inline-edit-original-value": @value }) do |f| %> <%= f.hidden_field :id, value: @sample.id %> <%= f.hidden_field :project_id, value: @sample.project.id %> <%= f.hidden_field :field, value: @field %> @@ -11,7 +12,7 @@ "w-full m-0 border-slate-300 text-slate-900 text-sm focus:ring-primary-500 focus:border-primary-500 block w-full p-2.5 dark:bg-slate-700 dark:border-slate-600 dark:text-white dark:focus:ring-primary-500 dark:focus:border-primary-500", data: { "inline-edit-target": "input", - action: "blur->inline-edit#submit", + action: "blur->inline-edit#submit keydown->inline-edit#cancel", } %> <% end %> diff --git a/app/views/projects/samples/_edit_field_form.html.erb b/app/views/projects/samples/_edit_field_form.html.erb index b52ebabff5..01e5576b48 100644 --- a/app/views/projects/samples/_edit_field_form.html.erb +++ b/app/views/projects/samples/_edit_field_form.html.erb @@ -1,6 +1,6 @@ <%= form_with(url: update_field_namespace_project_samples_path(@project.namespace.parent, - @project), method: :patch, data: { controller: "inline-edit" }) do |f| %> + @project), method: :patch, data: { controller: "inline-edit", "inline-edit-original-value": @value }) do |f| %> <%= f.hidden_field :id, value: @sample.id %> <%= f.hidden_field :field, value: @field %> <%= f.hidden_field :original_value, value: @value %> @@ -11,7 +11,7 @@ "w-full m-0 border-slate-300 text-slate-900 text-sm focus:ring-primary-500 focus:border-primary-500 block w-full p-2.5 dark:bg-slate-700 dark:border-slate-600 dark:text-white dark:focus:ring-primary-500 dark:focus:border-primary-500", data: { "inline-edit-target": "input", - action: "blur->inline-edit#submit", + action: "blur->inline-edit#submit keydown->inline-edit#cancel", } %> <% end %> From b09db052680088d1a2517c68e80227c8774fc8c8 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Fri, 13 Dec 2024 06:45:58 -0600 Subject: [PATCH 12/29] feat: Add field_editable? method to Sample model for user metadata validation - Introduced `field_editable?` method to determine if a field is editable based on its provenance. - Enhanced the model's capability to manage user-specific metadata, improving the inline editing functionality. --- app/models/sample.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/sample.rb b/app/models/sample.rb index 2918fa37e5..c77b2ebe22 100644 --- a/app/models/sample.rb +++ b/app/models/sample.rb @@ -98,6 +98,10 @@ def search_data } end + def field_editable?(field) + metadata_provenance.key?(field) && metadata_provenance[field]['source'] == 'user' + end + def should_index? !deleted? end From 0c30cce1706ec2386d75be50031122746cd2b461 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Fri, 13 Dec 2024 07:06:49 -0600 Subject: [PATCH 13/29] refactor: Update sample field test for inline editing functionality - Renamed test to clarify it checks inline editing of a sample metadata field. - Added assertions to verify the presence of metadata toggle label and correct sample values in the table. - Enhanced test coverage for inline editing interactions, ensuring accurate representation of sample data. --- test/system/projects/samples_test.rb | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/system/projects/samples_test.rb b/test/system/projects/samples_test.rb index 7887ab64f0..7e71ed661e 100644 --- a/test/system/projects/samples_test.rb +++ b/test/system/projects/samples_test.rb @@ -1548,14 +1548,17 @@ def retrieve_puids end end - test 'can edit a sample field' do + test 'can inline edit a sample metadata field' do visit namespace_project_samples_url(@namespace, @project) + + assert_selector 'label', text: I18n.t(:'projects.samples.shared.metadata_toggle.label'), count: 1 find('label', text: I18n.t('projects.samples.shared.metadata_toggle.label')).click - within 'tbody' do - assert_text 'value1' - click_button 'value1' - assert_field 'value1' + assert_selector '#samples-table table thead tr th', count: 8 + within('#samples-table table tbody tr:first-child') do + assert_text @sample3.name + assert_selector 'td:nth-child(6)', text: 'value1' + assert_selector 'td:nth-child(7)', text: 'value2' end end From 7efae73a7479ab509dee16a43aebd17651051ffa Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Fri, 13 Dec 2024 12:32:40 -0600 Subject: [PATCH 14/29] refactor: Simplify editable cell functionality and remove unused actions - Updated `EditableCell` component to remove the `check_editable_url` parameter, streamlining the initialization process. - Refactored `SamplesController` to eliminate `check_editable` and `update_field` actions, consolidating field update logic into the `metadata/fields_controller`. - Introduced new `editable` and `update_value` actions in `fields_controller` to handle inline editing more effectively. - Removed obsolete partials related to inline editing and updated views to reflect the new structure. - Enhanced Turbo Stream responses for editable fields, improving user experience during inline edits. --- app/components/samples/editable_cell.rb | 10 ++-- .../samples/table_component.html.erb | 6 +-- app/components/samples/table_component.rb | 2 - app/controllers/groups/samples_controller.rb | 44 --------------- .../samples/metadata/fields_controller.rb | 54 +++++++++++++++++++ .../projects/samples_controller.rb | 43 --------------- .../controllers/inline_edit_controller.js | 8 ++- app/models/sample.rb | 2 +- .../samples/_editable_table_field.html.erb | 5 -- app/views/groups/samples/_table.html.erb | 1 - .../samples/_edit_field_form.html.erb | 17 ------ .../samples/_editable_table_field.html.erb | 9 ---- app/views/projects/samples/_table.html.erb | 5 -- app/views/shared/_flash.html.erb | 3 ++ .../fields/_editable_field_cell.html.erb | 1 + .../fields/_editing_field_cell.html.erb} | 3 +- config/routes/group.rb | 2 - config/routes/project.rb | 7 +-- 18 files changed, 74 insertions(+), 148 deletions(-) delete mode 100644 app/views/groups/samples/_editable_table_field.html.erb delete mode 100644 app/views/projects/samples/_edit_field_form.html.erb delete mode 100644 app/views/projects/samples/_editable_table_field.html.erb create mode 100644 app/views/shared/_flash.html.erb create mode 100644 app/views/shared/samples/metadata/fields/_editable_field_cell.html.erb rename app/views/{groups/samples/_edit_field_form.html.erb => shared/samples/metadata/fields/_editing_field_cell.html.erb} (85%) diff --git a/app/components/samples/editable_cell.rb b/app/components/samples/editable_cell.rb index b602e591b8..e1a2a29e1c 100644 --- a/app/components/samples/editable_cell.rb +++ b/app/components/samples/editable_cell.rb @@ -7,21 +7,17 @@ class EditableCell < Component erb_template <<~ERB - <%= form_with(url: @check_editable_url, method: :get) do |form| %> - <%= form.hidden_field :id, value: @sample.id %> + <%= form_with(url: editable_namespace_project_sample_metadata_field_path(@sample.project.namespace.parent, @sample.project, @sample), method: :get) do |form| %> <%= form.hidden_field :field, value: @field %> - <%= form.hidden_field :value, value: @value %> <%= form.hidden_field :format, value: "turbo_stream" %> - <%= form.submit @value, class: "cursor-pointer p-4 hover:bg-slate-50 dark:hover:bg-slate-600 w-full text-left" %> + <%= form.submit @sample.metadata[@field], class: "cursor-pointer p-4 hover:bg-slate-50 dark:hover:bg-slate-600 w-full text-left" %> <% end %> ERB - def initialize(field:, sample:, check_editable_url:) + def initialize(field:, sample:) @sample = sample @field = field - @value = sample.metadata[field] - @check_editable_url = check_editable_url end end end diff --git a/app/components/samples/table_component.html.erb b/app/components/samples/table_component.html.erb index 72878b3077..c350534038 100644 --- a/app/components/samples/table_component.html.erb +++ b/app/components/samples/table_component.html.erb @@ -132,11 +132,7 @@ <% end %> <% end %> <% end %> - <%= render Samples::EditableCell.with_collection( - @metadata_fields, - sample: sample, - check_editable_url: @check_editable_url, - ) %> + <%= render Samples::EditableCell.with_collection(@metadata_fields, sample: sample) %> <% if @renders_row_actions %> <%= render_cell( tag: 'td', diff --git a/app/components/samples/table_component.rb b/app/components/samples/table_component.rb index b908fa2b90..ea42e0bde0 100644 --- a/app/components/samples/table_component.rb +++ b/app/components/samples/table_component.rb @@ -18,7 +18,6 @@ def initialize( search_params: {}, row_actions: {}, empty: {}, - check_editable_url: nil, **system_arguments ) @samples = samples @@ -30,7 +29,6 @@ def initialize( @search_params = search_params @row_actions = row_actions @empty = empty - @check_editable_url = check_editable_url @renders_row_actions = @row_actions.select { |_key, value| value }.count.positive? @system_arguments = system_arguments diff --git a/app/controllers/groups/samples_controller.rb b/app/controllers/groups/samples_controller.rb index d067ef071b..a827b35a4b 100644 --- a/app/controllers/groups/samples_controller.rb +++ b/app/controllers/groups/samples_controller.rb @@ -34,50 +34,6 @@ def select end end - def check_editable - @sample = Sample.find(params[:id]) - @field = params[:field] - @value = params[:value] - - if @sample.field_editable?(@field) - render turbo_stream: turbo_stream.replace( - helpers.dom_id(@sample, @field), - partial: 'edit_field_form', - locals: { sample: @sample, field: @field, value: @value } - ) - else - render turbo_stream: turbo_stream.append('flash', - partial: 'shared/alert', - locals: { message: 'This field is not editable' }) - end - end - - def update_field - @sample = Sample.find(params[:id]) - @project = Project.find(params[:project_id]) - @field = params[:field] - @value = params[:value] - @original_value = params[:original_value] - - updated_fields = ::Samples::Metadata::Fields::UpdateService.new(@project, @sample, current_user, - { - 'update_field' => { - 'key' => { - @field => @field - }, - 'value' => { - @original_value => @value - } - } - }).execute - - render turbo_stream: turbo_stream.replace( - helpers.dom_id(@sample, @field), - partial: 'editable_table_field', - locals: { sample: @sample, field: @field, value: @value } - ) - end - private def group diff --git a/app/controllers/projects/samples/metadata/fields_controller.rb b/app/controllers/projects/samples/metadata/fields_controller.rb index 81dcd376d6..408dd9b267 100644 --- a/app/controllers/projects/samples/metadata/fields_controller.rb +++ b/app/controllers/projects/samples/metadata/fields_controller.rb @@ -42,6 +42,60 @@ def update # rubocop:disable Metrics/AbcSize end end + def editable + authorize! @project, to: :update_sample? + @field = params[:field] + @value = @sample.metadata[@field] + if @sample.can_update_field?(params[:field]) + render turbo_stream: turbo_stream.replace( + helpers.dom_id(@sample, @field), + partial: 'shared/samples/metadata/fields/editing_field_cell', # TODO: Move this partial to the component + locals: { sample: @sample, field: @field, value: @value } + ) + else + render turbo_stream: turbo_stream.append('flashes', partial: 'shared/flash', + locals: { type: 'error', message: 'This field is not editable' }) + end + end + + def update_value + authorize! @project, to: :update_sample? + + @field = params[:field] + value = params[:value] + original_value = params[:original_value] + + if value == original_value + render turbo_stream: turbo_stream.replace( + helpers.dom_id(@sample, @field), + partial: 'shared/samples/metadata/fields/editable_field_cell', + locals: { sample: @sample, field: @field } + ) + else + ::Samples::Metadata::Fields::UpdateService.new(@project, @sample, current_user, + { + 'update_field' => { + 'key' => { + @field => @field + }, + 'value' => { + original_value => value + } + } + }).execute + if @sample.errors.any? + render status: :unprocessable_entity, + locals: { type: 'error', message: @sample.errors.full_messages.first } + else + render turbo_stream: turbo_stream.replace( + helpers.dom_id(@sample, @field), + partial: 'shared/samples/metadata/fields/editable_field_cell', + locals: { sample: @sample, field: @field } + ) + end + end + end + private def create_field_params diff --git a/app/controllers/projects/samples_controller.rb b/app/controllers/projects/samples_controller.rb index 29c5b8f393..b7cb6abc06 100644 --- a/app/controllers/projects/samples_controller.rb +++ b/app/controllers/projects/samples_controller.rb @@ -91,49 +91,6 @@ def select end end - def check_editable - @sample = Sample.find(params[:id]) - @field = params[:field] - @value = params[:value] - - if @sample.field_editable?(@field) - render turbo_stream: turbo_stream.replace( - helpers.dom_id(@sample, @field), - partial: 'edit_field_form', - locals: { sample: @sample, field: @field, value: @value } - ) - else - render turbo_stream: turbo_stream.append('flash', - partial: 'shared/alert', - locals: { message: 'This field is not editable' }) - end - end - - def update_field - @sample = Sample.find(params[:id]) - @field = params[:field] - @value = params[:value] - @original_value = params[:original_value] - - updated_fields = ::Samples::Metadata::Fields::UpdateService.new(@project, @sample, current_user, - { - 'update_field' => { - 'key' => { - @field => @field - }, - 'value' => { - @original_value => @value - } - } - }).execute - - render turbo_stream: turbo_stream.replace( - helpers.dom_id(@sample, @field), - partial: 'editable_table_field', - locals: { sample: @sample, field: @field, value: @value } - ) - end - private def sample diff --git a/app/javascript/controllers/inline_edit_controller.js b/app/javascript/controllers/inline_edit_controller.js index b88b55a990..b4c769a8dc 100644 --- a/app/javascript/controllers/inline_edit_controller.js +++ b/app/javascript/controllers/inline_edit_controller.js @@ -11,14 +11,18 @@ export default class extends Controller { this.inputTarget.select(); } - submit(event) { - this.element.requestSubmit(); + submit() { + if (!this.submitted) { + this.element.requestSubmit(); + } } cancel(event) { if (event.key === "Escape") { this.inputTarget.value = this.originalValue; this.inputTarget.blur(); + } else if (event.key === "Enter") { + this.submitted = true; } } } diff --git a/app/models/sample.rb b/app/models/sample.rb index c77b2ebe22..97968c4949 100644 --- a/app/models/sample.rb +++ b/app/models/sample.rb @@ -98,7 +98,7 @@ def search_data } end - def field_editable?(field) + def can_update_field?(field) metadata_provenance.key?(field) && metadata_provenance[field]['source'] == 'user' end diff --git a/app/views/groups/samples/_editable_table_field.html.erb b/app/views/groups/samples/_editable_table_field.html.erb deleted file mode 100644 index 26fce3ecde..0000000000 --- a/app/views/groups/samples/_editable_table_field.html.erb +++ /dev/null @@ -1,5 +0,0 @@ -<%= render Samples::EditableCell.new( - field: @field, - sample: @sample, - check_editable_url: check_editable_group_samples_path(@group), -) %> diff --git a/app/views/groups/samples/_table.html.erb b/app/views/groups/samples/_table.html.erb index 0591d0e058..d97389a312 100644 --- a/app/views/groups/samples/_table.html.erb +++ b/app/views/groups/samples/_table.html.erb @@ -15,5 +15,4 @@ title: t(:"groups.samples.table.no_samples"), description: t(:"groups.samples.table.no_associated_samples"), }, - check_editable_url: check_editable_group_samples_path(group), ) %> diff --git a/app/views/projects/samples/_edit_field_form.html.erb b/app/views/projects/samples/_edit_field_form.html.erb deleted file mode 100644 index 01e5576b48..0000000000 --- a/app/views/projects/samples/_edit_field_form.html.erb +++ /dev/null @@ -1,17 +0,0 @@ - - <%= form_with(url: update_field_namespace_project_samples_path(@project.namespace.parent, - @project), method: :patch, data: { controller: "inline-edit", "inline-edit-original-value": @value }) do |f| %> - <%= f.hidden_field :id, value: @sample.id %> - <%= f.hidden_field :field, value: @field %> - <%= f.hidden_field :original_value, value: @value %> - <%= f.hidden_field :format, value: "turbo_stream" %> - <%= f.text_field :value, - value: @value, - class: - "w-full m-0 border-slate-300 text-slate-900 text-sm focus:ring-primary-500 focus:border-primary-500 block w-full p-2.5 dark:bg-slate-700 dark:border-slate-600 dark:text-white dark:focus:ring-primary-500 dark:focus:border-primary-500", - data: { - "inline-edit-target": "input", - action: "blur->inline-edit#submit keydown->inline-edit#cancel", - } %> - <% end %> - diff --git a/app/views/projects/samples/_editable_table_field.html.erb b/app/views/projects/samples/_editable_table_field.html.erb deleted file mode 100644 index 52addea5d8..0000000000 --- a/app/views/projects/samples/_editable_table_field.html.erb +++ /dev/null @@ -1,9 +0,0 @@ -<%= render Samples::EditableCell.new( - field: @field, - sample: @sample, - check_editable_url: - check_editable_namespace_project_samples_path( - @sample.project.namespace.parent, - @sample.project, - ), -) %> diff --git a/app/views/projects/samples/_table.html.erb b/app/views/projects/samples/_table.html.erb index c071b6f662..1dc9438313 100644 --- a/app/views/projects/samples/_table.html.erb +++ b/app/views/projects/samples/_table.html.erb @@ -21,9 +21,4 @@ title: t(:"projects.samples.index.no_samples"), description: t(:"projects.samples.index.no_associated_samples"), }, - check_editable_url: - check_editable_namespace_project_samples_path( - project.namespace.parent, - project, - ), ) %> diff --git a/app/views/shared/_flash.html.erb b/app/views/shared/_flash.html.erb new file mode 100644 index 0000000000..83ebf6d09e --- /dev/null +++ b/app/views/shared/_flash.html.erb @@ -0,0 +1,3 @@ +<%= turbo_stream.prepend 'flashes' do %> + <%= viral_flash(type: type, data: message) %> +<% end %> diff --git a/app/views/shared/samples/metadata/fields/_editable_field_cell.html.erb b/app/views/shared/samples/metadata/fields/_editable_field_cell.html.erb new file mode 100644 index 0000000000..fcda66c0fe --- /dev/null +++ b/app/views/shared/samples/metadata/fields/_editable_field_cell.html.erb @@ -0,0 +1 @@ +<%= render Samples::EditableCell.new(field: @field, sample: @sample) %> diff --git a/app/views/groups/samples/_edit_field_form.html.erb b/app/views/shared/samples/metadata/fields/_editing_field_cell.html.erb similarity index 85% rename from app/views/groups/samples/_edit_field_form.html.erb rename to app/views/shared/samples/metadata/fields/_editing_field_cell.html.erb index 2d88014630..1518a0109d 100644 --- a/app/views/groups/samples/_edit_field_form.html.erb +++ b/app/views/shared/samples/metadata/fields/_editing_field_cell.html.erb @@ -1,8 +1,7 @@ - <%= form_with(url: update_field_group_samples_path(@group), method: :patch, + <%= form_with(url: update_value_namespace_project_sample_metadata_field_path(sample_id: @sample.id), method: :patch, data: { controller: "inline-edit", "inline-edit-original-value": @value }) do |f| %> <%= f.hidden_field :id, value: @sample.id %> - <%= f.hidden_field :project_id, value: @sample.project.id %> <%= f.hidden_field :field, value: @field %> <%= f.hidden_field :original_value, value: @value %> <%= f.hidden_field :format, value: "turbo_stream" %> diff --git a/config/routes/group.rb b/config/routes/group.rb index 008ab03b17..dfa60aab2b 100644 --- a/config/routes/group.rb +++ b/config/routes/group.rb @@ -38,8 +38,6 @@ collection do get :select post :search - get :check_editable - patch :update_field end end resources :subgroups, only: %i[index] diff --git a/config/routes/project.rb b/config/routes/project.rb index beaac6b946..3c5465798e 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -59,8 +59,6 @@ get :select post :list post :search - get :check_editable - patch :update_field end resources :attachments, module: :samples, only: %i[new create destroy] do scope module: :attachments, as: :attachments do @@ -76,7 +74,10 @@ resource :metadata, module: :samples, only: %i[new edit destroy] do scope module: :metadata, as: :metadata do collection do - resource :field, only: %i[update create] + resource :field, only: %i[update create] do + get :editable + patch :update_value + end end collection do resource :deletion, only: %i[new destroy] From 36311ccb364fcf7bd2c700624247f380edfa85d8 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Fri, 13 Dec 2024 13:06:40 -0600 Subject: [PATCH 15/29] fix: Update sample field test to assert button interactions for inline editing - Modified assertions in `samples_test.rb` to check for button elements instead of table cell text for sample metadata values. - Ensured that the test accurately reflects the current inline editing functionality by verifying the presence of buttons for sample values. --- test/system/projects/samples_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/system/projects/samples_test.rb b/test/system/projects/samples_test.rb index 7e71ed661e..7e341a2eed 100644 --- a/test/system/projects/samples_test.rb +++ b/test/system/projects/samples_test.rb @@ -827,8 +827,8 @@ class SamplesTest < ApplicationSystemTestCase assert_selector '#samples-table table thead tr th', count: 8 within('#samples-table table tbody tr:first-child') do assert_text @sample3.name - assert_selector 'td:nth-child(6)', text: 'value1' - assert_selector 'td:nth-child(7)', text: 'value2' + assert_selector 'td:nth-child(6) button', text: 'value1' + assert_selector 'td:nth-child(7) button', text: 'value2' end find('label', text: I18n.t('projects.samples.shared.metadata_toggle.label')).click assert_selector '#samples-table table thead tr th', count: 6 From ec4818a011e716e346b381f3182fa2304c4c5e84 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Fri, 13 Dec 2024 13:07:19 -0600 Subject: [PATCH 16/29] fix: Update sample field test to assert button presence for inline editing - Modified assertions in `samples_test.rb` to check for button elements instead of table cell text for sample metadata values. - Ensured the test accurately reflects the current inline editing functionality by verifying the presence of buttons for sample values. --- test/system/projects/samples_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/system/projects/samples_test.rb b/test/system/projects/samples_test.rb index 7e341a2eed..e8ae41ea4c 100644 --- a/test/system/projects/samples_test.rb +++ b/test/system/projects/samples_test.rb @@ -1557,8 +1557,8 @@ def retrieve_puids assert_selector '#samples-table table thead tr th', count: 8 within('#samples-table table tbody tr:first-child') do assert_text @sample3.name - assert_selector 'td:nth-child(6)', text: 'value1' - assert_selector 'td:nth-child(7)', text: 'value2' + assert_selector 'td:nth-child(6) button', text: 'value1' + assert_selector 'td:nth-child(7) button', text: 'value2' end end From 58040c9c5a41b5b8a6700c60bca0425a5a091c13 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Mon, 16 Dec 2024 06:51:39 -0600 Subject: [PATCH 17/29] refactor: Streamline field update logic and improve rendering methods --- .../samples/metadata/fields_controller.rb | 77 ++++++++++++------- 1 file changed, 51 insertions(+), 26 deletions(-) diff --git a/app/controllers/projects/samples/metadata/fields_controller.rb b/app/controllers/projects/samples/metadata/fields_controller.rb index 408dd9b267..e728162613 100644 --- a/app/controllers/projects/samples/metadata/fields_controller.rb +++ b/app/controllers/projects/samples/metadata/fields_controller.rb @@ -66,33 +66,9 @@ def update_value original_value = params[:original_value] if value == original_value - render turbo_stream: turbo_stream.replace( - helpers.dom_id(@sample, @field), - partial: 'shared/samples/metadata/fields/editable_field_cell', - locals: { sample: @sample, field: @field } - ) + render_unchanged_field else - ::Samples::Metadata::Fields::UpdateService.new(@project, @sample, current_user, - { - 'update_field' => { - 'key' => { - @field => @field - }, - 'value' => { - original_value => value - } - } - }).execute - if @sample.errors.any? - render status: :unprocessable_entity, - locals: { type: 'error', message: @sample.errors.full_messages.first } - else - render turbo_stream: turbo_stream.replace( - helpers.dom_id(@sample, @field), - partial: 'shared/samples/metadata/fields/editable_field_cell', - locals: { sample: @sample, field: @field } - ) - end + update_field_value(original_value, value) end end @@ -153,6 +129,55 @@ def get_update_status_and_message(updated_metadata_field) end update_render_params end + + def render_unchanged_field + render turbo_stream: turbo_stream.replace( + helpers.dom_id(@sample, @field), + partial: 'shared/samples/metadata/fields/editable_field_cell', + locals: { sample: @sample, field: @field } + ) + end + + def update_field_value(original_value, new_value) + perform_field_update(original_value, new_value) + + if @sample.errors.any? + render_update_error + else + render_update_success + end + end + + def perform_field_update(original_value, new_value) + ::Samples::Metadata::Fields::UpdateService.new( + @project, + @sample, + current_user, + build_update_params(original_value, new_value) + ).execute + end + + def build_update_params(original_value, new_value) + { + 'update_field' => { + 'key' => { @field => @field }, + 'value' => { original_value => new_value } + } + } + end + + def render_update_error + render status: :unprocessable_entity, + locals: { type: 'error', message: @sample.errors.full_messages.first } + end + + def render_update_success + render turbo_stream: turbo_stream.replace( + helpers.dom_id(@sample, @field), + partial: 'shared/samples/metadata/fields/editable_field_cell', + locals: { sample: @sample, field: @field } + ) + end end end end From a940d709a54f3f99fc143c08e5682d0679b078d1 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Mon, 16 Dec 2024 11:49:46 -0600 Subject: [PATCH 18/29] refactor: Remove unused hidden field for sample ID in editing field cell --- .../shared/samples/metadata/fields/_editing_field_cell.html.erb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/views/shared/samples/metadata/fields/_editing_field_cell.html.erb b/app/views/shared/samples/metadata/fields/_editing_field_cell.html.erb index 1518a0109d..f051eaba6e 100644 --- a/app/views/shared/samples/metadata/fields/_editing_field_cell.html.erb +++ b/app/views/shared/samples/metadata/fields/_editing_field_cell.html.erb @@ -1,7 +1,6 @@ <%= form_with(url: update_value_namespace_project_sample_metadata_field_path(sample_id: @sample.id), method: :patch, data: { controller: "inline-edit", "inline-edit-original-value": @value }) do |f| %> - <%= f.hidden_field :id, value: @sample.id %> <%= f.hidden_field :field, value: @field %> <%= f.hidden_field :original_value, value: @value %> <%= f.hidden_field :format, value: "turbo_stream" %> From 289c1577d51d6e106a7bd5ff34a7995dff30cd2b Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Mon, 16 Dec 2024 11:51:31 -0600 Subject: [PATCH 19/29] test: Add tests for unchanged field rendering and sample metadata updates --- .../metadata/fields/fields_controller_test.rb | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb b/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb index 7c73e357f3..b8382b512f 100644 --- a/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb +++ b/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb @@ -156,6 +156,28 @@ class FieldsControllerTest < ActionDispatch::IntegrationTest } }, format: :turbo_stream } assert_response :unprocessable_entity end + + test 'renders unchanged field when value does not change' do + patch namespace_project_sample_metadata_field_path(@namespace, @project29, @sample32), + params: { 'sample' => { 'update_field' => { + 'key' => { 'metadatafield1' => 'metadatafield1' }, + 'value' => { 'value1' => 'value1' } + } }, format: :turbo_stream } + assert_response :unprocessable_entity + end + + test 'updates sample metadata with new value' do + patch update_value_namespace_project_sample_metadata_field_path( + @namespace, @project29, @sample32 + ), + params: { + 'field' => 'metadatafield1', + 'value' => 'new_value', + 'original_value' => 'old_value', + 'format' => :turbo_stream + } + assert_response :ok + end end end end From 776a33b68310db0bd1868696a48f0e009abe540b Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Mon, 16 Dec 2024 14:08:41 -0600 Subject: [PATCH 20/29] test: Add test for editing a metadata field and update unchanged field response --- .../metadata/fields/fields_controller_test.rb | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb b/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb index b8382b512f..57c4301f66 100644 --- a/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb +++ b/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb @@ -157,13 +157,27 @@ class FieldsControllerTest < ActionDispatch::IntegrationTest assert_response :unprocessable_entity end + test 'check to edit a metadata field' do + get editable_namespace_project_sample_metadata_field_path( + @namespace, @project29, @sample32 + ), params: { + 'field' => 'metadatafield1', + 'format' => :turbo_stream + } + assert_response :ok + end + test 'renders unchanged field when value does not change' do - patch namespace_project_sample_metadata_field_path(@namespace, @project29, @sample32), - params: { 'sample' => { 'update_field' => { - 'key' => { 'metadatafield1' => 'metadatafield1' }, - 'value' => { 'value1' => 'value1' } - } }, format: :turbo_stream } - assert_response :unprocessable_entity + patch update_value_namespace_project_sample_metadata_field_path( + @namespace, @project29, @sample32 + ), + params: { + 'field' => 'metadatafield1', + 'value' => 'old_value', + 'original_value' => 'old_value', + 'format' => :turbo_stream + } + assert_response :ok end test 'updates sample metadata with new value' do From 8351bb997fb41d5fd829747d8bd875cbc1893654 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Mon, 16 Dec 2024 14:38:26 -0600 Subject: [PATCH 21/29] fix: Update response status for editable field checks and add test for analysis field restriction --- .../samples/metadata/fields_controller.rb | 8 ++++---- .../metadata/fields/fields_controller_test.rb | 15 ++++++++++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/app/controllers/projects/samples/metadata/fields_controller.rb b/app/controllers/projects/samples/metadata/fields_controller.rb index e728162613..a3f33bc4f4 100644 --- a/app/controllers/projects/samples/metadata/fields_controller.rb +++ b/app/controllers/projects/samples/metadata/fields_controller.rb @@ -47,14 +47,14 @@ def editable @field = params[:field] @value = @sample.metadata[@field] if @sample.can_update_field?(params[:field]) - render turbo_stream: turbo_stream.replace( + render status: :partial_content, turbo_stream: turbo_stream.replace( helpers.dom_id(@sample, @field), - partial: 'shared/samples/metadata/fields/editing_field_cell', # TODO: Move this partial to the component + partial: 'shared/samples/metadata/fields/editing_field_cell', locals: { sample: @sample, field: @field, value: @value } ) else - render turbo_stream: turbo_stream.append('flashes', partial: 'shared/flash', - locals: { type: 'error', message: 'This field is not editable' }) + render status: :unprocessable_entity, turbo_stream: turbo_stream.append('flashes', partial: 'shared/flash', + locals: { type: 'error', message: 'This field is not editable' }) end end diff --git a/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb b/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb index 57c4301f66..95a031e1ad 100644 --- a/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb +++ b/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb @@ -164,7 +164,20 @@ class FieldsControllerTest < ActionDispatch::IntegrationTest 'field' => 'metadatafield1', 'format' => :turbo_stream } - assert_response :ok + assert_response :partial_content + end + + test 'checking to see if a field can be edited fails if it belongs to an analysis' do + sample34 = samples(:sample34) + project31 = projects(:project31) + namespace = groups(:subgroup_twelve_a_a) + get editable_namespace_project_sample_metadata_field_path( + namespace, project31, sample34 + ), params: { + 'field' => 'metadatafield1', + 'format' => :turbo_stream + } + assert_response :unprocessable_entity end test 'renders unchanged field when value does not change' do From ea3fbfc04354ed617f9446b6d25ff08624b7fdee Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Mon, 16 Dec 2024 14:40:49 -0600 Subject: [PATCH 22/29] test: Add test for building update parameters in FieldsController --- .../metadata/fields/fields_controller_test.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb b/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb index 95a031e1ad..1b6f4b8ef0 100644 --- a/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb +++ b/test/controllers/projects/samples/metadata/fields/fields_controller_test.rb @@ -180,6 +180,21 @@ class FieldsControllerTest < ActionDispatch::IntegrationTest assert_response :unprocessable_entity end + test 'builds correct update params for updating a value' do + controller = FieldsController.new + controller.instance_variable_set(:@field, @field) + + expected_params = { + 'update_field' => { + 'key' => { @field => @field }, + 'value' => { 'old_value' => 'new_value' } + } + } + + assert_equal expected_params, + controller.send(:build_update_params, 'old_value', 'new_value') + end + test 'renders unchanged field when value does not change' do patch update_value_namespace_project_sample_metadata_field_path( @namespace, @project29, @sample32 From 19ce375da68dba969f28114a53b5e6e4229b7d56 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Mon, 16 Dec 2024 15:12:34 -0600 Subject: [PATCH 23/29] test: Update sample button selectors to use input[type="submit"] --- test/system/projects/samples_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/system/projects/samples_test.rb b/test/system/projects/samples_test.rb index e8ae41ea4c..3bd05a165f 100644 --- a/test/system/projects/samples_test.rb +++ b/test/system/projects/samples_test.rb @@ -827,8 +827,8 @@ class SamplesTest < ApplicationSystemTestCase assert_selector '#samples-table table thead tr th', count: 8 within('#samples-table table tbody tr:first-child') do assert_text @sample3.name - assert_selector 'td:nth-child(6) button', text: 'value1' - assert_selector 'td:nth-child(7) button', text: 'value2' + assert_selector 'td input[type="submit"][value="value1"]' + assert_selector 'td input[type="submit"][value="value2"]' end find('label', text: I18n.t('projects.samples.shared.metadata_toggle.label')).click assert_selector '#samples-table table thead tr th', count: 6 From 0b9033ee2e43c85c795bb1d5d0a12398b1a2ff9a Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Mon, 16 Dec 2024 21:24:07 -0600 Subject: [PATCH 24/29] test: Update sample table assertions to check input values instead of text --- test/system/groups/samples_test.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/system/groups/samples_test.rb b/test/system/groups/samples_test.rb index 5eda210ec4..ebe03a9c49 100644 --- a/test/system/groups/samples_test.rb +++ b/test/system/groups/samples_test.rb @@ -391,16 +391,16 @@ def retrieve_puids assert_selector 'table thead tr th', count: 9 within('table tbody tr:first-child') do assert_text @sample30.name - assert_selector 'td:nth-child(7)', text: 'value1' - assert_selector 'td:nth-child(8)', text: 'value2' - assert_selector 'td:nth-child(9)', text: '' + assert_selector 'td:nth-child(7) input[value="value1"]' + assert_selector 'td:nth-child(8) input[value="value2"]' + assert_selector 'td:nth-child(9) input[type="submit"]' end within('table tbody tr:nth-child(3)') do assert_text @sample28.name - assert_selector 'td:nth-child(7)', text: '' - assert_selector 'td:nth-child(8)', text: '' - assert_selector 'td:nth-child(9)', text: 'unique_value' + assert_selector 'td:nth-child(7) input[type="submit"]' + assert_selector 'td:nth-child(8) input[type="submit"]' + assert_selector 'td:nth-child(9) input[value="unique_value"]' end find('label', text: I18n.t('projects.samples.shared.metadata_toggle.label')).click From 1d4a26c39d338e0912a5fbda95631f6b1d1a357f Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Mon, 16 Dec 2024 21:54:08 -0600 Subject: [PATCH 25/29] test: Add test for updating metadata value not from an analysis --- test/system/groups/samples_test.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/system/groups/samples_test.rb b/test/system/groups/samples_test.rb index ebe03a9c49..c4358dcca3 100644 --- a/test/system/groups/samples_test.rb +++ b/test/system/groups/samples_test.rb @@ -767,5 +767,28 @@ def retrieve_puids click_on I18n.t('shared.samples.metadata.file_imports.errors.ok_button') end end + + test 'can update metadata value that is not from an analysis' do + visit group_samples_url(@group) + assert_selector 'table thead tr th', count: 6 + + click_on 'Last Updated' + assert_selector 'table thead th:nth-child(5) svg.icon-arrow_up' + + assert_selector 'label', text: I18n.t('projects.samples.shared.metadata_toggle.label'), count: 1 + find('label', text: I18n.t('projects.samples.shared.metadata_toggle.label')).click + + within 'div.overflow-auto.scrollbar' do |div| + div.scroll_to div.find('table thead th:nth-child(7)') + end + + assert_selector 'table thead tr th', count: 9 + within('table tbody tr:first-child') do + assert_text @sample30.name + assert_selector 'td:nth-child(7) input[type="submit"][value="value1"]' + click_button 'input[type="submit"][value="value1"]' + assert_selector 'input[type="text"][value="value1"]' + end + end end end From 74b7cf6a4feaffa48fc8d81020f0cebd5f86e249 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Tue, 17 Dec 2024 11:42:43 -0600 Subject: [PATCH 26/29] feat: Enhance metadata field handling and submission logic --- app/components/samples/editable_cell.rb | 2 +- .../projects/samples/metadata/fields_controller.rb | 14 +++++++++++++- app/models/sample.rb | 6 +++++- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/app/components/samples/editable_cell.rb b/app/components/samples/editable_cell.rb index e1a2a29e1c..642a866700 100644 --- a/app/components/samples/editable_cell.rb +++ b/app/components/samples/editable_cell.rb @@ -10,7 +10,7 @@ class EditableCell < Component <%= form_with(url: editable_namespace_project_sample_metadata_field_path(@sample.project.namespace.parent, @sample.project, @sample), method: :get) do |form| %> <%= form.hidden_field :field, value: @field %> <%= form.hidden_field :format, value: "turbo_stream" %> - <%= form.submit @sample.metadata[@field], class: "cursor-pointer p-4 hover:bg-slate-50 dark:hover:bg-slate-600 w-full text-left" %> + <%= form.submit @sample.metadata[@field] || "", class: "cursor-pointer p-4 hover:bg-slate-50 dark:hover:bg-slate-600 w-full text-left" %> <% end %> ERB diff --git a/app/controllers/projects/samples/metadata/fields_controller.rb b/app/controllers/projects/samples/metadata/fields_controller.rb index a3f33bc4f4..c81b566e3d 100644 --- a/app/controllers/projects/samples/metadata/fields_controller.rb +++ b/app/controllers/projects/samples/metadata/fields_controller.rb @@ -46,7 +46,7 @@ def editable authorize! @project, to: :update_sample? @field = params[:field] @value = @sample.metadata[@field] - if @sample.can_update_field?(params[:field]) + if !@sample.field?(@field) || @sample.updatable_field?(params[:field]) render status: :partial_content, turbo_stream: turbo_stream.replace( helpers.dom_id(@sample, @field), partial: 'shared/samples/metadata/fields/editing_field_cell', @@ -67,6 +67,8 @@ def update_value if value == original_value render_unchanged_field + elsif original_value.blank? + create_metadata_field(@field, value) else update_field_value(original_value, value) end @@ -138,6 +140,16 @@ def render_unchanged_field ) end + def create_metadata_field(field, value) + ::Samples::Metadata::Fields::CreateService.new(@project, @sample, current_user, { field => value }).execute + + if @sample.errors.any? + render_update_error + else + render_update_success + end + end + def update_field_value(original_value, new_value) perform_field_update(original_value, new_value) diff --git a/app/models/sample.rb b/app/models/sample.rb index 97968c4949..95ef7f47ca 100644 --- a/app/models/sample.rb +++ b/app/models/sample.rb @@ -98,7 +98,11 @@ def search_data } end - def can_update_field?(field) + def field?(field) + metadata.key?(field) + end + + def updatable_field?(field) metadata_provenance.key?(field) && metadata_provenance[field]['source'] == 'user' end From a12b156a143dfeb694b0828fd4f1569583c0c444 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Tue, 17 Dec 2024 14:47:58 -0600 Subject: [PATCH 27/29] test: Refactor sample table interaction and update value submission logic --- test/system/groups/samples_test.rb | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/test/system/groups/samples_test.rb b/test/system/groups/samples_test.rb index c4358dcca3..cf424e0df1 100644 --- a/test/system/groups/samples_test.rb +++ b/test/system/groups/samples_test.rb @@ -772,9 +772,6 @@ def retrieve_puids visit group_samples_url(@group) assert_selector 'table thead tr th', count: 6 - click_on 'Last Updated' - assert_selector 'table thead th:nth-child(5) svg.icon-arrow_up' - assert_selector 'label', text: I18n.t('projects.samples.shared.metadata_toggle.label'), count: 1 find('label', text: I18n.t('projects.samples.shared.metadata_toggle.label')).click @@ -783,11 +780,20 @@ def retrieve_puids end assert_selector 'table thead tr th', count: 9 - within('table tbody tr:first-child') do - assert_text @sample30.name - assert_selector 'td:nth-child(7) input[type="submit"][value="value1"]' - click_button 'input[type="submit"][value="value1"]' - assert_selector 'input[type="text"][value="value1"]' + within('table tbody tr:first-child td:nth-child(7)') do + # check within the from with method get that the value is 'value1': + within('form[method="get"]') do + find("input[type='submit']").click + end + assert_selector "form[data-controller='inline-edit']" + + within('form[data-controller="inline-edit"]') do + find('input[name="value"]').send_keys 'value2' + find('input[name="value"]').send_keys :return + end + assert_no_selector "form[data-controller='inline-edit']" + assert_selector 'form[method="get"]' + assert_selector 'input[value="value2"]' end end end From e1d218e7ad0edd743bb43067d6a35b6ca18d13e8 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Tue, 17 Dec 2024 14:58:40 -0600 Subject: [PATCH 28/29] test: Add check to prevent updating metadata value from an analysis --- test/system/groups/samples_test.rb | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/system/groups/samples_test.rb b/test/system/groups/samples_test.rb index cf424e0df1..2367d9952b 100644 --- a/test/system/groups/samples_test.rb +++ b/test/system/groups/samples_test.rb @@ -796,5 +796,28 @@ def retrieve_puids assert_selector 'input[value="value2"]' end end + + test 'should not update metadata value that is from an analysis' do + visit group_samples_url(@group) + assert_selector 'table thead tr th', count: 6 + + click_on 'Last Updated' + assert_selector 'table thead th:nth-child(5) svg.icon-arrow_up' + + assert_selector 'label', text: I18n.t('projects.samples.shared.metadata_toggle.label'), count: 1 + find('label', text: I18n.t('projects.samples.shared.metadata_toggle.label')).click + + within 'div.overflow-auto.scrollbar' do |div| + div.scroll_to div.find('table thead th:nth-child(7)') + end + + assert_selector 'table thead tr th', count: 9 + within('table tbody tr:nth-child(3) td:nth-child(7)') do + within('form[method="get"]') do + find("input[type='submit']").click + end + assert_no_selector "form[data-controller='inline-edit']" + end + end end end From 203ac1d860342d38da9963a30e3de6b2407fb647 Mon Sep 17 00:00:00 2001 From: Josh Adam Date: Tue, 17 Dec 2024 15:50:44 -0600 Subject: [PATCH 29/29] test: Update metadata value handling in sample tests --- test/system/projects/samples_test.rb | 52 ++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/test/system/projects/samples_test.rb b/test/system/projects/samples_test.rb index 3bd05a165f..a024e53609 100644 --- a/test/system/projects/samples_test.rb +++ b/test/system/projects/samples_test.rb @@ -1548,17 +1548,55 @@ def retrieve_puids end end - test 'can inline edit a sample metadata field' do + test 'can update metadata value that is not from an analysis' do visit namespace_project_samples_url(@namespace, @project) + assert_selector 'table thead tr th', count: 6 - assert_selector 'label', text: I18n.t(:'projects.samples.shared.metadata_toggle.label'), count: 1 + assert_selector 'label', text: I18n.t('projects.samples.shared.metadata_toggle.label'), count: 1 find('label', text: I18n.t('projects.samples.shared.metadata_toggle.label')).click - assert_selector '#samples-table table thead tr th', count: 8 - within('#samples-table table tbody tr:first-child') do - assert_text @sample3.name - assert_selector 'td:nth-child(6) button', text: 'value1' - assert_selector 'td:nth-child(7) button', text: 'value2' + within 'div.overflow-auto.scrollbar' do |div| + div.scroll_to div.find('table thead th:nth-child(7)') + end + + assert_selector 'table thead tr th', count: 8 + within('table tbody tr:first-child td:nth-child(7)') do + # check within the from with method get that the value is 'value1': + within('form[method="get"]') do + find("input[type='submit']").click + end + assert_selector "form[data-controller='inline-edit']" + + within('form[data-controller="inline-edit"]') do + find('input[name="value"]').send_keys 'value2' + find('input[name="value"]').send_keys :return + end + assert_no_selector "form[data-controller='inline-edit']" + assert_selector 'form[method="get"]' + assert_selector 'input[value="value2"]' + end + end + + test 'should not update metadata value that is from an analysis' do + visit namespace_project_samples_url(@namespace, @project) + assert_selector 'table thead tr th', count: 6 + + click_on I18n.t('projects.samples.show.table_header.last_updated') + assert_selector 'table thead th:nth-child(4) svg.icon-arrow_up' + + assert_selector 'label', text: I18n.t('projects.samples.shared.metadata_toggle.label'), count: 1 + find('label', text: I18n.t('projects.samples.shared.metadata_toggle.label')).click + + within 'div.overflow-auto.scrollbar' do |div| + div.scroll_to div.find('table thead th:nth-child(8)') + end + + assert_selector 'table thead tr th', count: 8 + within('table tbody tr:nth-child(1) td:nth-child(5)') do + within('form[method="get"]') do + find("input[type='submit']").click + end + assert_no_selector "form[data-controller='inline-edit']" end end