diff --git a/Gemfile b/Gemfile index 6cc3e289..8b4a2ef4 100644 --- a/Gemfile +++ b/Gemfile @@ -14,6 +14,7 @@ gem 'bootstrap', '~> 4.6.2' gem 'coffee-rails', '~> 5.0' gem 'csv' gem 'd3-rails' +gem 'diff-lcs', '~> 2.0' gem 'dartsass-sprockets' # Sass engine (required by bootstrap gem 'devise' gem 'dkim', '~> 1.1' diff --git a/Gemfile.lock b/Gemfile.lock index 9e1cbdb6..27949cdc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -114,6 +114,7 @@ GEM railties (>= 4.1.0) responders warden (~> 1.2.3) + diff-lcs (2.0.0) dkim (1.1.0) erb (6.0.4) erubi (1.13.1) @@ -487,6 +488,7 @@ DEPENDENCIES d3-rails dartsass-sprockets devise + diff-lcs (~> 2.0) dkim (~> 1.1) font_awesome5_rails (~> 1.5) httparty (~> 0.23) diff --git a/app/assets/stylesheets/names.scss b/app/assets/stylesheets/names.scss index 6064a6a6..e0c46162 100644 --- a/app/assets/stylesheets/names.scss +++ b/app/assets/stylesheets/names.scss @@ -16,3 +16,36 @@ } } +.name-history-table { + table-layout: fixed; + min-width: 42rem; + + thead th:first-child { + width: 12rem; + } + + thead th:nth-child(2) { + width: 10rem; + } + + .name-history-date { + color: #6c757d; + font-size: 0.875rem; + font-weight: normal; + white-space: nowrap; + } + + .name-history-version:nth-of-type(even) { + background-color: #f7f8fa; + } + + .name-history-version + .name-history-version { + border-top: 0.75rem solid var(--white); + } + + .name-history-before, + .name-history-after { + white-space: pre-wrap; + overflow-wrap: anywhere; + } +} diff --git a/app/controllers/names_controller.rb b/app/controllers/names_controller.rb index a7c26ee3..c9870b03 100644 --- a/app/controllers/names_controller.rb +++ b/app/controllers/names_controller.rb @@ -12,7 +12,7 @@ class NamesController < ApplicationController before_action( :set_name, only: %i[ - edit update destroy network wiki + edit update destroy network wiki history proposed_in not_validly_proposed_in emended_in assigned_in corrigendum_in corrigendum_orphan corrigendum edit_description edit_rank edit_notes edit_etymology edit_links edit_type @@ -215,6 +215,13 @@ def show end end + # GET /names/1/history + def history + @history = Name::PublicHistory.new(@name) + @versions = @history.versions(page: params[:page]) + @crumbs = [['Names', names_path], [@name.abbr_name, @name], 'History'] + end + # GET /names/linkout.xml # GET /names/1/linkout.xml def linkout diff --git a/app/helpers/names_helper.rb b/app/helpers/names_helper.rb index 0fd59158..fba9f636 100644 --- a/app/helpers/names_helper.rb +++ b/app/helpers/names_helper.rb @@ -1,4 +1,46 @@ module NamesHelper + def history_values(changes, attribute:) + values = + case attribute + when 'etymology' + history_diff_parts(*changes.map { |attributes| Name.new(attributes).full_etymology }) + when 'proposed_in_id' + changes.map { |id| history_publication_reference(id) } + when 'status' + changes.map { |code| Name.status_hash.dig(code, :name) || code } + else + changes.any? { |value| value.is_a?(String) } ? history_diff_parts(*changes) : changes + end + + values.map { |value| safe_join(Array(value)).presence || '—' } + end + + def history_publication_reference(publication_id) + publication = Publication.find_by(id: publication_id) if publication_id + if publication + link_to(publication.short_citation, publication) + elsif publication_id + "Publication ##{publication_id} (deleted)" + end + end + + def history_diff_parts(before, after) + chunks = TextDiff.chunks(before, after) + + before_parts = chunks.filter_map do |unchanged, text, _| + next if text.empty? + + unchanged ? text : content_tag(:del, text, class: 'text-danger') + end + after_parts = chunks.filter_map do |unchanged, _, text| + next if text.empty? + + unchanged ? text : content_tag(:ins, text, class: 'text-success') + end + + [before_parts, after_parts] + end + def link_to_name_type(name) if name.type_is_name? if name.type_name @@ -64,4 +106,7 @@ def name_lineage(name, links: true, last: true, register: nil, visited: []) out.inject(:+) end + + private + end diff --git a/app/models/concerns/versioned.rb b/app/models/concerns/versioned.rb new file mode 100644 index 00000000..d0a8ed37 --- /dev/null +++ b/app/models/concerns/versioned.rb @@ -0,0 +1,62 @@ +module Versioned + extend ActiveSupport::Concern + + included do + has_many :versions, as: :record + class_attribute :versioned_attribute_sets, default: [] + + after_create :record_create_version + after_update :record_update_version + after_destroy :record_delete_version + end + + class_methods do + # Changing any member records before/after values for every member. + def versioned_together(*attributes) + self.versioned_attribute_sets = versioned_attribute_sets + [attributes.map(&:to_s)] + end + end + + private + + def record_create_version + versions.create!(operation: 'create', changeset: {}) + end + + def record_update_version + changeset = versioned_changes + versions.create!(operation: 'update', changeset: changeset) if changeset.any? + end + + def record_delete_version + # The owner is already destroyed, so create directly rather than through + # its association. Deletion records only the event and retains history. + Version.create!(record: self, operation: 'delete', changeset: {}) + end + + # An update changeset maps each changed attribute to [before, after], e.g. + # { 'syllabication' => [nil, 'co.li'] } for E. coli. + # Create and delete versions have an empty changeset; timestamps are omitted. + def versioned_changes + changeset = saved_changes.except('created_at', 'updated_at') + + changed_columns = changeset.keys + self.class.versioned_attribute_sets.each do |columns| + changeset.merge!(versioned_together_changes(columns, changed_columns)) + end + changeset + end + + # If any listed column changed, include each member with a value as an + # ordinary [before, after] pair, including unchanged columns such as language: + # { 'etymology_xx_lang' => ['N.L.', 'N.L.'], + # 'etymology_xx_description' => ['of the colon', 'from the colon'] }. + def versioned_together_changes(columns, changed_columns) + return {} if (changed_columns & columns).empty? + + columns.filter_map do |column| + values = [attribute_before_last_save(column), self[column]] + [column, values] unless values == [nil, nil] + end.to_h + end +end diff --git a/app/models/name.rb b/app/models/name.rb index 9fef19ca..c6a240ff 100644 --- a/app/models/name.rb +++ b/app/models/name.rb @@ -147,6 +147,10 @@ class Name < ApplicationRecord include Name::Wiki include Name::FuzzySearch include TypeMaterial + include Versioned + + ETYMOLOGY_COLUMNS = column_names.grep(/\Aetymology_/).freeze + versioned_together(:name, *ETYMOLOGY_COLUMNS) attr_accessor :only_display attr_accessor :nomenclatural_type_entry diff --git a/app/models/name/public_history.rb b/app/models/name/public_history.rb new file mode 100644 index 00000000..00bb1040 --- /dev/null +++ b/app/models/name/public_history.rb @@ -0,0 +1,52 @@ +class Name::PublicHistory + # Only these recorded attributes appear in the public history. + KEYS = %w[ + name rank syllabication status priority_date authority + nomenclatural_status taxonomic_status proposal_kind corrigendum_from + type_material type_accession proposed_in_id + ].concat(Name::ETYMOLOGY_COLUMNS).freeze + + def initialize(name) + @name = name + end + + # Filter before paginating so each page contains public updates only. + def versions(page:) + @name.versions + .where(operation: 'update') + .where( + 'EXISTS (SELECT 1 FROM jsonb_object_keys(versions.changeset) AS key WHERE key IN (?))', + KEYS + ) + .order(created_at: :desc, id: :desc) + .paginate(page: page, per_page: 25) + end + + # Ordinary fields keep their [before, after] pairs. Etymology fields become + # one pair of attribute snapshots for the view to render together. + def changes_for(version) + changes = version.changeset.slice(*KEYS) + etymology = changes.extract!(*Name::ETYMOLOGY_COLUMNS) + if etymology.any? + changes['etymology'] = etymology_snapshots(etymology, changes['name']) + end + # versioned_together records unchanged companion values too. + changes.reject! { |_, values| values.first == values.last } + changes + end + + private + + # Include the name because full_etymology derives the :xx particle from its + # last word. Older versions without a name pair use the current name. + def etymology_snapshots(etymology, names) + before_name, after_name = names || [@name.name, @name.name] + before_attributes = etymology.transform_values(&:first) + after_attributes = etymology.transform_values(&:last) + + [ + before_attributes.merge('name' => before_name), + after_attributes.merge('name' => after_name) + ] + end +end diff --git a/app/models/version.rb b/app/models/version.rb new file mode 100644 index 00000000..588d1a2a --- /dev/null +++ b/app/models/version.rb @@ -0,0 +1,3 @@ +class Version < ApplicationRecord + belongs_to :record, polymorphic: true, optional: true +end diff --git a/app/views/names/_title.html.erb b/app/views/names/_title.html.erb index 636bf2e2..b85062bb 100644 --- a/app/views/names/_title.html.erb +++ b/app/views/names/_title.html.erb @@ -18,6 +18,7 @@ %> <%= download_buttons([ + [history_name_path(@name), 'history', 'History'], [name_url(@name, format: :json), 'file-code', 'JSON'], [name_url(@name, format: :pdf), 'file-pdf', 'PDF'], observe @@ -78,4 +79,3 @@ <% end %>
- diff --git a/app/views/names/history.html.erb b/app/views/names/history.html.erb new file mode 100644 index 00000000..23020cb6 --- /dev/null +++ b/app/views/names/history.html.erb @@ -0,0 +1,43 @@ +<% provide(:title, "History of #{@name.abbr_name_raw}") %> + +

