Enhancement: Edit line list metadata in place (#864)

* feat: ✨ Initial work on editing line list

* feat: Add check_editable route to project for enhanced editing capabilities

* 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.

* 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.

* 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.

* 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.

* 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.

* 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.

* 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.

* 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.

* 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.

* 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.

* 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.

* 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.

* 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.

* refactor: Streamline field update logic and improve rendering methods

* refactor: Remove unused hidden field for sample ID in editing field cell

* test: Add tests for unchanged field rendering and sample metadata updates

* test: Add test for editing a metadata field and update unchanged field response

* fix: Update response status for editable field checks and add test for analysis field restriction

* test: Add test for building update parameters in FieldsController

* test: Update sample table assertions to check input values instead of text

* test: Add test for updating metadata value not from an analysis

* feat: Enhance metadata field handling and submission logic

* test: Refactor sample table interaction and update value submission logic

* test: Add check to prevent updating metadata value from an analysis

* test: Update metadata value handling in sample tests

* test: Update table cell index in sample tests for accurate element selection

* feat: Add success flash message upon successful field update in metadata

* feat: Add autofocus option to editable cell for improved user experience

* feat: Refactor editable cell to use button for submission and improve accessibility

* feat: Improve button accessibility by updating aria-label and maintaining autofocus option

* refactor: Simplify field rendering logic and improve error handling in fields controller

* fix: Update error message to display specific validation error for non-editable fields

* feat: Enhance editable field handling by adding non-editable field error message and create metadata field functionality

* feat: Add success message for metadata updates and refactor button interactions in sample fields

* feat: Implement editable cell component with button interactions and update tests for button selectors

* fix: Remove unnecessary class from editable cell and adjust z-index for sticky table headers

* test: Refactor sample tests to include setup, actions, and verification sections for clarity

* test: Refactor sample tests to include setup, actions, and verification sections for better clarity

* test: Add new test to ensure project analysts cannot edit samples, enhancing access control verification

* feat: Enhance sample table rendering by conditionally displaying editable cells based on user permissions, improving user experience and access control

* refactor: Simplify editable cell initialization and update table rendering logic to enhance user experience by conditionally rendering editable cells based on row actions

* test: Test to ensure need permissions to edit metadata

* test: Analysts should not be able to edit metadata

* test: Remove redundant label click assertion in sample metadata tests to streamline test flow

* refactor: Clean up HTML class attributes in table component for improved readability

* test: Remove outdated test for metadata update restriction to streamline test suite

* fix: Update metadata access methods to use field? for consistency

* test: Remove unnecessary assertions in sample metadata tests to streamline test flow

* fix: Update sample since the other one went away :(

* test: Simplify sample metadata toggle interaction in system tests

* test: Add assertion for the number of table headers in sample metadata toggle

* refactor: Disable RuboCop class length metric for FieldsController

* test: Add assertion for updated table header count in sample metadata toggle

* test: Update sample metadata toggle test to use specific project and subgroup

* test: Add assertion for the presence of table headers in sample metadata toggle

* fix: Use fetch method for safer metadata access in metadata_value

* fix: Use fetch method for safer access to sample metadata in editable_cell

* fix: Improve updatable_field? method to handle missing metadata provenance

* fix: Simplify condition for rendering editable fields in fields_controller

* test: Add unit tests for field? and updatable_field? methods in Sample model

* test: Update sample tests to assert table header count after metadata toggle

* feat: Refactor timestamp handling in sample forms to use a shared partial

* fix: Update timestamp handling in fields_controller and timestamp_input partial

* fix: Adjust timestamp handling in fields_controller to prevent timing issues

* fix: Update timestamp assignment in fields_controller to use sample's updated_at

app/components/nextflow/samplesheet/metadata_cell_component.rb

@@ -15,7 +15,7 @@ def initialize(form:, name:, sample:, field: nil, required: false)
       end
 
       def metadata_value(name)
-        sample.metadata.key?(name) ? sample.metadata[name] : ''
+        sample.metadata.fetch(name, '')
       end
     end
   end

app/components/samples/editable_cell.html.erb

@@ -0,0 +1,29 @@
+<td id="<%= dom_id(@sample, @field) %>" role="gridcell">
+  <%= form_with(
+              url: editable_namespace_project_sample_metadata_field_path(
+                @sample.project.namespace.parent,
+                @sample.project,
+                @sample
+              ),
+              method: :get,
+              class: "w-full"
+            ) do |form| %>
+
+    <%= form.hidden_field :field, value: @field %>
+    <%= form.hidden_field :format, value: "turbo_stream" %>
+
+    <button
+      type="submit"
+      class="
+        w-full p-4 text-left cursor-pointer hover:bg-slate-50 focus:outline-none
+        dark:hover:bg-slate-600
+      "
+      aria-label="<%= t(:".aria_label", value: @sample.metadata[@field]) %> value"
+      <%= "autofocus" if @autofocus %>
+    >
+      <span class="block truncate">
+        <%= @sample.metadata.fetch(@field, "") %>
+      </span>
+    </button>
+  <% end %>
+</td>

app/components/samples/editable_cell.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module Samples
+  # Component for rendering an editable cell
+  class EditableCell < Component
+    def initialize(field:, sample:, autofocus: false)
+      @sample = sample
+      @field = field
+      @autofocus = autofocus
+    end
+  end
+end

app/components/samples/table_component.html.erb

@@ -2,16 +2,10 @@
   <%= render Viral::BaseComponent.new(**wrapper_arguments) do %>
     <%= render Viral::BaseComponent.new(**system_arguments) do %>
       <table
-        class='
-          w-full text-sm text-left rtl:text-right text-slate-500 dark:text-slate-400
-          whitespace-nowrap
-        '
+        class='w-full text-sm text-left  rtl:text-right text-slate-500 dark:text-slate-400 whitespace-nowrap'
       >
         <thead
-          class='
-            sticky top-0 z-10 text-xs uppercase text-slate-700 bg-slate-50 dark:bg-slate-700
-            dark:text-slate-300
-          '
+          class='sticky top-0 z-10 text-xs uppercase  text-slate-700 bg-slate-50 dark:bg-slate-700 dark:text-slate-300'
         >
           <tr>
             <% @columns.each_with_index do |column, index| %>
@@ -73,18 +67,15 @@
           </tr>
         </thead>
         <tbody
-          class='
-            overflow-y-auto bg-white divide-y divide-slate-200 dark:bg-slate-800
-            dark:divide-slate-700
-          '
+          class='overflow-y-auto bg-white divide-y  divide-slate-200 dark:bg-slate-800 dark:divide-slate-700'
         >
           <% @samples.each do |sample| %>
             <%= render Viral::BaseComponent.new(**row_arguments(sample)) do %>
               <% @columns.each_with_index do |column, index| %>
                 <%= render_cell(
                 tag: index.zero? ? 'th' :'td',
                 scope: index.zero? ? 'row' : nil,
-                classes: class_names('px-3 py-3', 'sticky left-0 min-w-56 max-w-56 bg-slate-50 dark:bg-slate-700': index.zero?, 'sticky left-56 bg-slate-50 dark:bg-slate-700': index == 1)
+                classes: class_names('px-3 py-3', 'sticky left-0 z-5 min-w-56 max-w-56 bg-slate-50 dark:bg-slate-700': index.zero?, 'sticky z-5 left-56 bg-slate-50 dark:bg-slate-700': index == 1)
               ) do %>
                   <% if index.zero? && @abilities[:select_samples] %>
                     <%= check_box_tag "sample_ids[]",
@@ -133,13 +124,18 @@
                 <% end %>
               <% end %>
               <% @metadata_fields.each do |field| %>
-                <%= render_cell(
+                <% if helpers.allowed_to?(:update_sample?, sample.project) %>
+                  <%= render Samples::EditableCell.new(field: field, sample: sample) %>
+                <% else %>
+                  <%= render_cell(
                 tag: 'td',
                 classes: class_names('px-3 py-3')
               ) do %>
-                  <%= sample.metadata[field] %>
+                    <%= sample.metadata[field] %>
+                  <% end %>
                 <% end %>
               <% end %>
+
               <% if @renders_row_actions %>
                 <%= render_cell(
                 tag: 'td',
@@ -183,10 +179,7 @@
           <tfoot class="sticky bottom-0 z-10">
             <tr class="border-t dark:border-slate-700 border-slate-200">
               <td
-                class="
-                  sticky left-0 z-10 px-6 py-3 text-slate-700 dark:text-slate-300 bg-slate-50
-                  dark:bg-slate-700
-                "
+                class="sticky left-0 z-10 px-6 py-3  text-slate-700 dark:text-slate-300 bg-slate-50 dark:bg-slate-700"
                 colspan="3"
               >
                 <span>
@@ -203,7 +196,7 @@
                   >0</strong>
                 </span>
               </td>
-              <td colspan="100%" class="px-3 py-3 bg-slate-50 dark:bg-slate-700"></td>
+              <td colspan="100%" class="p-3 bg-slate-50 dark:bg-slate-700"></td>
             </tr>
           </tfoot>
         <% end %>

app/controllers/projects/samples/metadata/fields_controller.rb

@@ -4,7 +4,7 @@ module Projects
   module Samples
     module Metadata
       # Controller actions for Project Samples Metadata Fields Controller
-      class FieldsController < Projects::Samples::ApplicationController
+      class FieldsController < Projects::Samples::ApplicationController # rubocop:disable Metrics/ClassLength
         respond_to :turbo_stream
 
         # Param received as:
@@ -42,6 +42,34 @@ def update # rubocop:disable Metrics/AbcSize
           end
         end
 
+        def editable
+          authorize! @project, to: :update_sample?
+          @field = params[:field]
+          @value = @sample.metadata[@field]
+
+          if @sample.updatable_field?(@field)
+            render_editable_field
+          else
+            render_non_editable_error
+          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_unchanged_field
+          elsif !@sample.field?(@field)
+            create_metadata_field(@field, value)
+          else
+            update_field_value(original_value, value)
+          end
+        end
+
         private
 
         def create_field_params
@@ -62,6 +90,22 @@ def get_create_status(added_keys, existing_keys)
           end
         end
 
+        def render_editable_field
+          render status: :partial_content, turbo_stream: turbo_stream.replace(
+            helpers.dom_id(@sample, @field),
+            partial: 'shared/samples/metadata/fields/editing_field_cell',
+            locals: { sample: @sample, field: @field, value: @value }
+          )
+        end
+
+        def render_non_editable_error
+          render status: :unprocessable_entity, turbo_stream: turbo_stream.append(
+            'flashes',
+            partial: 'shared/flash',
+            locals: { type: 'error', message: t('samples.editable_cell.not_editable', field: @field) }
+          )
+        end
+
         def get_create_messages(added_keys, existing_keys) # rubocop:disable Metrics/MethodLength
           messages = []
           if added_keys.count == 1
@@ -99,6 +143,77 @@ def get_update_status_and_message(updated_metadata_field)
           end
           update_render_params
         end
+
+        def create_metadata_field(field, value)
+          create_params = { field => value }
+          ::Samples::Metadata::Fields::CreateService.new(@project, @sample, current_user, create_params).execute
+
+          if @sample.errors.any?
+            render_update_error
+          else
+            render_update_success
+          end
+        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
+          # When the timestamp is rendered to the form, it only renders down to the second, this was causing timing
+          # issues for selecting current samples.  To fix this, we are adding a second to the timestamp so that the
+          # timestamp is always greater than the current time.
+          @timestamp = @sample.updated_at + 1.second
+          render turbo_stream: [turbo_stream.replace(
+            helpers.dom_id(@sample, @field),
+            partial: 'shared/samples/metadata/fields/editable_field_cell',
+            locals: { sample: @sample, field: @field }
+          ),
+                                turbo_stream.append(
+                                  'flashes',
+                                  partial: 'shared/flash',
+                                  locals: { type: 'success',
+                                            message: t('samples.editable_cell.update_success') }
+                                ),
+                                turbo_stream.replace('timestamp', partial: 'shared/samples/timestamp_input')]
+        end
       end
     end
   end

app/javascript/controllers/inline_edit_controller.js

@@ -0,0 +1,28 @@
+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() {
+    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;
+    }
+  }
+}

app/models/sample.rb

@@ -144,6 +144,16 @@ def search_data
     }.compact
   end
 
+  def field?(field)
+    metadata.key?(field)
+  end
+
+  def updatable_field?(field)
+    return true unless metadata_provenance.key?(field)
+
+    metadata_provenance[field]['source'] == 'user'
+  end
+
   def should_index?
     !deleted?
   end

app/services/samples/metadata/update_service.rb

@@ -114,14 +114,14 @@ def add_metadata_to_sample(key, value)
 
       def get_metadata_change_status(key, value) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
         if value.blank?
-          :deleted if @sample.metadata.key?(key)
+          :deleted if @sample.field?(key)
         elsif @sample.metadata_provenance.key?(key) && @analysis_id.nil? &&
               @sample.metadata_provenance[key]['source'] == 'analysis'
           :not_updated
-        elsif @sample.metadata.key?(key) && @sample.metadata[key] == value
+        elsif @sample.field?(key) && @sample.metadata[key] == value
           :unchanged
         else
-          @sample.metadata.key?(key) ? :updated : :added
+          @sample.field?(key) ? :updated : :added
         end
       end
 

app/views/groups/samples/index.html.erb

@@ -72,12 +72,7 @@
                 ) do |f| %>
             <input type="hidden" name="format" value="turbo_stream"/>
             <input type="hidden" name="select" value="on"/>
