From a4c6818ffc1f8560aec2ce6703199fabbc412645 Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Thu, 31 Oct 2024 11:59:42 +0000 Subject: [PATCH 1/3] Fix crash when looking up snapshot's parent when running under sub-URI #2042 --- app/controllers/snapshots_controller.rb | 45 +++++++++---------- app/views/snapshots/_buttons.html.erb | 12 ++--- app/views/snapshots/export_preview.html.erb | 6 +-- app/views/snapshots/mint_doi_confirm.html.erb | 6 +-- app/views/snapshots/new.html.erb | 10 ++--- app/views/snapshots/show.html.erb | 4 +- test/functional/snapshots_controller_test.rb | 18 +++++++- 7 files changed, 55 insertions(+), 46 deletions(-) diff --git a/app/controllers/snapshots_controller.rb b/app/controllers/snapshots_controller.rb index ebeb52f528..cd22f09a2c 100644 --- a/app/controllers/snapshots_controller.rb +++ b/app/controllers/snapshots_controller.rb @@ -2,7 +2,7 @@ require 'zenodo-client' class SnapshotsController < ApplicationController - before_action :find_resource + before_action :get_parent_resource before_action :auth_resource, only: [:mint_doi_confirm, :mint_doi, :new, :create, :export_preview, :export_submit, :destroy] before_action :check_resource_permitted_for_ro, only: [:new, :create] before_action :find_snapshot, only: [:show, :mint_doi_confirm, :mint_doi, :download, :export_preview, :export_submit, :destroy] @@ -14,13 +14,13 @@ class SnapshotsController < ApplicationController include Seek::ExternalServiceWrapper def create - @snapshot = @resource.create_snapshot + @snapshot = @parent_resource.create_snapshot if @snapshot flash[:notice] = "Snapshot created" - redirect_to polymorphic_path([@resource, @snapshot]) + redirect_to polymorphic_path([@parent_resource, @snapshot]) else - flash[:error] = @resource.errors.full_messages.join(', ') - redirect_to polymorphic_path(@resource) + flash[:error] = @parent_resource.errors.full_messages.join(', ') + redirect_to polymorphic_path(@parent_resource) end end @@ -50,13 +50,13 @@ def mint_doi_confirm end def mint_doi - wrap_service('DataCite', polymorphic_path([@resource, @snapshot])) do + wrap_service('DataCite', polymorphic_path([@parent_resource, @snapshot])) do if @snapshot.mint_doi flash[:notice] = "DOI successfully minted" - redirect_to polymorphic_path([@resource, @snapshot]) + redirect_to polymorphic_path([@parent_resource, @snapshot]) else flash[:error] = @snapshot.errors.full_messages - redirect_to polymorphic_path([@resource, @snapshot]) + redirect_to polymorphic_path([@parent_resource, @snapshot]) end end end @@ -67,13 +67,13 @@ def export_preview def export_submit # Export AND publish access_token = @oauth_session.access_token - wrap_service('Zenodo', polymorphic_path([@resource, @snapshot]), rescue_all: !Rails.env.test?) do + wrap_service('Zenodo', polymorphic_path([@parent_resource, @snapshot]), rescue_all: !Rails.env.test?) do if @snapshot.export_to_zenodo(access_token, metadata_params) && @snapshot.publish_in_zenodo(access_token) flash[:notice] = "Snapshot successfully exported to Zenodo" - redirect_to polymorphic_path([@resource, @snapshot]) + redirect_to polymorphic_path([@parent_resource, @snapshot]) else flash[:error] = @snapshot.errors.full_messages - redirect_to polymorphic_path([@resource, @snapshot]) + redirect_to polymorphic_path([@parent_resource, @snapshot]) end end end @@ -81,47 +81,42 @@ def export_submit # Export AND publish def destroy if @snapshot.has_doi? flash[:error] = "You cannot delete a snapshot that has a DOI." - redirect_to polymorphic_path([@resource, @snapshot]) + redirect_to polymorphic_path([@parent_resource, @snapshot]) else @snapshot.destroy flash[:notice] = "Snapshot successfully deleted" - redirect_to polymorphic_path(@resource) + redirect_to polymorphic_path(@parent_resource) end end private - def find_resource # This is hacky :( - resource, id = request.path.split('/')[1, 2] - @resource = safe_class_lookup(resource.singularize.classify).find(id) - end - def auth_resource - unless is_auth?(@resource, :manage) + unless is_auth?(@parent_resource, :manage) flash[:error] = "You are not authorized to manage snapshots of this resource." - redirect_to polymorphic_path(@resource) + redirect_to polymorphic_path(@parent_resource) end end def check_resource_permitted_for_ro - unless @resource.permitted_for_research_object? + unless @parent_resource.permitted_for_research_object? flash[:error] = "You may only create snapshots of publicly accessible resources." - redirect_to polymorphic_path(@resource) + redirect_to polymorphic_path(@parent_resource) end end def find_snapshot - @snapshot = @resource.snapshots.where(snapshot_number: params[:id]).first + @snapshot = @parent_resource.snapshots.where(snapshot_number: params[:id]).first if @snapshot.nil? flash[:error] = "Snapshot #{params[:id]} doesn't exist." - redirect_to polymorphic_path(@resource) + redirect_to polymorphic_path(@parent_resource) end end def doi_minting_enabled? unless Seek::Config.doi_minting_enabled flash[:error] = "DOI minting is not enabled." - redirect_to polymorphic_path([@resource, @snapshot]) + redirect_to polymorphic_path([@parent_resource, @snapshot]) end end diff --git a/app/views/snapshots/_buttons.html.erb b/app/views/snapshots/_buttons.html.erb index 9a932bf48b..c02e752515 100644 --- a/app/views/snapshots/_buttons.html.erb +++ b/app/views/snapshots/_buttons.html.erb @@ -1,15 +1,15 @@ -<% if @resource.can_manage? %> +<% if @parent_resource.can_manage? %> <% if @snapshot.can_export_to_zenodo? %> - <%= button_link_to("Export to Zenodo", 'snapshot_export', @zenodo_oauth_client.authorize_url(polymorphic_path([@resource, @snapshot], :action => 'export'))) %> + <%= button_link_to("Export to Zenodo", 'snapshot_export', @zenodo_oauth_client.authorize_url(polymorphic_path([@parent_resource, @snapshot], :action => 'export'))) %> <% end %> <% unless @snapshot.has_doi? %> <% if @snapshot.can_mint_doi? %> - <%= button_link_to("Generate a DOI", 'doi', polymorphic_path([@resource, @snapshot], action: 'mint_doi_confirm')) %> + <%= button_link_to("Generate a DOI", 'doi', polymorphic_path([@parent_resource, @snapshot], action: 'mint_doi_confirm')) %> <% end %> - <%= button_link_to("Delete", 'destroy', polymorphic_path([@resource, @snapshot]), + <%= button_link_to("Delete", 'destroy', polymorphic_path([@parent_resource, @snapshot]), { confirm: "Are you sure you wish to delete this snapshot?", method: :delete }) %> <% end %> <% end %> -<% if @resource.can_view? %> - <%= button_link_to("Download", 'download', polymorphic_path([@resource, @snapshot], :action => 'download')) %> +<% if @parent_resource.can_view? %> + <%= button_link_to("Download", 'download', polymorphic_path([@parent_resource, @snapshot], :action => 'download')) %> <% end %> diff --git a/app/views/snapshots/export_preview.html.erb b/app/views/snapshots/export_preview.html.erb index ff130bd7a8..2c3e0d514e 100644 --- a/app/views/snapshots/export_preview.html.erb +++ b/app/views/snapshots/export_preview.html.erb @@ -1,11 +1,11 @@
<%= panel("Confirmation") do %> - <%= form_tag(polymorphic_path([@resource, @snapshot], :action => 'export'), method: :post) do %> + <%= form_tag(polymorphic_path([@parent_resource, @snapshot], :action => 'export'), method: :post) do %> <%= image_tag('logos/zenodo.png', :class => 'zenodo-publish') %>

