From 671cc0d5a0ac96394646429d48ae301384c3dc62 Mon Sep 17 00:00:00 2001 From: Robert Fletcher Date: Thu, 25 Jun 2026 10:07:50 -0700 Subject: [PATCH] Flake: make import a synchronous submit We're seeing some build flake when the JS hasn't attached by the time that we attach the file, so the auto-submit doesn't work. In this case, I think making it a standard form element works just as well, and maybe a bit more intuitively. --- .rubocop_todo.yml | 3 +- .../controllers/auto_submit_controller.ts | 7 ---- app/javascript/controllers/index.ts | 3 -- app/views/imports/new.html.erb | 5 ++- config/locales/en.yml | 1 + .../auto_submit_controller_spec.ts | 38 ------------------- spec/system/import_spec.rb | 3 ++ 7 files changed, 9 insertions(+), 51 deletions(-) delete mode 100644 app/javascript/controllers/auto_submit_controller.ts delete mode 100644 spec/javascript/controllers/auto_submit_controller_spec.ts diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index fb85e99a5..c195f601b 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -116,7 +116,7 @@ RSpec/DescribeClass: Exclude: - 'spec/integration/feed_importing_spec.rb' -# Offense count: 68 +# Offense count: 70 # Configuration parameters: Max, CountAsOne. RSpec/ExampleLength: Exclude: @@ -138,6 +138,7 @@ RSpec/ExampleLength: - 'spec/system/feed_show_spec.rb' - 'spec/system/feeds_index_spec.rb' - 'spec/system/good_job_spec.rb' + - 'spec/system/import_spec.rb' - 'spec/system/keyboard_shortcuts_spec.rb' - 'spec/system/starred_spec.rb' - 'spec/system/stories_index_spec.rb' diff --git a/app/javascript/controllers/auto_submit_controller.ts b/app/javascript/controllers/auto_submit_controller.ts deleted file mode 100644 index 4c91e6533..000000000 --- a/app/javascript/controllers/auto_submit_controller.ts +++ /dev/null @@ -1,7 +0,0 @@ -import {Controller} from "@hotwired/stimulus"; - -export default class extends Controller { - submit(): void { - this.element.requestSubmit(); - } -} diff --git a/app/javascript/controllers/index.ts b/app/javascript/controllers/index.ts index 18b84d38d..6a53fd8da 100644 --- a/app/javascript/controllers/index.ts +++ b/app/javascript/controllers/index.ts @@ -6,9 +6,6 @@ import {application} from "./application"; -import AutoSubmitController from "./auto_submit_controller"; -application.register("auto-submit", AutoSubmitController); - import DialogController from "./dialog_controller"; application.register("dialog", DialogController); diff --git a/app/views/imports/new.html.erb b/app/views/imports/new.html.erb index 05fa39546..edcba8d79 100644 --- a/app/views/imports/new.html.erb +++ b/app/views/imports/new.html.erb @@ -14,8 +14,9 @@

- <%= form_with(url: "/feeds/import", id: "import", multipart: true, data: { controller: "auto-submit" }) do %> - + <%= form_with(url: "/feeds/import", id: "import", multipart: true) do %> + + <%= submit_tag(t('import.fields.import'), class: "btn btn-primary") %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 80cf7c65c..52f415fca 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -60,6 +60,7 @@ en: import: description: Import your feeds from another service. fields: + choose_file: Choose file import: Import not_now: Not now subtitle: Let's setup your feeds. diff --git a/spec/javascript/controllers/auto_submit_controller_spec.ts b/spec/javascript/controllers/auto_submit_controller_spec.ts deleted file mode 100644 index 15ede5ea5..000000000 --- a/spec/javascript/controllers/auto_submit_controller_spec.ts +++ /dev/null @@ -1,38 +0,0 @@ -import {bootStimulus, getController} from "support/stimulus"; -import Controller from "controllers/auto_submit_controller"; -import {assert} from "helpers/assert"; - -function setupDOM(): void { - document.body.innerHTML = ` -
- -
- `; -} - -async function setupController(): Promise { - setupDOM(); - - await bootStimulus("auto-submit", Controller); -} - -function element(): HTMLFormElement { - const selector = "[data-controller='auto-submit']"; - - return assert(document.querySelector(selector)); -} - -function controller(): Controller { - return getController(element(), "auto-submit", Controller); -} - -describe("submit", () => { - it("submits the form", async () => { - await setupController(); - const submitSpy = vi.spyOn(element(), "requestSubmit"); - - controller().submit(); - - expect(submitSpy).toHaveBeenCalledWith(); - }); -}); diff --git a/spec/system/import_spec.rb b/spec/system/import_spec.rb index 743fa584a..e8e4d7a00 100644 --- a/spec/system/import_spec.rb +++ b/spec/system/import_spec.rb @@ -16,6 +16,7 @@ file_path = Rails.root.join("spec/fixtures/feeds.opml") attach_file("opml_file", file_path, visible: false) + within("#import") { click_on("Import") } expect(page).to have_text("We're getting you some stories to read") end @@ -26,6 +27,7 @@ file_path = Rails.root.join("spec/fixtures/invalid.opml") attach_file("opml_file", file_path, visible: false) + within("#import") { click_on("Import") } expect(page).to have_text("We're getting you some stories to read") end @@ -33,6 +35,7 @@ def import_grouped_opml file_path = Rails.root.join("spec/fixtures/feeds.opml") attach_file("opml_file", file_path, visible: false) + within("#import") { click_on("Import") } expect(page).to have_text("We're getting you some stories to read") end