-            <input
-              type="hidden"
-              name="timestamp"
-              value="<%=@timestamp%>"
-              data-turbo-temporary
-            />
+            <%= render "shared/samples/timestamp_input" %>
             <%= f.submit t(".select_all_button"),
                      class: "button button--state-default button--size-default" %>
           <% end %>
@@ -104,9 +99,7 @@
                              "t-search-component block w-full p-2.5 pl-10 text-sm text-slate-900 border border-slate-300 rounded-lg bg-slate-50 focus:ring-primary-500 focus:border-primary-500 dark:bg-slate-700 dark:border-slate-600 dark:placeholder-slate-400 dark:text-white dark:focus:ring-primary-500 dark:focus:border-primary-500",
                            placeholder: t(".search.placeholder") %>
             <div
-              class="
-                absolute inset-y-0 left-0 flex items-center pl-3 pointer-events-none
-              "
+              class="absolute inset-y-0 left-0 flex items-center pl-3 pointer-events-none "
             >
               <%= viral_icon(
                 name: "magnifying_glass",

app/views/projects/samples/index.html.erb

@@ -123,12 +123,7 @@
                 ) do |f| %>
           <input type="hidden" name="format" value="turbo_stream"/>
           <input type="hidden" name="select" value="on"/>
-          <input
-            type="hidden"
-            name="timestamp"
-            value="<%=@timestamp%>"
-            data-turbo-temporary
-          >
+          <%= render "shared/samples/timestamp_input" %>
           <%= f.submit t(".select_all_button"),
                    class: "button button--state-default button--size-default" %>
         <% end %>
@@ -153,9 +148,7 @@
                            "t-search-component block w-full p-2.5 pl-10 text-sm text-slate-900 border border-slate-300 rounded-lg bg-slate-50 focus:ring-primary-500 focus:border-primary-500 dark:bg-slate-700 dark:border-slate-600 dark:placeholder-slate-400 dark:text-white dark:focus:ring-primary-500 dark:focus:border-primary-500",
                          placeholder: t(".search.placeholder") %>
           <div
-            class="
-              absolute inset-y-0 left-0 flex items-center pl-3 pointer-events-none
-            "
+            class="absolute inset-y-0 left-0 flex items-center pl-3 pointer-events-none "
           >
             <%= viral_icon(
               name: "magnifying_glass",

app/views/shared/_flash.html.erb

@@ -0,0 +1,3 @@
+<%= turbo_stream.prepend 'flashes' do %>
+  <%= viral_flash(type: type, data: message) %>
+<% end %>