You are about to publish: - <%= @resource.title %> to Zenodo.
+ <%= @parent_resource.title %> to Zenodo.
Please fill out some additional metadata required by Zenodo before submitting.

@@ -77,7 +77,7 @@

<%= submit_tag 'Publish', :class => 'btn btn-primary' %> - <%= cancel_button(polymorphic_path(@resource)) %> + <%= cancel_button(polymorphic_path(@parent_resource)) %>

<% end %> <% end %> diff --git a/app/views/snapshots/mint_doi_confirm.html.erb b/app/views/snapshots/mint_doi_confirm.html.erb index 90eeffdbae..e5e27634d0 100644 --- a/app/views/snapshots/mint_doi_confirm.html.erb +++ b/app/views/snapshots/mint_doi_confirm.html.erb @@ -1,9 +1,9 @@ -<% type_text = "#{text_for_resource(@resource)} snapshot" %> +<% type_text = "#{text_for_resource(@parent_resource)} snapshot" %>
<%= panel("Confirmation") do %> - <%= form_tag(polymorphic_path([@resource, @snapshot], action: 'mint_doi'), method: :post) do %> + <%= form_tag(polymorphic_path([@parent_resource, @snapshot], action: 'mint_doi'), method: :post) do %> <%= image_tag('logos/doi_logo.svg', class: 'mint-doi') %>

