From bc60b5958357af69f9b65c9b1e00b5fa611bbbf2 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Wed, 14 Aug 2024 16:49:02 -0500 Subject: [PATCH] Version links shouldn't have a 'v' in them Fixes #1132 --- .rubocop_todo.yml | 6 +-- .../version_actions_component.html.erb | 2 +- app/components/version_actions_component.rb | 5 +- app/components/version_row_component.html.erb | 7 +++ app/components/version_row_component.rb | 17 +++++++ app/components/versions_component.html.erb | 14 ++---- app/components/versions_component.rb | 8 --- app/models/purl_version.rb | 4 ++ config/routes.rb | 11 +++- spec/components/version_row_component_spec.rb | 50 +++++++++++++++++++ spec/components/versions_component_spec.rb | 7 +-- 11 files changed, 98 insertions(+), 33 deletions(-) create mode 100644 app/components/version_row_component.html.erb create mode 100644 app/components/version_row_component.rb create mode 100644 spec/components/version_row_component_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index b220a8f3..1140b738 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2024-08-09 21:39:57 UTC using RuboCop version 1.65.1. +# on 2024-08-14 21:47:26 UTC using RuboCop version 1.65.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -21,7 +21,7 @@ Metrics/BlockLength: # Offense count: 3 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 189 + Max: 192 # Offense count: 2 # Configuration parameters: AllowedMethods, AllowedPatterns. @@ -79,7 +79,7 @@ RSpec/LeadingSubject: - 'spec/model/content_metadata_spec.rb' - 'spec/model/iiif_presentation_manifest_spec.rb' -# Offense count: 77 +# Offense count: 80 RSpec/MultipleExpectations: Max: 19 diff --git a/app/components/version_actions_component.html.erb b/app/components/version_actions_component.html.erb index dcaed50d..43e99661 100644 --- a/app/components/version_actions_component.html.erb +++ b/app/components/version_actions_component.html.erb @@ -2,7 +2,7 @@ <% if requested_version? %> You are viewing this version <% else %> - View + <%= link_to 'View', version %> <% end %> | Copy URL <% else %> diff --git a/app/components/version_actions_component.rb b/app/components/version_actions_component.rb index 3cb15fca..fd8431ce 100644 --- a/app/components/version_actions_component.rb +++ b/app/components/version_actions_component.rb @@ -1,14 +1,13 @@ # frozen_string_literal: true class VersionActionsComponent < ViewComponent::Base - def initialize(version:, requested_version:, url:) + def initialize(version:, requested_version:) @version = version @requested_version = requested_version - @url = url super end - attr_reader :version, :url + attr_reader :version delegate :state, to: :version diff --git a/app/components/version_row_component.html.erb b/app/components/version_row_component.html.erb new file mode 100644 index 00000000..f22d7e11 --- /dev/null +++ b/app/components/version_row_component.html.erb @@ -0,0 +1,7 @@ + + Version <%= version.version_id %> + <%= updated_at %> + + <%= render VersionActionsComponent.new(version:, requested_version:) %> + + diff --git a/app/components/version_row_component.rb b/app/components/version_row_component.rb new file mode 100644 index 00000000..4e55cedb --- /dev/null +++ b/app/components/version_row_component.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class VersionRowComponent < ViewComponent::Base + with_collection_parameter :version + + def initialize(version:, requested_version:) + @version = version + @requested_version = requested_version + super + end + + attr_reader :version, :requested_version + + def updated_at + l(version.updated_at.to_date, format: :short) if version.updated_at + end +end diff --git a/app/components/versions_component.html.erb b/app/components/versions_component.html.erb index 0d93de17..e6ee68ff 100644 --- a/app/components/versions_component.html.erb +++ b/app/components/versions_component.html.erb @@ -1,16 +1,8 @@ <%= render SectionComponent.new(label: 'Versions', label_id:) do %> <%= render TableComponent.new(label_id:) do %> - <% versions.each do |version| %> - - - Version <%= version.version_id %> - <%= updated_at(version) %> - - <%= render VersionActionsComponent.new(version:, requested_version: @version, url: url(version)) %> - - - - <% end %> + + <%= render VersionRowComponent.with_collection(versions, requested_version: @version) %> + <% end %>

