From 1bc75f5d4019909560e9cd16234ce530f233215f Mon Sep 17 00:00:00 2001 From: barbie-bot Date: Sat, 4 Apr 2026 10:24:26 +0000 Subject: [PATCH] Fix column header lookup fragility, add fetch error handling, add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace position-based column header lookup (ths[3]/ths[4]) with data-column attribute selectors — immune to column reordering - Add .catch() error handlers to editMeasurementWide and setDefaultUnit fetch calls so failures surface in the console instead of silently disappearing - Add MeasurementsController integration tests covering index auth, create with taken_at, empty-readout create, destroy, cross-user destroy isolation, and update Co-Authored-By: Claude Sonnet 4.6 --- app/javascript/application.js | 12 ++-- app/views/measurements/index.html.erb | 4 +- .../measurements_controller_test.rb | 64 ++++++++++++++++++- 3 files changed, 69 insertions(+), 11 deletions(-) diff --git a/app/javascript/application.js b/app/javascript/application.js index 4eb29d3..9ba77ce 100644 --- a/app/javascript/application.js +++ b/app/javascript/application.js @@ -47,10 +47,8 @@ function buildWideTable() { groupMap.get(key).rows.push(r); }); - // Read column headers from compact thead - var ths = document.querySelectorAll('.measurements-compact thead th'); - var takenAtHeader = ths[3] ? ths[3].textContent : ''; - var createdAtHeader = ths[4] ? ths[4].textContent : ''; + var takenAtHeader = (document.querySelector('[data-column="taken-at"]') || {}).textContent || ''; + var createdAtHeader = (document.querySelector('[data-column="created-at"]') || {}).textContent || ''; var table = document.createElement('table'); table.className = 'items-table'; @@ -190,7 +188,8 @@ function editMeasurementWide(url) { panel.scrollIntoView({behavior: 'smooth', block: 'nearest'}); } }); - }); + }) + .catch(err => console.error('editMeasurementWide failed:', err)); } window.editMeasurementWide = editMeasurementWide @@ -229,7 +228,8 @@ function setDefaultUnit(button) { } return response.text(); }) - .then(html => Turbo.renderStreamMessage(html)); + .then(html => Turbo.renderStreamMessage(html)) + .catch(err => console.error('setDefaultUnit failed:', err)); } window.setDefaultUnit = setDefaultUnit diff --git a/app/views/measurements/index.html.erb b/app/views/measurements/index.html.erb index 2aac86b..9c354e9 100644 --- a/app/views/measurements/index.html.erb +++ b/app/views/measurements/index.html.erb @@ -21,8 +21,8 @@ <%= Quantity.model_name.human %> <%= Readout.human_attribute_name(:value) %> <%= Unit.model_name.human %> - <%= Readout.human_attribute_name(:taken_at) %> - <%= Readout.human_attribute_name(:created_at) %> + <%= Readout.human_attribute_name(:taken_at) %> + <%= Readout.human_attribute_name(:created_at) %> <% if current_user.at_least(:active) %> <% end %> diff --git a/test/controllers/measurements_controller_test.rb b/test/controllers/measurements_controller_test.rb index 0adaa76..9d14d3e 100644 --- a/test/controllers/measurements_controller_test.rb +++ b/test/controllers/measurements_controller_test.rb @@ -1,7 +1,65 @@ require "test_helper" class MeasurementsControllerTest < ActionDispatch::IntegrationTest - # test "the truth" do - # assert true - # end + setup do + host! '127.0.0.1' + @user = users(:alice) + post new_user_session_path, params: { user: { email: @user.email, password: 'alice' } } + @quantity = @user.quantities.create!(name: 'Weight') + @unit = @user.units.create!(symbol: 'kg') + end + + test "index returns ok" do + get measurements_path + assert_response :success + end + + test "index requires authentication" do + delete destroy_user_session_path + get measurements_path + assert_response :redirect + end + + test "create records readout with taken_at" do + taken_at = 1.day.ago.change(usec: 0) + assert_difference -> { @user.readouts.count } do + post measurements_path, params: { + taken_at: taken_at.iso8601, + readouts: [{ quantity_id: @quantity.id, value: '82.5', unit_id: @unit.id }] + }, as: :turbo_stream + end + assert_response :success + assert_equal taken_at, @user.readouts.last.taken_at + end + + test "create with no readouts selected shows alert" do + post measurements_path, params: { taken_at: Time.now.iso8601 }, as: :turbo_stream + assert_response :success + end + + test "destroy removes readout" do + readout = @user.readouts.create!(quantity: @quantity, unit: @unit, value: 82.5, taken_at: 1.day.ago) + assert_difference -> { @user.readouts.count }, -1 do + delete measurement_path(readout), as: :turbo_stream + end + assert_response :success + end + + test "destroy cannot remove another user's readout" do + other_quantity = users(:bob).quantities.create!(name: 'Weight') + other_unit = users(:bob).units.create!(symbol: 'kg') + readout = users(:bob).readouts.create!(quantity: other_quantity, unit: other_unit, value: 70.0, taken_at: 1.day.ago) + assert_no_difference -> { users(:bob).readouts.count } do + delete measurement_path(readout), as: :turbo_stream + end + end + + test "update changes readout value" do + readout = @user.readouts.create!(quantity: @quantity, unit: @unit, value: 82.5, taken_at: 1.day.ago) + patch measurement_path(readout), params: { + readout: { value: '83.0', unit_id: @unit.id, taken_at: readout.taken_at.iso8601 } + }, as: :turbo_stream + assert_response :success + assert_in_delta 83.0, readout.reload.value + end end