diff --git a/app/components/samples/editable_cell.rb b/app/components/samples/editable_cell.rb new file mode 100644 index 0000000000..642a866700 --- /dev/null +++ b/app/components/samples/editable_cell.rb @@ -0,0 +1,23 @@ +# 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: 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" %> + <% end %> + + ERB + + def initialize(field:, sample:) + @sample = sample + @field = field + end + end +end diff --git a/app/components/samples/table_component.html.erb b/app/components/samples/table_component.html.erb index af09179b27..c350534038 100644 --- a/app/components/samples/table_component.html.erb +++ b/app/components/samples/table_component.html.erb @@ -132,14 +132,7 @@ <% end %> <% end %> <% end %> - <% @metadata_fields.each do |field| %> - <%= render_cell( - tag: 'td', - classes: class_names('px-3 py-3') - ) do %> - <%= sample.metadata[field] %> - <% end %> - <% end %> + <%= render Samples::EditableCell.with_collection(@metadata_fields, sample: sample) %> <% if @renders_row_actions %> <%= render_cell( tag: 'td', @@ -199,7 +192,7 @@ >0 - + <% end %> diff --git a/app/controllers/projects/samples/metadata/fields_controller.rb b/app/controllers/projects/samples/metadata/fields_controller.rb index 81dcd376d6..c81b566e3d 100644 --- a/app/controllers/projects/samples/metadata/fields_controller.rb +++ b/app/controllers/projects/samples/metadata/fields_controller.rb @@ -42,6 +42,38 @@ def update # rubocop:disable Metrics/AbcSize end end + def editable + authorize! @project, to: :update_sample? + @field = params[:field] + @value = @sample.metadata[@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', + locals: { sample: @sample, field: @field, value: @value } + ) + else + render status: :unprocessable_entity, 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_unchanged_field + elsif original_value.blank? + create_metadata_field(@field, value) + else + update_field_value(original_value, value) + end + end + private def create_field_params @@ -99,6 +131,65 @@ 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 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) + + 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 diff --git a/app/javascript/controllers/inline_edit_controller.js b/app/javascript/controllers/inline_edit_controller.js new file mode 100644 index 0000000000..b4c769a8dc --- /dev/null +++ b/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; + } + } +} diff --git a/app/models/sample.rb b/app/models/sample.rb index 2918fa37e5..95ef7f47ca 100644 --- a/app/models/sample.rb +++ b/app/models/sample.rb @@ -98,6 +98,14 @@ def search_data } end + def field?(field) + metadata.key?(field) + end + + def updatable_field?(field) + metadata_provenance.key?(field) && metadata_provenance[field]['source'] == 'user' + end + def should_index? !deleted? end 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/shared/samples/metadata/fields/_editing_field_cell.html.erb b/app/views/shared/samples/metadata/fields/_editing_field_cell.html.erb new file mode 100644 index 0000000000..f051eaba6e --- /dev/null +++ b/app/views/shared/samples/metadata/fields/_editing_field_cell.html.erb @@ -0,0 +1,16 @@ + + <%= 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 :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/config/routes/project.rb b/config/routes/project.rb index d329342ed2..3c5465798e 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -74,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] 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..1b6f4b8ef0 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,70 @@ class FieldsControllerTest < ActionDispatch::IntegrationTest } }, format: :turbo_stream } 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 :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 '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 + ), + 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 + 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 diff --git a/test/system/groups/samples_test.rb b/test/system/groups/samples_test.rb index 5eda210ec4..2367d9952b 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 @@ -767,5 +767,57 @@ 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 + + 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 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 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 diff --git a/test/system/projects/samples_test.rb b/test/system/projects/samples_test.rb index 86d3cff973..a024e53609 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 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 @@ -1548,6 +1548,58 @@ def retrieve_puids end end + 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 + 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: 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 + 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