Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement: Edit line list metadata in place #864

Draft
wants to merge 24 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f1e8457
feat: :sparkles: Initial work on editing line list
joshsadam Dec 11, 2024
0fd8069
feat: Add check_editable route to project for enhanced editing capabi…
joshsadam Dec 11, 2024
75c712e
feat: Enhance sample editing functionality with update_field action a…
joshsadam Dec 11, 2024
196d732
feat: Implement editable cell component for enhanced inline editing o…
joshsadam Dec 11, 2024
54ecb45
feat: Refactor sample metadata update logic in SamplesController
joshsadam Dec 11, 2024
10dc64b
feat: Refactor update_field action in SamplesController for improved …
joshsadam Dec 11, 2024
4a5b718
feat: Enhance editable cell functionality and refactor update_field a…
joshsadam Dec 11, 2024
9108642
feat: Add test for editing sample fields in samples_test.rb
joshsadam Dec 11, 2024
5e7e46f
feat: Implement inline editing for sample fields with check_editable …
joshsadam Dec 12, 2024
2fbd0e6
fix: Update sample field test to assert correct button interaction an…
joshsadam Dec 12, 2024
bb00771
feat: Enhance inline editing functionality with cancel support and or…
joshsadam Dec 12, 2024
b09db05
feat: Add field_editable? method to Sample model for user metadata va…
joshsadam Dec 13, 2024
0c30cce
refactor: Update sample field test for inline editing functionality
joshsadam Dec 13, 2024
7efae73
refactor: Simplify editable cell functionality and remove unused actions
joshsadam Dec 13, 2024
36311cc
fix: Update sample field test to assert button interactions for inlin…
joshsadam Dec 13, 2024
ec4818a
fix: Update sample field test to assert button presence for inline ed…
joshsadam Dec 13, 2024
58040c9
refactor: Streamline field update logic and improve rendering methods
joshsadam Dec 16, 2024
a940d70
refactor: Remove unused hidden field for sample ID in editing field cell
joshsadam Dec 16, 2024
289c157
test: Add tests for unchanged field rendering and sample metadata upd…
joshsadam Dec 16, 2024
776a33b
test: Add test for editing a metadata field and update unchanged fiel…
joshsadam Dec 16, 2024
8351bb9
fix: Update response status for editable field checks and add test fo…
joshsadam Dec 16, 2024
ea3fbfc
test: Add test for building update parameters in FieldsController
joshsadam Dec 16, 2024
0b9033e
test: Update sample table assertions to check input values instead of…
joshsadam Dec 17, 2024
1d4a26c
test: Add test for updating metadata value not from an analysis
joshsadam Dec 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions app/components/samples/editable_cell.rb
Original file line number Diff line number Diff line change
@@ -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
<td id="<%= dom_id(@sample, @field) %>">
<%= 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 %>
</td>
ERB

def initialize(field:, sample:)
@sample = sample
@field = field
end
end
end
11 changes: 2 additions & 9 deletions app/components/samples/table_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -199,7 +192,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 %>
Expand Down
79 changes: 79 additions & 0 deletions app/controllers/projects/samples/metadata/fields_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
module Samples
module Metadata
# Controller actions for Project Samples Metadata Fields Controller
class FieldsController < Projects::Samples::ApplicationController

Check warning on line 7 in app/controllers/projects/samples/metadata/fields_controller.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop

[rubocop] reported by reviewdog 🐶 Class has too many lines. [147/100] Raw Output: app/controllers/projects/samples/metadata/fields_controller.rb:7:7: C: Metrics/ClassLength: Class has too many lines. [147/100]
respond_to :turbo_stream

# Param received as:
Expand Down Expand Up @@ -42,6 +42,36 @@
end
end

def editable
authorize! @project, to: :update_sample?
@field = params[:field]
@value = @sample.metadata[@field]
if @sample.can_update_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' })

Check warning on line 57 in app/controllers/projects/samples/metadata/fields_controller.rb

View workflow job for this annotation

GitHub Actions / runner / rubocop

[rubocop] reported by reviewdog 🐶 Line is too long. [160/120] Raw Output: app/controllers/projects/samples/metadata/fields_controller.rb:57:121: C: Layout/LineLength: Line is too long. [160/120]
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
else
update_field_value(original_value, value)
end
end

private

def create_field_params
Expand Down Expand Up @@ -99,6 +129,55 @@
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
Expand Down
28 changes: 28 additions & 0 deletions app/javascript/controllers/inline_edit_controller.js
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
4 changes: 4 additions & 0 deletions app/models/sample.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ def search_data
}
end

def can_update_field?(field)
metadata_provenance.key?(field) && metadata_provenance[field]['source'] == 'user'
end

def should_index?
!deleted?
end
Expand Down
3 changes: 3 additions & 0 deletions app/views/shared/_flash.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<%= turbo_stream.prepend 'flashes' do %>
<%= viral_flash(type: type, data: message) %>
<% end %>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<%= render Samples::EditableCell.new(field: @field, sample: @sample) %>
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<td id="<%= dom_id(@sample, @field) %>">
<%= 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 %>
</td>
5 changes: 4 additions & 1 deletion config/routes/project.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 29 additions & 6 deletions test/system/groups/samples_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
18 changes: 16 additions & 2 deletions test/system/projects/samples_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1548,6 +1548,20 @@ def retrieve_puids
end
end

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

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'
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
Expand Down
Loading