From b05f3ef386bfd7126a37dd03f39fd8eeb7d0632f Mon Sep 17 00:00:00 2001 From: lynn Date: Thu, 7 May 2026 16:03:06 -0700 Subject: [PATCH 1/4] AO3-7414 Display error messages for multi-URL import --- app/controllers/works_controller.rb | 6 +++- app/models/story_parser.rb | 2 +- .../works/works_controller_spec.rb | 33 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 61e6ad43a02..38bb2eebbcd 100755 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -537,7 +537,11 @@ def import_multiple(urls, options) # collect the errors neatly, matching each error to the failed url unless failed_urls.empty? - error_msgs = 0.upto(failed_urls.length).map { |index| "
#{failed_urls[index]}
#{errors[index]}
" }.join("\n") + error_msgs = (0...failed_urls.length).map do |index| + # each failed url may have multiple errors, so show them in a bulleted list underneath the url + errors_per_url = errors[index].map { |error| "
  • #{error}
    • " }.join("\n") + "
    #{failed_urls[index]}
    " + end.join("\n") flash.now[:error] = "

    #{ts('Failed Imports')}

    #{error_msgs}
    ".html_safe end diff --git a/app/models/story_parser.rb b/app/models/story_parser.rb index c9e869a21f5..536cb7a3bab 100644 --- a/app/models/story_parser.rb +++ b/app/models/story_parser.rb @@ -178,7 +178,7 @@ def import_from_urls(urls, options = {}) works << work else failed_urls << url - errors << work.errors.values.join(", ") + errors << work.errors.full_messages work.delete if work end rescue Timeout::Error diff --git a/spec/controllers/works/works_controller_spec.rb b/spec/controllers/works/works_controller_spec.rb index 2ab467ecd25..b58434f7bf3 100644 --- a/spec/controllers/works/works_controller_spec.rb +++ b/spec/controllers/works/works_controller_spec.rb @@ -24,4 +24,37 @@ end end end + + describe ".import_multiple" do + context "when the import has a failure from at least one URL" do + before do + allow_any_instance_of(StoryParser).to receive(:import_from_urls).and_return([ + [], + urls, + [["failedurl1 first error", "failedurl1 second error"], ["failedurl2 first error"]] + ]) + allow(controller).to receive(:render).with(:new_import) + end + let(:urls) { ["https://www.failedurl1.com", "https://www.failedurl2.com"] } + + it 'returns a well-formatted error message' do + expect(controller).to receive(:render).with(:new_import) + controller.send(:import_multiple, urls, {}) + expect(flash[:error]).to eq( + "

    Failed Imports

    " + + "
    " + + "
    https://www.failedurl1.com
    " + + "\n" + + "
    https://www.failedurl2.com
    " + + "" + + "
    " + ) + end + end + end end From 6365ed6bc8c27e9cef49e57acf032a0e49ff7219 Mon Sep 17 00:00:00 2001 From: lynn Date: Thu, 7 May 2026 16:18:50 -0700 Subject: [PATCH 2/4] AO3-7414 Prefer backslash over + (rubocop) --- .../works/works_controller_spec.rb | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/spec/controllers/works/works_controller_spec.rb b/spec/controllers/works/works_controller_spec.rb index b58434f7bf3..6fe9e003ef7 100644 --- a/spec/controllers/works/works_controller_spec.rb +++ b/spec/controllers/works/works_controller_spec.rb @@ -41,17 +41,17 @@ expect(controller).to receive(:render).with(:new_import) controller.send(:import_multiple, urls, {}) expect(flash[:error]).to eq( - "

    Failed Imports

    " + - "
    " + - "
    https://www.failedurl1.com
    " + - "
      " + - "
    • failedurl1 first error
      • \n" + - "
    • failedurl1 second error
      • " + - "
    \n" + - "
    https://www.failedurl2.com
    " + - "
      " + - "
    • failedurl2 first error
      • " + - "
    " + + "

    Failed Imports

    " \ + "
    " \ + "
    https://www.failedurl1.com
    " \ + "
      " \ + "
    • failedurl1 first error
      • \n" \ + "
    • failedurl1 second error
      • " \ + "
    \n" \ + "
    https://www.failedurl2.com
    " \ + "
      " \ + "
    • failedurl2 first error
      • " \ + "
    " \ "
    " ) end From c28fc84c9049761f800e95fdb9a314a3dba7622d Mon Sep 17 00:00:00 2001 From: lynn Date: Thu, 7 May 2026 16:22:47 -0700 Subject: [PATCH 3/4] AO3-7414 Fix Rubocop offenses --- app/controllers/works_controller.rb | 3 ++- spec/controllers/works/works_controller_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 38bb2eebbcd..74ba1f41c4b 100755 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -539,7 +539,8 @@ def import_multiple(urls, options) unless failed_urls.empty? error_msgs = (0...failed_urls.length).map do |index| # each failed url may have multiple errors, so show them in a bulleted list underneath the url - errors_per_url = errors[index].map { |error| "
  • #{error}
    • " }.join("\n") + errors_per_url = errors[index].map { |error| "
  • #{error}
    • " } + .join("\n") "
    #{failed_urls[index]}
      #{errors_per_url}
    " end.join("\n") flash.now[:error] = "

    #{ts('Failed Imports')}

    #{error_msgs}
    ".html_safe diff --git a/spec/controllers/works/works_controller_spec.rb b/spec/controllers/works/works_controller_spec.rb index 6fe9e003ef7..bfd3a6d0e26 100644 --- a/spec/controllers/works/works_controller_spec.rb +++ b/spec/controllers/works/works_controller_spec.rb @@ -37,7 +37,7 @@ end let(:urls) { ["https://www.failedurl1.com", "https://www.failedurl2.com"] } - it 'returns a well-formatted error message' do + it "returns a well-formatted error message" do expect(controller).to receive(:render).with(:new_import) controller.send(:import_multiple, urls, {}) expect(flash[:error]).to eq( @@ -53,7 +53,7 @@ "
  • failedurl2 first error
    • " \ "" \ "
    " - ) + ) end end end From 3e4a5444e17e634656c448fc3cd54adaf4c2b788 Mon Sep 17 00:00:00 2001 From: lynn Date: Thu, 7 May 2026 16:28:08 -0700 Subject: [PATCH 4/4] AO3-7414 Fix Rubocop offenses --- app/controllers/works_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/works_controller.rb b/app/controllers/works_controller.rb index 74ba1f41c4b..7c629dbb6b4 100755 --- a/app/controllers/works_controller.rb +++ b/app/controllers/works_controller.rb @@ -540,7 +540,7 @@ def import_multiple(urls, options) error_msgs = (0...failed_urls.length).map do |index| # each failed url may have multiple errors, so show them in a bulleted list underneath the url errors_per_url = errors[index].map { |error| "
  • #{error}
    • " } - .join("\n") + .join("\n") "
    #{failed_urls[index]}
      #{errors_per_url}
    " end.join("\n") flash.now[:error] = "

    #{ts('Failed Imports')}

    #{error_msgs}
    ".html_safe