@@ -28,7 +28,7 @@

<%= submit_tag 'Mint DOI', class: 'btn btn-primary' %> - <%= cancel_button(polymorphic_path(@resource)) %> + <%= cancel_button(polymorphic_path(@parent_resource)) %>

<% end %> <% end %> diff --git a/app/views/snapshots/new.html.erb b/app/views/snapshots/new.html.erb index d834dc68cb..35f9c07b42 100644 --- a/app/views/snapshots/new.html.erb +++ b/app/views/snapshots/new.html.erb @@ -1,5 +1,5 @@ <%= render :partial => "general/page_title", :locals => { :title => 'Snapshot Preview' } %> -<% items = Seek::ResearchObjects::Generator.new(@resource).gather_entries(true).group_by(&:permitted_for_research_object?) %> +<% items = Seek::ResearchObjects::Generator.new(@parent_resource).gather_entries(true).group_by(&:permitted_for_research_object?) %> <% included_items = items[true] || [] %> <% excluded_items = items[false] || [] %> @@ -11,7 +11,7 @@ These resources will be excluded from the snapshot due to not having public access privileges. To include an excluded resource in the snapshot, please make it publicly accessible. <% if excluded_items.any? %> -
To quickly publish these items you can select the Publish full <%=t(@resource.class.name.downcase)%> button below. +
To quickly publish these items you can select the Publish full <%=t(@parent_resource.class.name.downcase)%> button below. <% end %> <% end %> @@ -39,9 +39,9 @@ <% end %>
-<%= link_to('Create Snapshot', polymorphic_path([@resource, :snapshots]), +<%= link_to('Create Snapshot', polymorphic_path([@parent_resource, :snapshots]), :method => :post, :class => 'btn btn-primary') %> -<%= image_tag_for_key('publish', polymorphic_path(@resource, :action => :check_related_items), nil, {:method=>:post,:class => 'btn btn-default'}, "Publish full #{t(@resource.class.name.downcase)}") if excluded_items.any? %> +<%= image_tag_for_key('publish', polymorphic_path(@parent_resource, :action => :check_related_items), nil, {:method=>:post,:class => 'btn btn-default'}, "Publish full #{t(@parent_resource.class.name.downcase)}") if excluded_items.any? %> -or <%= cancel_button polymorphic_path(@resource) -%> \ No newline at end of file +or <%= cancel_button polymorphic_path(@parent_resource) -%> \ No newline at end of file diff --git a/app/views/snapshots/show.html.erb b/app/views/snapshots/show.html.erb index 10c45aafd3..e4ed83edcf 100644 --- a/app/views/snapshots/show.html.erb +++ b/app/views/snapshots/show.html.erb @@ -55,9 +55,9 @@
<%= render partial: 'assets/citation_box', locals: { doi: @snapshot.doi } unless @snapshot.doi.blank? %> - <%= render partial: 'snapshots/snapshots', locals: { snapshots: @resource.snapshots, + <%= render partial: 'snapshots/snapshots', locals: { snapshots: @parent_resource.snapshots, selected: @snapshot, - resource: @resource } %> + resource: @parent_resource } %> <%#logging for snapshots didnt start until ~september (dates will vary depending on update time, but is to give a rough indication only) %> <%= render partial: 'assets/usage_info_box', locals: { resource: @snapshot, logging_start_date: Date.parse('01-09-2018') } diff --git a/test/functional/snapshots_controller_test.rb b/test/functional/snapshots_controller_test.rb index 000b40a499..a0eb8c35ba 100644 --- a/test/functional/snapshots_controller_test.rb +++ b/test/functional/snapshots_controller_test.rb @@ -97,6 +97,22 @@ class SnapshotsControllerTest < ActionController::TestCase assert_redirected_to assay_snapshot_path(assay, assigns(:snapshot).snapshot_number) end + test 'can create snapshot under sub-uri' do + with_relative_root('/seek/seek123') do + user = FactoryBot.create(:user) + assay = FactoryBot.create(:assay, policy: FactoryBot.create(:publicly_viewable_policy), + contributor: user.person, creators: [user.person]) + login_as(user) + + assert_difference('Snapshot.count') do + post :create, params: { assay_id: assay } + end + + assert assay.can_manage?(user) + assert_redirected_to assay_snapshot_path(assay, assigns(:snapshot).snapshot_number) + end + end + test "can't create snapshot if no manage permissions" do user = FactoryBot.create(:user) investigation = FactoryBot.create(:investigation, policy: FactoryBot.create(:publicly_viewable_policy), @@ -195,8 +211,6 @@ class SnapshotsControllerTest < ActionController::TestCase assert flash[:error].include?('exist') end - - test 'can get confirmation when minting DOI for snapshot' do datacite_mock create_investigation_snapshot From b41ce8bb64fb7dc45a9f7022b4266ae2cef0b5bf Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Thu, 31 Oct 2024 12:44:23 +0000 Subject: [PATCH 2/3] Fix variable name --- app/views/assets/_usage_info_box.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/assets/_usage_info_box.html.erb b/app/views/assets/_usage_info_box.html.erb index 54606597ac..c65819c045 100644 --- a/app/views/assets/_usage_info_box.html.erb +++ b/app/views/assets/_usage_info_box.html.erb @@ -37,7 +37,7 @@
<%# for items created before we started collecting logs %> - <% if logging_start_date && @resource.created_at <= logging_start_date %> + <% if logging_start_date && @parent_resource.created_at <= logging_start_date %>

(Since <%= logging_start_date.strftime('%B %Y') %>)

From eaa5795b6e26fa8f8df31cd1ae93ddeb648ec0a2 Mon Sep 17 00:00:00 2001 From: Finn Bacall Date: Wed, 13 Nov 2024 14:56:18 +0000 Subject: [PATCH 3/3] Ensure links in samples table include sub-URI --- app/assets/javascripts/samples.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/samples.js b/app/assets/javascripts/samples.js index d1be4e785e..647f2af410 100644 --- a/app/assets/javascripts/samples.js +++ b/app/assets/javascripts/samples.js @@ -81,10 +81,12 @@ Samples.initTable = function (selector, enableRowSelection, opts) { "targets": strainColumns, "render": function (data, type, row) { if(data && data.id) { - if (data.title) - return '' + data.title + ''; - else + if (data.title) { + var href = URL_ROOT + '/strains/' + data.id; + return '' + data.title + ''; + } else { return '' + data.id + ''; + } } else { return 'Not specified'; } @@ -105,10 +107,12 @@ Samples.initTable = function (selector, enableRowSelection, opts) { var values = Array.isArray(data) ? data : [data]; var result = $j.map(values, function(value, i) { if(value && value.id) { - if (value.title) - return '' + value.title + ''; - else + if (value.title) { + var href = URL_ROOT + '/samples/' + value.id; + return '' + value.title + ''; + } else { return '' + (value.id || value.title) + ''; + } } else { return 'Not specified'; } @@ -124,7 +128,8 @@ Samples.initTable = function (selector, enableRowSelection, opts) { options["columnDefs"].push({ "targets": [index], "render": function (data, type, row) { - return '' + row.title + ''; + var href = URL_ROOT + '/samples/' + row.id; + return '' + row.title + ''; } }); }