Each version has a distinct URL, but you can use this PURL to access the latest version.
<%= link_to embeddable_url, embeddable_url %> diff --git a/app/components/versions_component.rb b/app/components/versions_component.rb index 5c2f1131..3d49fb7a 100644 --- a/app/components/versions_component.rb +++ b/app/components/versions_component.rb @@ -14,14 +14,6 @@ def embeddable_url @embeddable_url ||= helpers.embeddable_url(purl.druid) end - def updated_at(version) - l(version.updated_at.to_date, format: :short) if version.updated_at - end - - def url(version) - versioned_purl_url(id: purl.druid, version: "v#{version.version_id}") - end - def versions @purl.versions.sort_by(&:version_id).reverse end diff --git a/app/models/purl_version.rb b/app/models/purl_version.rb index e3d4b7eb..566661d1 100644 --- a/app/models/purl_version.rb +++ b/app/models/purl_version.rb @@ -14,6 +14,10 @@ class ObjectNotReady < StandardError; end MODS_NS = 'http://www.loc.gov/mods/v3'.freeze + def persisted? + true + end + def head? head end diff --git a/config/routes.rb b/config/routes.rb index 66a2ec7a..0355fd4f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -20,9 +20,10 @@ get ':id/file/:file' => 'purl#file', as: :purl_file get ':id/metrics' => 'purl#metrics', as: :purl_metrics + resources :purl, only: [:show], path: '/' do member do - get 'version/:version', to: 'purl#show', as: :versioned + get 'version/:version', to: 'purl#show', as: :version get 'embed', to: redirect("/iframe/?url=#{Settings.embed.url % { druid: '%{id}' }}") # These routes should only be used until our viewers support v3 manifests. # We should aim to only serve a IIIF resource from a single URL. @@ -81,3 +82,11 @@ options '/:id/*whatever', to: 'iiif#options' end + +Rails.application.routes.named_routes.path_helpers_module.module_eval do + # @param [PurlVersion] a PurlVersion object + # @return path for the show route + def purl_version_path(purl_version, options = {}) + version_purl_path(id: purl_version.druid, version: purl_version.version_id) + end +end diff --git a/spec/components/version_row_component_spec.rb b/spec/components/version_row_component_spec.rb new file mode 100644 index 00000000..ddcdf75f --- /dev/null +++ b/spec/components/version_row_component_spec.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe VersionRowComponent, type: :component do + let(:purl) { PurlResource.new(id: 'wp335yr5649') } + + before { render_inline(instance) } + + context 'when rendering not the requested version' do + let(:instance) { described_class.new(version: purl.version(:head), requested_version: purl.version(2)) } + + it 'renders the version row' do + expect(page).to have_css 'th', text: 'Version 4' + expect(page).to have_css 'td', text: 'Jul 24, 2024' + expect(page).to have_link 'View', href: '/wp335yr5649/version/4' + expect(page).to have_css '[data-clipboard-url-value="http://test.host/wp335yr5649/version/4"]', text: 'Copy URL' + end + end + + context 'when rendering the requested version' do + let(:instance) { described_class.new(version: purl.version(2), requested_version: purl.version(2)) } + + it 'renders the version row' do + expect(page).to have_css 'th', text: 'Version 2' + expect(page).to have_css 'td', text: 'Jul 22, 2024' + expect(page).to have_css 'td', text: 'You are viewing this version' + expect(page).to have_css '[data-clipboard-url-value="http://test.host/wp335yr5649/version/2"]', text: 'Copy URL' + end + end + + context 'when rendering a withdrawn version' do + let(:instance) { described_class.new(version: purl.version(3), requested_version: purl.version(2)) } + + it 'renders the version row' do + expect(page).to have_css 'th', text: 'Version 3' + expect(page).to have_css 'td', text: 'Jul 23, 2024' + expect(page).to have_css 'td', text: 'Withdrawn' + end + end + + context 'when rendering a permanently withdrawn version' do + let(:instance) { described_class.new(version: purl.version(1), requested_version: purl.version(2)) } + + it 'renders the version row' do + expect(page).to have_css 'th', text: 'Version 1' + expect(page).to have_css 'td', text: 'Permanently withdrawn' + end + end +end diff --git a/spec/components/versions_component_spec.rb b/spec/components/versions_component_spec.rb index a8f858ca..754e80fa 100644 --- a/spec/components/versions_component_spec.rb +++ b/spec/components/versions_component_spec.rb @@ -12,12 +12,7 @@ expect(page).to have_css '.section-header', text: 'Versions' expect(page).to have_css 'p', text: 'Each version has a distinct URL, but you can use this PURL to access the latest version.' expect(page).to have_link 'https://purl.stanford.edu/wp335yr5649' - end - it 'displays versions as expected' do - expect(page.text).to match(/Version 4\s+Jul 24, 2024\s+You are viewing this version | Copy URL/) - expect(page.text).to match(/Version 3\s+Jul 23, 2024\s+Withdrawn/) - expect(page.text).to match(/Version 2\s+Jul 22, 2024\s+View | Copy URL/) - expect(page.text).to match(/Version 1\s+Permanently withdrawn/) + expect(page).to have_css 'table tr', count: 4 end end