History of <%= @name.name_html %>

+

<%= link_to('Back to name', @name) %>

+ +<% if @versions.empty? %> +

No history has been recorded for this name.

+<% else %> +
+ + + + + + + + + + <% @versions.each do |version| %> + <% changes = @history.changes_for(version) %> + + <% changes.each_with_index do |(attribute, values), index| %> + <% before, after = history_values(values, attribute: attribute) %> + + <% if index.zero? %> + + <% end %> + + + + + <% end %> + + <% end %> +
DateAttributeBeforeAfter
+ + <%= Name.human_attribute_name(attribute) %><%= before %><%= after %>
+
+<% end %> + +<%= will_paginate(@versions) %> diff --git a/config/routes.rb b/config/routes.rb index d8b5962c..f10d0dea 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -86,6 +86,7 @@ end member do # --> Display name + get :history get :network get :linkout get :wiki diff --git a/db/migrate/20260917100000_create_versions.rb b/db/migrate/20260917100000_create_versions.rb new file mode 100644 index 00000000..886bd899 --- /dev/null +++ b/db/migrate/20260917100000_create_versions.rb @@ -0,0 +1,10 @@ +class CreateVersions < ActiveRecord::Migration[6.1] + def change + create_table :versions do |t| + t.references :record, polymorphic: true, null: false + t.string :operation, null: false + t.jsonb :changeset, null: false + t.datetime :created_at, null: false + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 6f900aa0..e166d71f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2026_08_19_100000) do +ActiveRecord::Schema.define(version: 2026_09_17_100000) do # These are extensions that must be enabled in order to support this database enable_extension "fuzzystrmatch" @@ -568,6 +568,15 @@ t.index ["username"], name: "index_users_on_username", unique: true end + create_table "versions", force: :cascade do |t| + t.string "record_type", null: false + t.bigint "record_id", null: false + t.string "operation", null: false + t.jsonb "changeset", null: false + t.datetime "created_at", null: false + t.index ["record_type", "record_id"], name: "index_versions_on_record" + end + create_table "wikispecies_credentials", force: :cascade do |t| t.bigint "user_id", null: false t.text "encrypted_access_token", null: false diff --git a/lib/text_diff.rb b/lib/text_diff.rb new file mode 100644 index 00000000..e19acd46 --- /dev/null +++ b/lib/text_diff.rb @@ -0,0 +1,20 @@ +require 'diff/lcs' + +module TextDiff + # Returns [unchanged, before_text, after_text] chunks. For an E. coli etymology + # edit, 'of the colon' -> 'from the colon' gives: + # [[false, 'of', 'from'], [true, ' the colon', ' the colon']]. + def self.chunks(before, after) + changes = Diff::LCS.sdiff(before.to_s.scan(/\X/), after.to_s.scan(/\X/)) + # Group characters into unchanged/changed runs, combining additions and deletions. + runs = changes.chunk { |change| change.action == '=' }.to_a + # Absorb unchanged islands of up to two characters between edits; keep the ends. + runs[1...-1].to_a.each { |run| run[0] = false if run[1].size <= 2 } + # Merge the resulting runs into contiguous text chunks. + runs.chunk_while { |a, b| a[0] == b[0] }.map do |group| + chunk = group.flat_map(&:last) + [group.first[0], chunk.map(&:old_element).compact.join, + chunk.map(&:new_element).compact.join] + end + end +end diff --git a/test/controllers/names/history_test.rb b/test/controllers/names/history_test.rb new file mode 100644 index 00000000..a8949e5b --- /dev/null +++ b/test/controllers/names/history_test.rb @@ -0,0 +1,26 @@ +require 'test_helper' + +class NamesHistoryTest < ActionDispatch::IntegrationTest + test 'renders public history' do + name = names(:escherichia_coli) + name.update!(authority: 'Smith', etymology_xx_description: 'of the colon') + + get history_name_path(name) + + assert_response :success + assert_select '.name-history-version', count: 1 + assert_select '.name-history-change', count: 2 + end + + test 'does not reveal private name history' do + name = Name.create!( + name: 'E. coli', rank: 'species', status: 5, created_by: users(:contributor) + ) + name.update!(syllabication: 'co.li') + + get history_name_path(name) + + assert_select 'h3', text: 'Insufficient permits' + assert_select '.name-history-version', count: 0 + end +end diff --git a/test/helpers/names_helper_test.rb b/test/helpers/names_helper_test.rb new file mode 100644 index 00000000..699ab95a --- /dev/null +++ b/test/helpers/names_helper_test.rb @@ -0,0 +1,41 @@ +require 'test_helper' + +class NamesHelperTest < ActionView::TestCase + test 'groups nearby etymology edits into one span' do + changes = [ + { name: 'E. coli', etymology_xx_description: 'of the colon' }, + { name: 'E. coli', etymology_xx_description: 'from the colon' } + ] + before, after = history_values(changes, attribute: 'etymology') + + assert_equal 'coli, of the colon', before + assert_equal 'coli, from the colon', after + end + + test 'keeps distant edits separate and escapes their content' do + changes = [ + { name: 'E. coli', etymology_xx_description: '< E. coli >' }, + { name: 'E. coli', etymology_xx_description: '& E. coli !' } + ] + before, after = history_values(changes, attribute: 'etymology') + + assert_equal 'coli, < E. coli >', before + assert_equal 'coli, & E. coli !', after + end + + test 'renders proposed publication changes as citations' do + publication = publications(:one) + citation = link_to(publication.short_citation, publication) + assert_equal [citation, '—'], + history_values([publication.id, nil], attribute: 'proposed_in_id') + assert_equal ['—', citation], + history_values([nil, publication.id], attribute: 'proposed_in_id') + end + test 'renders a retained publication id when the publication no longer exists' do + publication = publications(:no_doi) + publication.destroy! + + assert_equal ['—', "Publication ##{publication.id} (deleted)"], + history_values([nil, publication.id], attribute: 'proposed_in_id') + end +end diff --git a/test/lib/text_diff_test.rb b/test/lib/text_diff_test.rb new file mode 100644 index 00000000..c31c04dc --- /dev/null +++ b/test/lib/text_diff_test.rb @@ -0,0 +1,39 @@ +require 'minitest/autorun' +require_relative '../../lib/text_diff' + +class TextDiffTest < Minitest::Test + def test_groups_nearby_edits_and_preserves_surrounding_text + assert_equal [ + [true, 'E. coli: ', 'E. coli: '], + [false, 'of', 'from'], + [true, ' the colon', ' the colon'] + ], TextDiff.chunks('E. coli: of the colon', 'E. coli: from the colon') + end + + def test_keeps_distant_edits_separate + assert_equal [ + [false, '<', '&'], + [true, ' E. coli ', ' E. coli '], + [false, '>', '!'] + ], TextDiff.chunks('< E. coli >', '& E. coli !') + end + + def test_represents_additions_and_removals_with_an_empty_side + assert_equal [[true, 'E. coli', 'E. coli'], [false, '', ' strain']], + TextDiff.chunks('E. coli', 'E. coli strain') + assert_equal [[true, 'E. coli', 'E. coli'], [false, ' strain', '']], + TextDiff.chunks('E. coli strain', 'E. coli') + end + + def test_handles_empty_and_unchanged_text + assert_equal [], TextDiff.chunks(nil, '') + assert_equal [[true, 'E. coli', 'E. coli']], TextDiff.chunks('E. coli', 'E. coli') + end + + def test_preserves_unicode_graphemes + assert_equal [[true, 'M', 'M'], [false, 'ü', 'ö'], [true, 'ller', 'ller']], + TextDiff.chunks('Müller', 'Möller') + assert_equal [[true, 'E. coli: ', 'E. coli: '], [false, "e\u0301", 'ö']], + TextDiff.chunks("E. coli: e\u0301", 'E. coli: ö') + end +end diff --git a/test/models/name/public_history_test.rb b/test/models/name/public_history_test.rb new file mode 100644 index 00000000..544948f4 --- /dev/null +++ b/test/models/name/public_history_test.rb @@ -0,0 +1,31 @@ +require 'test_helper' + +class Name::PublicHistoryTest < ActiveSupport::TestCase + setup do + @name = names(:escherichia_coli) + @history = Name::PublicHistory.new(@name) + end + + test 'keeps public changes and groups etymology snapshots without unchanged companions' do + version = Version.new(record_type: 'Name', changeset: { + 'name' => ['E. coli', 'E. coli'], + 'authority' => [nil, 'Smith'], + 'proposed_in_id' => [nil, publications(:one).id], + 'gtdb_accession' => [nil, 'GB_GCA_000005845.2'], + 'etymology_xx_lang' => ['N.L.', 'N.L.'], + 'etymology_xx_description' => ['of the colon', 'from the colon'] + }) + + assert_equal({ + 'authority' => [nil, 'Smith'], + 'proposed_in_id' => [nil, publications(:one).id], + 'etymology' => [ + { 'name' => 'E. coli', 'etymology_xx_lang' => 'N.L.', + 'etymology_xx_description' => 'of the colon' }, + { 'name' => 'E. coli', 'etymology_xx_lang' => 'N.L.', + 'etymology_xx_description' => 'from the colon' } + ] + }, @history.changes_for(version)) + end + +end diff --git a/test/models/name/versioned_test.rb b/test/models/name/versioned_test.rb new file mode 100644 index 00000000..f1c18178 --- /dev/null +++ b/test/models/name/versioned_test.rb @@ -0,0 +1,92 @@ +require 'test_helper' + +class Name::VersionedTest < ActiveSupport::TestCase + setup do + @name = Name.new(name: 'E. coli', rank: 'species') + end + + test 'creating a name records an empty changeset' do + assert_difference -> { @name.versions.count } do + @name.save! + end + + version = @name.versions.last + assert_equal 'create', version.operation + assert_empty version.changeset + end + + test 'updating a name records only changed attributes' do + @name.save! + + assert_difference -> { @name.versions.count } do + @name.update!(syllabication: 'co.li') + end + + version = @name.versions.last + assert_equal 'update', version.operation + assert_equal [nil, 'co.li'], version.changeset['syllabication'] + assert_equal %w[syllabication], version.changeset.keys + end + + test 'updating only timestamps does not add a version' do + @name.save! + + assert_no_difference -> { @name.versions.count } do + @name.update!(created_at: 1.day.ago, updated_at: 1.hour.ago) + end + end + + test 'changing an etymology description snapshots valued etymology attributes' do + @name.assign_attributes( + etymology_xx_lang: 'N.L.', etymology_xx_description: 'of the colon' + ) + @name.save! + + @name.update!(etymology_xx_description: 'from the colon') + + changeset = @name.versions.last.changeset + assert_equal %w[etymology_xx_description etymology_xx_lang name], changeset.keys.sort + assert_equal ['of the colon', 'from the colon'], changeset['etymology_xx_description'] + assert_equal ['N.L.', 'N.L.'], changeset['etymology_xx_lang'] + assert_equal ['E. coli', 'E. coli'], changeset['name'] + end + + test 'saving an unchanged name does not add a version' do + @name.save! + + assert_no_difference -> { @name.versions.count } do + @name.save! + end + end + + test 'deleting a name records an empty changeset' do + @name.save! + versions = Version.where(record: @name) + + assert_difference -> { versions.count } do + @name.destroy! + end + + version = versions.last + assert_equal 'delete', version.operation + assert_empty version.changeset + end + + test 'deleting a name retains its history' do + @name.save! + version = @name.versions.last + + @name.destroy! + + assert Version.exists?(version.id) + end + + test 'versions roll back with the name' do + assert_no_difference -> { Version.count } do + Name.transaction do + @name.save! + raise ActiveRecord::Rollback + end + end + end +end