From bb90e1bd46d6b379c6deb0bf392af9d3531675ba Mon Sep 17 00:00:00 2001 From: Joe Cohen Date: Sat, 14 Dec 2024 15:53:13 -0800 Subject: [PATCH 01/37] Update InatImportJobTracker show with Stimulus 1st draft - Uses Stimulus controller to update stats 1/second - **Does not work**; element on page doesn't update, and there's an infinite loop --- .../controllers/inat-import-job_controller.js | 69 +++++++++++++++++++ app/models/inat_import_job_tracker.rb | 6 +- .../inat_import_job_trackers/show.html.erb | 20 +++++- 3 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 app/javascript/controllers/inat-import-job_controller.js diff --git a/app/javascript/controllers/inat-import-job_controller.js b/app/javascript/controllers/inat-import-job_controller.js new file mode 100644 index 0000000000..04bcc33606 --- /dev/null +++ b/app/javascript/controllers/inat-import-job_controller.js @@ -0,0 +1,69 @@ +import { Controller } from "@hotwired/stimulus" +import { get } from "@rails/request.js" // allows us to call `get` below + +// Updates the inat_import_job page with the current status of the import +// Connects to data-controller="inat-import-job" +export default class extends Controller { + static targets = ["status"] + + initialize() { + // wherever the AJAX is going is printed on the element as a data attribute, + // so it can be different per PDF. More importantly, it's in ruby so it can + // be different in development, test and production + this.intervalId = null + this.endpoint_url = this.element.dataset.endpoint + } + + connect() { + // Just a "sanity check" convention, so you can tell "is this thing on?" + this.element.dataset.stimulus = "inat-import-job-connected"; + this.status_id = this.element.dataset.status + + this.start_timer_sending_requests() + } + + // Clear any intervals when the controller is disconnected + disconnect() { + const stimulus = this.element.dataset.stimulus.split(" ") + if (stimulus.includes("inat-import-job-connected")) { + const idx = stimulus.indexOf("inat-import-job-connected") + stimulus.splice(idx, 1) + this.element.setAttribute("data-stimulus", stimulus.join(" ")) + } + if (this.intervalId != null) { + clearInterval(this.intervalId) + } + } + + // Every second, send a get request to find out the status import_job. + // NOTE: Can't call a class function from `setInterval` because it resets + // the context of `this` + start_timer_sending_requests() { + // Done: 4 + // NOTE: Can this be written without a magic number? + // Something like InatImport.states[:Done] + if (this.status_id != "4") { + // Set the intervalId to the interval so we can clear it later + this.intervalId = setInterval(async () => { + console.log("sending fetch request to " + this.endpoint_url) + const response = await get(this.endpoint_url, + { responseKind: "turbo-stream" }); + if (response.ok) { + // Turbo replaces the element in the page already + } else { + console.log(`got a ${response.status}`); + } + }, 1000); + } else { + // If the import is done, we can remove this Stimulus controller from the + // element and stop the timer. (NOTE: there may be other controllers.) + console.log("inat-import-job is done") + const controllers = this.element.dataset.controller.split(" ") + if (controllers.includes("inat-import-job")) { + const idx = controllers.indexOf("inat-import-job") + controllers.splice(idx, 1) + this.element.setAttribute("inat-import-job", controllers.join(" ")) + } + } + } +} diff --git a/app/models/inat_import_job_tracker.rb b/app/models/inat_import_job_tracker.rb index 754a36fe2c..66b910d292 100644 --- a/app/models/inat_import_job_tracker.rb +++ b/app/models/inat_import_job_tracker.rb @@ -4,7 +4,11 @@ # # == Attributes # -# inat_import:: the iNat import for the job +# inat_import:: the iNatImport for the job +# status:: the state of the iNat import of this tracker # class InatImportJobTracker < ApplicationRecord + def status + InatImport.find(inat_import).state + end end diff --git a/app/views/controllers/inat_import_job_trackers/show.html.erb b/app/views/controllers/inat_import_job_trackers/show.html.erb index c09e9934ef..66709f879f 100644 --- a/app/views/controllers/inat_import_job_trackers/show.html.erb +++ b/app/views/controllers/inat_import_job_trackers/show.html.erb @@ -1,13 +1,27 @@ <% -@container = :wide -add_page_title(:inat_import_tracker.t) -# add_tab_set(article_show_tabs(article: @article, user: @user)) + @container = :wide + add_page_title(:inat_import_tracker.t) + # add_tab_set(article_show_tabs(article: @article, user: @user)) %>

<%= :inat_import_tracker_status.t %>: <%= @inat_import.state %>

+ +

+ Via Stimulus: + <%= + tag.span(id: "status", class: "text-right", + data: { controller: "inat-import-job", + status: @tracker.status, + endpoint: inat_import_job_tracker_path(@tracker.id), + inat_import_job_target: "status" }) do + @tracker.status + end + %> +

+ <% if @inat_import.response_errors.present? %>

<%= :ERRORS.t %>: From e5303b3fad03b0c3f214a8cb98d33bff1f08b339 Mon Sep 17 00:00:00 2001 From: Joe Cohen Date: Sun, 15 Dec 2024 11:08:59 -0800 Subject: [PATCH 02/37] job_controller gets status as a string - `InatImportJobTracker.status` returns a tring describing the status, e.g. "Done" rather than an ordinal. - Misc changes to comments --- .../controllers/inat-import-job_controller.js | 12 +++--------- .../inat_import_job_trackers/show.html.erb | 12 ++++++------ 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/app/javascript/controllers/inat-import-job_controller.js b/app/javascript/controllers/inat-import-job_controller.js index 04bcc33606..d4aefc50f7 100644 --- a/app/javascript/controllers/inat-import-job_controller.js +++ b/app/javascript/controllers/inat-import-job_controller.js @@ -7,9 +7,6 @@ export default class extends Controller { static targets = ["status"] initialize() { - // wherever the AJAX is going is printed on the element as a data attribute, - // so it can be different per PDF. More importantly, it's in ruby so it can - // be different in development, test and production this.intervalId = null this.endpoint_url = this.element.dataset.endpoint } @@ -17,7 +14,7 @@ export default class extends Controller { connect() { // Just a "sanity check" convention, so you can tell "is this thing on?" this.element.dataset.stimulus = "inat-import-job-connected"; - this.status_id = this.element.dataset.status + this.status = this.element.dataset.status this.start_timer_sending_requests() } @@ -35,14 +32,11 @@ export default class extends Controller { } } - // Every second, send a get request to find out the status import_job. + // Every second, send a get request to find out the status of the import. // NOTE: Can't call a class function from `setInterval` because it resets // the context of `this` start_timer_sending_requests() { - // Done: 4 - // NOTE: Can this be written without a magic number? - // Something like InatImport.states[:Done] - if (this.status_id != "4") { + if (this.status != "Done") { // Set the intervalId to the interval so we can clear it later this.intervalId = setInterval(async () => { console.log("sending fetch request to " + this.endpoint_url) diff --git a/app/views/controllers/inat_import_job_trackers/show.html.erb b/app/views/controllers/inat_import_job_trackers/show.html.erb index 66709f879f..06bb4f50d0 100644 --- a/app/views/controllers/inat_import_job_trackers/show.html.erb +++ b/app/views/controllers/inat_import_job_trackers/show.html.erb @@ -13,12 +13,12 @@ Via Stimulus: <%= tag.span(id: "status", class: "text-right", - data: { controller: "inat-import-job", - status: @tracker.status, - endpoint: inat_import_job_tracker_path(@tracker.id), - inat_import_job_target: "status" }) do - @tracker.status - end + data: { controller: "inat-import-job", + status: @tracker.status, + endpoint: inat_import_job_tracker_path(@tracker.id), + inat_import_job_target: "status" }) do + @tracker.status + end %>

From 488b26b64cc63fd2fe5e2b862eff29dd344cb14d Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Tue, 17 Dec 2024 11:57:14 -0800 Subject: [PATCH 03/37] Add turbo response to trackers controller --- .../inat_import_job_trackers_controller.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/controllers/inat_import_job_trackers_controller.rb b/app/controllers/inat_import_job_trackers_controller.rb index a37139fd56..ff7d759ac0 100644 --- a/app/controllers/inat_import_job_trackers_controller.rb +++ b/app/controllers/inat_import_job_trackers_controller.rb @@ -6,5 +6,16 @@ class InatImportJobTrackersController < ApplicationController def show @tracker = InatImportJobTracker.find(params[:id]) @inat_import = InatImport.find(@tracker.inat_import) + + respond_to do |format| + format.html + format.turbo_stream { render_current_status } + end + end + + private + + def render_current_status + turbo_stream.update("status") { status } end end From a007b3ab36e358b584ca0fb48efbcd5349af0128 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Tue, 17 Dec 2024 12:28:39 -0800 Subject: [PATCH 04/37] Split controller to inat_imports/job_trackers Write turbo response and view Also change routes --- .../inat_import_job_trackers_controller.rb | 21 ------------------ .../inat_imports/job_trackers_controller.rb | 22 +++++++++++++++++++ app/controllers/inat_imports_controller.rb | 10 +++++++++ .../inat_imports/job_trackers/_status.erb | 1 + .../show.html.erb | 0 config/routes.rb | 4 +++- 6 files changed, 36 insertions(+), 22 deletions(-) delete mode 100644 app/controllers/inat_import_job_trackers_controller.rb create mode 100644 app/controllers/inat_imports/job_trackers_controller.rb create mode 100644 app/controllers/inat_imports_controller.rb create mode 100644 app/views/controllers/inat_imports/job_trackers/_status.erb rename app/views/controllers/{inat_import_job_trackers => inat_imports}/show.html.erb (100%) diff --git a/app/controllers/inat_import_job_trackers_controller.rb b/app/controllers/inat_import_job_trackers_controller.rb deleted file mode 100644 index ff7d759ac0..0000000000 --- a/app/controllers/inat_import_job_trackers_controller.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -class InatImportJobTrackersController < ApplicationController - before_action :login_required - - def show - @tracker = InatImportJobTracker.find(params[:id]) - @inat_import = InatImport.find(@tracker.inat_import) - - respond_to do |format| - format.html - format.turbo_stream { render_current_status } - end - end - - private - - def render_current_status - turbo_stream.update("status") { status } - end -end diff --git a/app/controllers/inat_imports/job_trackers_controller.rb b/app/controllers/inat_imports/job_trackers_controller.rb new file mode 100644 index 0000000000..d61a457fd3 --- /dev/null +++ b/app/controllers/inat_imports/job_trackers_controller.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module InatImports + class JobTrackersController < ApplicationController + before_action :login_required + + # This is only a Turbo endpoint updating the row of a particular tracker. + def show + return unless (@tracker = InatImportJobTracker.find(params[:id])) + + respond_to do |format| + format.turbo_stream do + render(turbo_stream: turbo_stream.update( + :status, # id of div to replace + partial: "inat_imports/job_trackers/status", + locals: { tracker: @tracker } + )) + end + end + end + end +end diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb new file mode 100644 index 0000000000..3107b00769 --- /dev/null +++ b/app/controllers/inat_imports_controller.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class InatImportsController < ApplicationController + before_action :login_required + + def show + @tracker = InatImportJobTracker.find(params[:id]) + @inat_import = InatImport.find(@tracker.inat_import) + end +end diff --git a/app/views/controllers/inat_imports/job_trackers/_status.erb b/app/views/controllers/inat_imports/job_trackers/_status.erb new file mode 100644 index 0000000000..bb0f99922c --- /dev/null +++ b/app/views/controllers/inat_imports/job_trackers/_status.erb @@ -0,0 +1 @@ +<%= tracker.status %> diff --git a/app/views/controllers/inat_import_job_trackers/show.html.erb b/app/views/controllers/inat_imports/show.html.erb similarity index 100% rename from app/views/controllers/inat_import_job_trackers/show.html.erb rename to app/views/controllers/inat_imports/show.html.erb diff --git a/config/routes.rb b/config/routes.rb index 58dca00723..8d7d73e8e7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -439,7 +439,9 @@ def route_actions_hash end end - resources :inat_import_job_trackers, only: [:show] + resources :inat_imports, only: [:show] do + resources :job_trackers, only: [:show] + end # ----- Info: no resources, just forms and pages ---------------------------- get("/info/how_to_help", to: "info#how_to_help") From 7e7c71e23dedadb8eb7626101c0a259719a88c1e Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Tue, 17 Dec 2024 12:38:11 -0800 Subject: [PATCH 05/37] Update the "endpoint" data sent to Stimulus, for the new controller route --- app/views/controllers/inat_imports/show.html.erb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index 06bb4f50d0..bac3eb972a 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -15,7 +15,9 @@ tag.span(id: "status", class: "text-right", data: { controller: "inat-import-job", status: @tracker.status, - endpoint: inat_import_job_tracker_path(@tracker.id), + endpoint: inat_import_job_tracker_path( + inat_import: @inat_import.id, id: @tracker.id + ), inat_import_job_target: "status" }) do @tracker.status end From 8097a5fbcc2e9198bf1bf4c51a19cde6568e4ed0 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Tue, 17 Dec 2024 12:53:18 -0800 Subject: [PATCH 06/37] Update stimulus contoller to update its internal `status` property after receiving Turbo response It's using the value of the innerHTML of the span for its `status`. This could be a data attribute of a replaced element, if we were replacing rather than updating. --- .../controllers/inat-import-job_controller.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/app/javascript/controllers/inat-import-job_controller.js b/app/javascript/controllers/inat-import-job_controller.js index d4aefc50f7..7c87b9e671 100644 --- a/app/javascript/controllers/inat-import-job_controller.js +++ b/app/javascript/controllers/inat-import-job_controller.js @@ -40,13 +40,17 @@ export default class extends Controller { // Set the intervalId to the interval so we can clear it later this.intervalId = setInterval(async () => { console.log("sending fetch request to " + this.endpoint_url) - const response = await get(this.endpoint_url, - { responseKind: "turbo-stream" }); + const response = await get(this.endpoint_url, + { responseKind: "turbo-stream" }); if (response.ok) { - // Turbo replaces the element in the page already + // Turbo updates the element in the page already, + // from the InatImport::JobTrackersController#show action } else { console.log(`got a ${response.status}`); } + // Update our status variable with the current status of the import, + // as printed by Turbo + this.status = this.element.innerHTML }, 1000); } else { // If the import is done, we can remove this Stimulus controller from the From 9cb8bae6bbe2d66c4b358c6952b60183b3452872 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Tue, 17 Dec 2024 19:49:56 -0800 Subject: [PATCH 07/37] Reconfigure tests according to controller namespacing --- .../job_trackers_controller_test.rb | 18 ++++++++++++++++++ ...test.rb => inat_imports_controller_test.rb} | 2 +- 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 test/controllers/inat_imports/job_trackers_controller_test.rb rename test/controllers/{inat_import_job_trackers_controller_test.rb => inat_imports_controller_test.rb} (88%) diff --git a/test/controllers/inat_imports/job_trackers_controller_test.rb b/test/controllers/inat_imports/job_trackers_controller_test.rb new file mode 100644 index 0000000000..009b389b3e --- /dev/null +++ b/test/controllers/inat_imports/job_trackers_controller_test.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require "test_helper" + +module InatImports + class JobTrackersControllerTest < FunctionalTestCase + # test that it shows the tracker status. + def test_show + import = inat_imports(:rolf_inat_import) + tracker = InatImportJobTracker.create(inat_import: import.id) + + login + get(:show, params: { inat_import_id: import.id, id: tracker.id }) + assert_response(:success) + # maybe check the text + end + end +end diff --git a/test/controllers/inat_import_job_trackers_controller_test.rb b/test/controllers/inat_imports_controller_test.rb similarity index 88% rename from test/controllers/inat_import_job_trackers_controller_test.rb rename to test/controllers/inat_imports_controller_test.rb index 3e3f6d526a..6a4fb5ea42 100644 --- a/test/controllers/inat_import_job_trackers_controller_test.rb +++ b/test/controllers/inat_imports_controller_test.rb @@ -2,7 +2,7 @@ require "test_helper" -class InatImportJobTrackersControllerTest < FunctionalTestCase +class InatImportsControllerTest < FunctionalTestCase def test_show import = inat_imports(:rolf_inat_import) tracker = InatImportJobTracker.create(inat_import: import.id) From 10a715d2cc593b7b5c5f47e522cd84057d4953f4 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Tue, 17 Dec 2024 19:51:23 -0800 Subject: [PATCH 08/37] Fix namespacing of PageParser --- app/classes/inat/page_parser.rb | 66 +++++++++++++++++++++++++++++++++ app/classes/inat_page_parser.rb | 64 -------------------------------- app/jobs/inat_import_job.rb | 2 +- 3 files changed, 67 insertions(+), 65 deletions(-) create mode 100644 app/classes/inat/page_parser.rb delete mode 100644 app/classes/inat_page_parser.rb diff --git a/app/classes/inat/page_parser.rb b/app/classes/inat/page_parser.rb new file mode 100644 index 0000000000..d192be64c5 --- /dev/null +++ b/app/classes/inat/page_parser.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +class Inat + class PageParser + attr_accessor :last_import_id + + # The iNat API + API_BASE = Observations::InatImportsController::API_BASE + # limit results iNat API requests, with Protozoa as a proxy for slime molds + ICONIC_TAXA = "Fungi,Protozoa" + + def initialize(importer, ids, restricted_user_login) + @importer = importer + @last_import_id = 0 + @ids = ids + @restricted_user_login = restricted_user_login + end + + # Get one page of observations (up to 200) + # This is where we actually hit the iNat API + # https://api.inaturalist.org/v1/docs/#!/Observations/get_observations + # https://stackoverflow.com/a/11251654/3357635 + # NOTE: The `ids` parameter may be a comma-separated list of iNat obs + # ids - that needs to be URL encoded to a string when passed as an arg here + # because URI.encode_www_form deals with arrays by passing the same key + # multiple times. + def next_page + result = next_request(id: @ids, id_above: @last_import_id, + user_login: @restricted_user_login) + return nil if response_bad?(result) + + JSON.parse(result) + end + + private + + def response_bad?(response) + response.is_a?(RestClient::RequestFailed) || + response.instance_of?(RestClient::Response) && response.code != 200 || + # RestClient was happy, but the user wasn't authorized + response.is_a?(Hash) && response[:status] == 401 + end + + def next_request(**args) + query_args = { + id: nil, id_above: nil, only_id: false, per_page: 200, + order: "asc", order_by: "id", + # obss of only the iNat user with iNat login @inat_import.inat_username + user_login: nil, + iconic_taxa: ICONIC_TAXA + }.merge(args) + query = URI.encode_www_form(query_args) + + # ::Inat.new(operation: query, token: @inat_import.token).body + # Nimmo 2024-06-19 jdc. Moving the request from the inat class to here. + # RestClient::Request.execute wasn't available in the class + headers = { authorization: "Bearer #{@importer.token}", accept: :json } + RestClient::Request.execute( + method: :get, url: "#{API_BASE}/observations?#{query}", headers: headers + ) + rescue RestClient::ExceptionWithResponse => e + @importer.add_response_error(e.response) + e.response + end + end +end diff --git a/app/classes/inat_page_parser.rb b/app/classes/inat_page_parser.rb deleted file mode 100644 index 4118385fee..0000000000 --- a/app/classes/inat_page_parser.rb +++ /dev/null @@ -1,64 +0,0 @@ -# frozen_string_literal: true - -class InatPageParser - attr_accessor :last_import_id - - # The iNat API - API_BASE = Observations::InatImportsController::API_BASE - # limit results iNat API requests, with Protozoa as a proxy for slime molds - ICONIC_TAXA = "Fungi,Protozoa" - - def initialize(importer, ids, restricted_user_login) - @importer = importer - @last_import_id = 0 - @ids = ids - @restricted_user_login = restricted_user_login - end - - # Get one page of observations (up to 200) - # This is where we actually hit the iNat API - # https://api.inaturalist.org/v1/docs/#!/Observations/get_observations - # https://stackoverflow.com/a/11251654/3357635 - # NOTE: The `ids` parameter may be a comma-separated list of iNat obs - # ids - that needs to be URL encoded to a string when passed as an arg here - # because URI.encode_www_form deals with arrays by passing the same key - # multiple times. - def next_page - result = next_request(id: @ids, id_above: @last_import_id, - user_login: @restricted_user_login) - return nil if response_bad?(result) - - JSON.parse(result) - end - - private - - def response_bad?(response) - response.is_a?(RestClient::RequestFailed) || - response.instance_of?(RestClient::Response) && response.code != 200 || - # RestClient was happy, but the user wasn't authorized - response.is_a?(Hash) && response[:status] == 401 - end - - def next_request(**args) - query_args = { - id: nil, id_above: nil, only_id: false, per_page: 200, - order: "asc", order_by: "id", - # obss of only the iNat user with iNat login @inat_import.inat_username - user_login: nil, - iconic_taxa: ICONIC_TAXA - }.merge(args) - query = URI.encode_www_form(query_args) - - # ::Inat.new(operation: query, token: @inat_import.token).body - # Nimmo 2024-06-19 jdc. Moving the request from the inat class to here. - # RestClient::Request.execute wasn't available in the class - headers = { authorization: "Bearer #{@importer.token}", accept: :json } - RestClient::Request.execute( - method: :get, url: "#{API_BASE}/observations?#{query}", headers: headers - ) - rescue RestClient::ExceptionWithResponse => e - @importer.add_response_error(e.response) - e.response - end -end diff --git a/app/jobs/inat_import_job.rb b/app/jobs/inat_import_job.rb index 32d7dd91e0..b9df72a72f 100644 --- a/app/jobs/inat_import_job.rb +++ b/app/jobs/inat_import_job.rb @@ -121,7 +121,7 @@ def import_requested_observations # Search for one page of results at a time, until done with all pages # To get one page, use iNats `per_page` & `id_above` params. # https://api.inaturalist.org/v1/docs/#!/Observations/get_observations - parser = InatPageParser.new(@inat_import, inat_ids, restricted_user_login) + parser = Inat::PageParser.new(@inat_import, inat_ids, restricted_user_login) while parsing(parser); end end From 7325ce6818ed58fd51e1c7e70f26366ea757515e Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Tue, 17 Dec 2024 21:14:28 -0800 Subject: [PATCH 09/37] Resolve namespacing in a hopefully easier way --- app/classes/inat/page_parser.rb | 2 +- app/controllers/inat_imports_controller.rb | 115 +++++++ .../validators.rb} | 2 +- .../observations/inat_imports_controller.rb | 122 ------- app/helpers/tabs/observations_helper.rb | 2 +- app/jobs/inat_import_job.rb | 8 +- .../inat_imports/authenticate.html.erb | 0 .../inat_imports/new.html.erb | 6 +- .../controllers/inat_imports/show.html.erb | 26 +- config/routes.rb | 10 +- .../job_trackers_controller_test.rb | 17 +- .../inat_imports_controller_test.rb | 293 +++++++++++++++++ .../inat_imports_controller_test.rb | 300 ------------------ .../observations_controller_create_test.rb | 2 +- .../capybara/inat_imports_integration_test.rb | 2 +- test/jobs/inat_import_job_test.rb | 6 +- 16 files changed, 450 insertions(+), 463 deletions(-) rename app/controllers/{observations/inat_imports_controller_validators.rb => inat_imports_controller/validators.rb} (97%) delete mode 100644 app/controllers/observations/inat_imports_controller.rb rename app/views/controllers/{observations => }/inat_imports/authenticate.html.erb (100%) rename app/views/controllers/{observations => }/inat_imports/new.html.erb (82%) delete mode 100644 test/controllers/observations/inat_imports_controller_test.rb diff --git a/app/classes/inat/page_parser.rb b/app/classes/inat/page_parser.rb index d192be64c5..b601960f0e 100644 --- a/app/classes/inat/page_parser.rb +++ b/app/classes/inat/page_parser.rb @@ -5,7 +5,7 @@ class PageParser attr_accessor :last_import_id # The iNat API - API_BASE = Observations::InatImportsController::API_BASE + API_BASE = InatImportsController::API_BASE # limit results iNat API requests, with Protozoa as a proxy for slime molds ICONIC_TAXA = "Fungi,Protozoa" diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index 3107b00769..4f7b503dd9 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -1,10 +1,125 @@ # frozen_string_literal: true +# import iNaturalist Observations as MO Observations +# Actions +# ------- +# new (get) +# create (post) +# authorization_response (get) +# +# Work flow: +# 1. User calls `new`, fills out form +# 2. create +# saves some user data in a InatImport instance +# attributes include: user, inat_ids, token, state +# passes things off (redirects) to iNat at the inat_authorization_url +# 3. iNat +# checks if MO is authorized to access iNat user's confidential data +# if not, asks iNat user for authorization +# iNat calls the MO redirect_url (authorization_response) with a code param +# 4. MO continues in the authorization_response action +# Reads the saved InatImport instance +# Updates the InatImport instance with the code received from iNat +# Enqueues an InatImportJob, passing in the InatImport instance +# 5. The rest happens in the background. The InatImportJob: +# Uses the `code` to obtain an oauth access_token +# Trades the oauth token for a JWT api_token +# Checks if the MO user is trying to import some else's obss +# Makes an authenticated iNat API request for the desired observations +# For each iNat obs in the results, +# creates an Inat::Obs +# adds an MO Observation, mapping Inat::Obs details to the MO Obs +# adds Inat photos to the MO Observation via the MO API +# maps iNat sequences to MO Sequences +# adds an MO Comment with a snapshot of the imported data +# updates the iNat obs with a Mushroom Observer URL Observation Field +# updates the iNat obs Notes +# class InatImportsController < ApplicationController + include Validators + before_action :login_required + before_action :pass_query_params def show @tracker = InatImportJobTracker.find(params[:id]) @inat_import = InatImport.find(@tracker.inat_import) end + + # Site for authorization and authentication requests + SITE = "https://www.inaturalist.org" + # iNat calls this after iNat user authorizes MO to access their data. + REDIRECT_URI = Rails.configuration.redirect_uri + # iNat's id for the MO application + APP_ID = Rails.application.credentials.inat.id + # The iNat API. Not called here, but referenced in tests and ActiveJob + API_BASE = "https://api.inaturalist.org/v1" + + def new; end + + def create + return reload_form unless params_valid? + + @inat_import = InatImport.find_or_create_by(user: User.current) + @inat_import.update(state: "Authorizing", + import_all: params[:all], + importables: 0, imported_count: 0, + inat_ids: params[:inat_ids], + inat_username: params[:inat_username].strip, + response_errors: "", token: "", log: []) + + request_inat_user_authorization + end + + # --------------------------------- + + private + + def reload_form + @inat_ids = params[:inat_ids] + @inat_username = params[:inat_username] + render(:new) + end + + def request_inat_user_authorization + redirect_to(inat_authorization_url, allow_other_host: true) + end + + def inat_authorization_url + "#{SITE}/oauth/authorize" \ + "?client_id=#{APP_ID}" \ + "&redirect_uri=#{REDIRECT_URI}" \ + "&response_type=code" + end + + # --------------------------------- + + public + + # iNat redirects here after user completes iNat authorization + def authorization_response + auth_code = params[:code] + return not_authorized if auth_code.blank? + + @inat_import = InatImport.find_or_create_by(user: User.current) + @inat_import.update(token: auth_code, state: "Authenticating") + InatImportJobTracker.create(inat_import: @inat_import.id) + + Rails.logger.info( + "Enqueuing InatImportJob for InatImport id: #{@inat_import.id}" + ) + # InatImportJob.perform_now(@inat_import) # for manual testing + InatImportJob.perform_later(@inat_import) + + redirect_to(inat_import_path(@inat_import.id)) + end + + # --------------------------------- + + private + + def not_authorized + flash_error(:inat_no_authorization.l) + redirect_to(observations_path) + end end diff --git a/app/controllers/observations/inat_imports_controller_validators.rb b/app/controllers/inat_imports_controller/validators.rb similarity index 97% rename from app/controllers/observations/inat_imports_controller_validators.rb rename to app/controllers/inat_imports_controller/validators.rb index 9ff9a33860..2cd3f18249 100644 --- a/app/controllers/observations/inat_imports_controller_validators.rb +++ b/app/controllers/inat_imports_controller/validators.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module Observations::InatImportsControllerValidators +module InatImportsController::Validators private SITE = "https://www.inaturalist.org" diff --git a/app/controllers/observations/inat_imports_controller.rb b/app/controllers/observations/inat_imports_controller.rb deleted file mode 100644 index 8eb28d530e..0000000000 --- a/app/controllers/observations/inat_imports_controller.rb +++ /dev/null @@ -1,122 +0,0 @@ -# frozen_string_literal: true - -# import iNaturalist Observations as MO Observations -# Actions -# ------- -# new (get) -# create (post) -# authorization_response (get) -# -# Work flow: -# 1. User calls `new`, fills out form -# 2. create -# saves some user data in a InatImport instance -# attributes include: user, inat_ids, token, state -# passes things off (redirects) to iNat at the inat_authorization_url -# 3. iNat -# checks if MO is authorized to access iNat user's confidential data -# if not, asks iNat user for authorization -# iNat calls the MO redirect_url (authorization_response) with a code param -# 4. MO continues in the authorization_response action -# Reads the saved InatImport instance -# Updates the InatImport instance with the code received from iNat -# Enqueues an InatImportJob, passing in the InatImport instance -# 5. The rest happens in the background. The InatImportJob: -# Uses the `code` to obtain an oauth access_token -# Trades the oauth token for a JWT api_token -# Checks if the MO user is trying to import some else's obss -# Makes an authenticated iNat API request for the desired observations -# For each iNat obs in the results, -# creates an Inat::Obs -# adds an MO Observation, mapping Inat::Obs details to the MO Obs -# adds Inat photos to the MO Observation via the MO API -# maps iNat sequences to MO Sequences -# adds an MO Comment with a snapshot of the imported data -# updates the iNat obs with a Mushroom Observer URL Observation Field -# updates the iNat obs Notes -# -module Observations - class InatImportsController < ApplicationController - include InatImportsControllerValidators - - before_action :login_required - before_action :pass_query_params - - # Site for authorization and authentication requests - SITE = "https://www.inaturalist.org" - # iNat calls this after iNat user authorizes MO to access their data. - REDIRECT_URI = Rails.configuration.redirect_uri - # iNat's id for the MO application - APP_ID = Rails.application.credentials.inat.id - # The iNat API. Not called here, but referenced in tests and ActiveJob - API_BASE = "https://api.inaturalist.org/v1" - - def new; end - - def create - return reload_form unless params_valid? - - @inat_import = InatImport.find_or_create_by(user: User.current) - @inat_import.update(state: "Authorizing", - import_all: params[:all], - importables: 0, imported_count: 0, - inat_ids: params[:inat_ids], - inat_username: params[:inat_username].strip, - response_errors: "", token: "", log: []) - - request_inat_user_authorization - end - - # --------------------------------- - - private - - def reload_form - @inat_ids = params[:inat_ids] - @inat_username = params[:inat_username] - render(:new) - end - - def request_inat_user_authorization - redirect_to(inat_authorization_url, allow_other_host: true) - end - - def inat_authorization_url - "#{SITE}/oauth/authorize" \ - "?client_id=#{APP_ID}" \ - "&redirect_uri=#{REDIRECT_URI}" \ - "&response_type=code" - end - - # --------------------------------- - - public - - # iNat redirects here after user completes iNat authorization - def authorization_response - auth_code = params[:code] - return not_authorized if auth_code.blank? - - @inat_import = InatImport.find_or_create_by(user: User.current) - @inat_import.update(token: auth_code, state: "Authenticating") - tracker = InatImportJobTracker.create(inat_import: @inat_import.id) - - Rails.logger.info( - "Enqueuing InatImportJob for InatImport id: #{@inat_import.id}" - ) - # InatImportJob.perform_now(@inat_import) # for manual testing - InatImportJob.perform_later(@inat_import) - - redirect_to(inat_import_job_tracker_path(tracker.id)) - end - - # --------------------------------- - - private - - def not_authorized - flash_error(:inat_no_authorization.l) - redirect_to(observations_path) - end - end -end diff --git a/app/helpers/tabs/observations_helper.rb b/app/helpers/tabs/observations_helper.rb index b7ef8349cb..893ba402c4 100644 --- a/app/helpers/tabs/observations_helper.rb +++ b/app/helpers/tabs/observations_helper.rb @@ -214,7 +214,7 @@ def observation_maps_tabs(query:) def new_inat_import_tab(query: nil) [:create_observation_inat_import_link.l, - add_query_param(new_observations_inat_import_path, query), + add_query_param(new_inat_import_path, query), { class: tab_id(__method__.to_s) }] end diff --git a/app/jobs/inat_import_job.rb b/app/jobs/inat_import_job.rb index b9df72a72f..358a4d6a86 100644 --- a/app/jobs/inat_import_job.rb +++ b/app/jobs/inat_import_job.rb @@ -2,13 +2,13 @@ class InatImportJob < ApplicationJob # iNat's id for the MO application - APP_ID = Observations::InatImportsController::APP_ID + APP_ID = InatImportsController::APP_ID # site for authorization, authentication - SITE = Observations::InatImportsController::SITE + SITE = InatImportsController::SITE # iNat calls this after iNat user authorizes MO access to user's data - REDIRECT_URI = Observations::InatImportsController::REDIRECT_URI + REDIRECT_URI = InatImportsController::REDIRECT_URI # The iNat API - API_BASE = Observations::InatImportsController::API_BASE + API_BASE = InatImportsController::API_BASE # limit results iNat API requests, with Protozoa as a proxy for slime molds ICONIC_TAXA = "Fungi,Protozoa" # This string + date is added to description of iNat observation diff --git a/app/views/controllers/observations/inat_imports/authenticate.html.erb b/app/views/controllers/inat_imports/authenticate.html.erb similarity index 100% rename from app/views/controllers/observations/inat_imports/authenticate.html.erb rename to app/views/controllers/inat_imports/authenticate.html.erb diff --git a/app/views/controllers/observations/inat_imports/new.html.erb b/app/views/controllers/inat_imports/new.html.erb similarity index 82% rename from app/views/controllers/observations/inat_imports/new.html.erb rename to app/views/controllers/inat_imports/new.html.erb index 50728d7f17..db9a9d7c8a 100644 --- a/app/views/controllers/observations/inat_imports/new.html.erb +++ b/app/views/controllers/inat_imports/new.html.erb @@ -3,15 +3,15 @@ %>
- <%= form_with url: observations_inat_imports_path, method: :post do |f| %> - <%= f.label :inat_username, "#{:inat_username.l}: " %> + <%= form_with(url: inat_imports_path, method: :post) do |f| %> + <%= f.label(:inat_username, "#{:inat_username.l}: ") %> <%= f.text_field(:inat_username, size: 10, class: "form-control", value: User.current.inat_username) %> <%= panel_block(id: "choose_observation", heading: :inat_choose_observations.l, class: "name-section") do %> - <%= f.label :inat_id, "#{:inat_import_list.l}: " %> + <%= f.label(:inat_id, "#{:inat_import_list.l}: ") %> <%= f.text_field(:inat_ids, size: 10, value: @inat_ids, class: "form-control") %> <%#= check_box_with_label(form: f, field: :all, diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index ce64dc1865..eb53b3327c 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -16,7 +16,7 @@ data: { controller: "inat-import-job", status: @tracker.status, endpoint: inat_import_job_tracker_path( - inat_import: @inat_import.id, id: @tracker.id + inat_import_id: @inat_import.id, id: @tracker.id ), inat_import_job_target: "status" }) do @tracker.status @@ -25,14 +25,14 @@

<% if @inat_import.response_errors.present? %> -

- <%= :ERRORS.t %>: - <%= tag.span(id: "errors", class: "violation-highlight") do %> - <% @inat_import.response_errors.each_line do |error| %> - <%= error %>
- <% end %> - <% end %> -

+

+ <%= :ERRORS.t %>: + <%= tag.span(id: "errors", class: "violation-highlight") do %> + <% @inat_import.response_errors.each_line do |error| %> + <%= error %>
+ <% end %> + <% end %> +

<% end %>

@@ -42,11 +42,11 @@ <%# Prominently refresh button if job is incomplete%> <% if import_incomplete?(@inat_import) %> -

- <%= link_to(:inat_import_tracker_refresh.l, +

+ <%= link_to(:inat_import_tracker_refresh.l, inat_import_job_tracker_path(@tracker), { class: "btn btn-default" } ) %> -

+

<% end %>

@@ -61,7 +61,7 @@

<% if import_done?(@inat_import) %> - <%= link_to(:app_your_observations.l, + <%= link_to(:app_your_observations.l, observations_path(by_user: User.current.id), { class: "btn btn-default" } ) %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 1afb996792..85dc57f5ab 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -439,9 +439,13 @@ def route_actions_hash end end - resources :inat_imports, only: [:show] do + # ----- InatImports ---------------------------- + resources :inat_imports, only: [:show, :new, :create] do resources :job_trackers, only: [:show] end + get("inat_imports/authorization_response", + to: "inat_imports#authorization_response", + as: "inat_import_authorization_response") # ----- Info: no resources, just forms and pages ---------------------------- get("/info/how_to_help", to: "info#how_to_help") @@ -584,10 +588,6 @@ def route_actions_hash # ----- Observations: standard actions ---------------------------- namespace :observations do resources :downloads, only: [:new, :create] - resources :inat_imports, only: [:new, :create] - get("inat_imports/authorization_response", - to: "inat_imports#authorization_response", - as: "inat_import_authorization_response") # Not under resources :observations because the obs doesn't have an id yet get("images/uploads/new", to: "images/uploads#new", diff --git a/test/controllers/inat_imports/job_trackers_controller_test.rb b/test/controllers/inat_imports/job_trackers_controller_test.rb index 009b389b3e..c16042adce 100644 --- a/test/controllers/inat_imports/job_trackers_controller_test.rb +++ b/test/controllers/inat_imports/job_trackers_controller_test.rb @@ -5,14 +5,15 @@ module InatImports class JobTrackersControllerTest < FunctionalTestCase # test that it shows the tracker status. - def test_show - import = inat_imports(:rolf_inat_import) - tracker = InatImportJobTracker.create(inat_import: import.id) + # Would need an HTML response, though. (Can you test a turbo response?) + # def test_show + # import = inat_imports(:rolf_inat_import) + # tracker = InatImportJobTracker.create(inat_import: import.id) - login - get(:show, params: { inat_import_id: import.id, id: tracker.id }) - assert_response(:success) - # maybe check the text - end + # login + # get(:show, params: { inat_import_id: import.id, id: tracker.id }) + # assert_response(:success) + # # maybe check the text + # end end end diff --git a/test/controllers/inat_imports_controller_test.rb b/test/controllers/inat_imports_controller_test.rb index 6a4fb5ea42..4c601d3132 100644 --- a/test/controllers/inat_imports_controller_test.rb +++ b/test/controllers/inat_imports_controller_test.rb @@ -2,7 +2,25 @@ require "test_helper" +# a duck type of API2::ImageAPI with enough attributes +# to preventInatImportsController from throwing an error +class MockImageAPI + attr_reader :errors, :results + + def initialize(errors: [], results: []) + @errors = errors + @results = results + end +end + +# test importing iNaturalist Observations to Mushroom Observer class InatImportsControllerTest < FunctionalTestCase + include ActiveJob::TestHelper + + SITE = InatImportsController::SITE + REDIRECT_URI = InatImportsController::REDIRECT_URI + API_BASE = InatImportsController::API_BASE + def test_show import = inat_imports(:rolf_inat_import) tracker = InatImportJobTracker.create(inat_import: import.id) @@ -16,4 +34,279 @@ def test_show assert_select("span#imported_count", /^\d+$/, "Imported count should be an integer") end + + def test_new_inat_import + login(users(:rolf).login) + get(:new) + + assert_response(:success) + assert_form_action(action: :create) + assert_select("input#inat_ids", true, + "Form needs a field for inputting iNat ids") + assert_select("input#inat_username", true, + "Form needs a field for inputting iNat username") + assert_select("input[type=checkbox][id=consent]", true, + "Form needs checkbox requiring consent") + end + + def test_new_inat_import_inat_username_prefilled + user = users(:mary) + assert(user.inat_username.present?, + "Test needs a user fixture with an inat_username") + + login(mary.login) + get(:new) + + assert_select( + "input[name=?][value=?]", "inat_username", user.inat_username, true, + "InatImport should pre-fill `inat_username` with user's inat_username" + ) + end + + def test_create_missing_username + user = users(:rolf) + id = "123" + params = { inat_ids: id } + + login(user.login) + post(:create, params: params) + + assert_flash_text(:inat_missing_username.l) + assert_form_action(action: :create) + end + + def test_create_no_observations_designated + params = { inat_username: "anything", inat_ids: "", + consent: 1 } + login + assert_no_difference("Observation.count", + "Imported observation(s) though none designated") do + post(:create, params: params) + end + + assert_flash_text(:inat_no_imports_designated.l) + end + + def test_create_illegal_observation_id + params = { inat_username: "anything", inat_ids: "123*", + consent: 1 } + login + assert_no_difference("Observation.count", + "Imported observation(s) though none designated") do + post(:create, params: params) + end + + assert_flash_text(:runtime_illegal_inat_id.l) + end + + def test_create_no_consent + params = { inat_username: "anything", inat_ids: 123, + consent: 0 } + login + assert_no_difference("Observation.count", + "iNat obss imported without consent") do + post(:create, params: params) + end + + assert_flash_text(:inat_consent_required.l) + end + + def test_create_too_many_ids_listed + # generate an id list that's barely too long + id_list = "" + id = 1_234_567_890 + id_list += "#{id += 1}," until id_list.length > 255 + params = { inat_username: "anything", inat_ids: id_list, consent: 1 } + + login + post(:create, params: params) + + assert_form_action(action: :create) + assert_flash_text(:inat_too_many_ids_listed.l) + end + + def test_create_previously_imported + user = users(:rolf) + inat_id = "1123456" + Observation.create( + where: "North Falmouth, Massachusetts, USA", + user: user, + when: "2024-09-08", + source: Observation.sources[:mo_inat_import], + inat_id: inat_id + ) + + params = { inat_username: "anything", inat_ids: inat_id, + consent: 1 } + login + assert_no_difference("Observation.count", + "Imported a previously imported iNat obs") do + post(:create, params: params) + end + + # NOTE: 2024-09-04 jdc + # I'd prefer that the flash include links to both obss, + # and that this (or another) assertion check for that. + # At the moment, it's taking too long to figure out how. + assert_flash_text(/iNat #{inat_id} previously imported/) + end + + def test_create_previously_mirrored + user = users(:rolf) + inat_id = "1234567" + mirrored_obs = Observation.create( + where: "North Falmouth, Massachusetts, USA", + user: user, + when: "2023-09-08", + inat_id: nil, + # When Pulk's `mirror`Python script copies an MO Obs to iNat, + # it adds a text in this form to the MO Obs notes + # See https://github.com/JacobPulk/mirror + notes: { Other: "Mirrored on iNaturalist as observation #{inat_id} on December 18, 2023" } + ) + params = { inat_username: "anything", inat_ids: inat_id, consent: 1 } + + login + assert_no_difference( + "Observation.count", + "Imported an iNat obs which had been 'mirrored' from MO" + ) do + post(:create, params: params) + end + + # NOTE: 2024-09-04 jdc + # I'd prefer that the flash include links to both obss, + # and that this (or another) assertion check for that. + # At the moment, it's taking too long to figure out how. + assert_flash_text( + "iNat #{inat_id} is a “mirror” of " \ + "existing MO Observation #{mirrored_obs.id}" + ) + end + + def test_create_strip_inat_username + user = users(:rolf) + inat_username = " rolf " + inat_import = inat_imports(:rolf_inat_import) + assert_equal("Unstarted", inat_import.state, + "Need a Unstarted inat_import fixture") + + stub_request(:any, authorization_url) + login(user.login) + + assert_no_difference( + "Observation.count", + "Authorization request to iNat shouldn't create MO Observation(s)" + ) do + post(:create, + params: { inat_ids: 123_456_789, inat_username: inat_username, + consent: 1 }) + end + + assert_response(:redirect) + assert_equal( + "rolf", inat_import.reload.inat_username, + "It should strip leading/trailing whitespace from inat_username" + ) + end + + def test_create_authorization_request + user = users(:rolf) + inat_username = "rolf" + inat_import = inat_imports(:rolf_inat_import) + assert_equal("Unstarted", inat_import.state, + "Need a Unstarted inat_import fixture") + + stub_request(:any, authorization_url) + login(user.login) + + assert_no_difference( + "Observation.count", + "Authorization request to iNat shouldn't create MO Observation(s)" + ) do + post(:create, + params: { inat_ids: 123_456_789, inat_username: inat_username, + consent: 1 }) + end + + assert_response(:redirect) + assert_equal("Authorizing", inat_import.reload.state, + "MO should be awaiting authorization from iNat") + assert_equal(inat_username, inat_import.inat_username, + "Failed to save InatImport.inat_username") + end + + def test_authorization_response_denied + login + + get(:authorization_response, params: authorization_denial_callback_params) + + assert_redirected_to(observations_path) + assert_flash_error + end + + def test_import_authorized + user = users(:rolf) + assert_blank(user.inat_username, + "Test needs user fixture without an iNat username") + inat_import = inat_imports(:rolf_inat_import) + + # empty id_list to prevent importing any observations in this test + inat_import.inat_ids = "" + inat_import.save + inat_authorization_callback_params = { code: "MockCode" } + + login(user.login) + + assert_difference( + "enqueued_jobs.size", 1, "Failed to enqueue background job" + ) do + assert_enqueued_with(job: InatImportJob) do + get(:authorization_response, + params: inat_authorization_callback_params) + end + end + + # tracker = InatImportJobTracker.find_by(inat_import: inat_import) + assert_redirected_to(inat_import_path(inat_import.id)) + end + + def test_inat_username_unchanged_if_authorization_denied + user = users(:rolf) + assert_blank(user.inat_username, + "Test needs user fixture without an iNat username") + inat_username = "inat_username" + inat_import = inat_imports(:rolf_inat_import) + inat_import.update( + inat_ids: "", # Blank id_list ito prevent importing any observations + inat_username: inat_username + ) + + login(user.login) + get(:authorization_response, + params: authorization_denial_callback_params) + + assert_blank( + user.reload.inat_username, + "User inat_username shouldn't change if user denies authorization to MO" + ) + end + + ########## Utilities + + # iNat url where user is sent in order to authorize MO access + # to iNat confidential data + # https://www.inaturalist.org/pages/api+reference#authorization_code_flow + def authorization_url + "https://www.inaturalist.org/oauthenticate/authorize?" \ + "client_id=#{Rails.application.credentials.inat.id}" \ + "&redirect_uri=#{REDIRECT_URI}" \ + "&response_type=code" + end + + def authorization_denial_callback_params + { error: "access_denied", + error_description: + "The resource owner or authorization server denied the request." } + end end diff --git a/test/controllers/observations/inat_imports_controller_test.rb b/test/controllers/observations/inat_imports_controller_test.rb deleted file mode 100644 index 64b4e0eb45..0000000000 --- a/test/controllers/observations/inat_imports_controller_test.rb +++ /dev/null @@ -1,300 +0,0 @@ -# frozen_string_literal: true - -require "test_helper" - -# test importing iNaturalist Observations to Mushroom Observer -module Observations - # a duck type of API2::ImageAPI with enough attributes - # to preventInatImportsController from throwing an error - class MockImageAPI - attr_reader :errors, :results - - def initialize(errors: [], results: []) - @errors = errors - @results = results - end - end - - class InatImportsControllerTest < FunctionalTestCase - include ActiveJob::TestHelper - - SITE = Observations::InatImportsController::SITE - REDIRECT_URI = Observations::InatImportsController::REDIRECT_URI - API_BASE = Observations::InatImportsController::API_BASE - - def test_new_inat_import - login(users(:rolf).login) - get(:new) - - assert_response(:success) - assert_form_action(action: :create) - assert_select("input#inat_ids", true, - "Form needs a field for inputting iNat ids") - assert_select("input#inat_username", true, - "Form needs a field for inputting iNat username") - assert_select("input[type=checkbox][id=consent]", true, - "Form needs checkbox requiring consent") - end - - def test_new_inat_import_inat_username_prefilled - user = users(:mary) - assert(user.inat_username.present?, - "Test needs a user fixture with an inat_username") - - login(mary.login) - get(:new) - - assert_select( - "input[name=?][value=?]", "inat_username", user.inat_username, true, - "InatImport should pre-fill `inat_username` with user's inat_username" - ) - end - - def test_create_missing_username - user = users(:rolf) - id = "123" - params = { inat_ids: id } - - login(user.login) - post(:create, params: params) - - assert_flash_text(:inat_missing_username.l) - assert_form_action(action: :create) - end - - def test_create_no_observations_designated - params = { inat_username: "anything", inat_ids: "", - consent: 1 } - login - assert_no_difference("Observation.count", - "Imported observation(s) though none designated") do - post(:create, params: params) - end - - assert_flash_text(:inat_no_imports_designated.l) - end - - def test_create_illegal_observation_id - params = { inat_username: "anything", inat_ids: "123*", - consent: 1 } - login - assert_no_difference("Observation.count", - "Imported observation(s) though none designated") do - post(:create, params: params) - end - - assert_flash_text(:runtime_illegal_inat_id.l) - end - - def test_create_no_consent - params = { inat_username: "anything", inat_ids: 123, - consent: 0 } - login - assert_no_difference("Observation.count", - "iNat obss imported without consent") do - post(:create, params: params) - end - - assert_flash_text(:inat_consent_required.l) - end - - def test_create_too_many_ids_listed - # generate an id list that's barely too long - id_list = "" - id = 1_234_567_890 - id_list += "#{id += 1}," until id_list.length > 255 - params = { inat_username: "anything", inat_ids: id_list, consent: 1 } - - login - post(:create, params: params) - - assert_form_action(action: :create) - assert_flash_text(:inat_too_many_ids_listed.l) - end - - def test_create_previously_imported - user = users(:rolf) - inat_id = "1123456" - Observation.create( - where: "North Falmouth, Massachusetts, USA", - user: user, - when: "2024-09-08", - source: Observation.sources[:mo_inat_import], - inat_id: inat_id - ) - - params = { inat_username: "anything", inat_ids: inat_id, - consent: 1 } - login - assert_no_difference("Observation.count", - "Imported a previously imported iNat obs") do - post(:create, params: params) - end - - # NOTE: 2024-09-04 jdc - # I'd prefer that the flash include links to both obss, - # and that this (or another) assertion check for that. - # At the moment, it's taking too long to figure out how. - assert_flash_text(/iNat #{inat_id} previously imported/) - end - - def test_create_previously_mirrored - user = users(:rolf) - inat_id = "1234567" - mirrored_obs = Observation.create( - where: "North Falmouth, Massachusetts, USA", - user: user, - when: "2023-09-08", - inat_id: nil, - # When Pulk's `mirror`Python script copies an MO Obs to iNat, - # it adds a text in this form to the MO Obs notes - # See https://github.com/JacobPulk/mirror - notes: { Other: "Mirrored on iNaturalist as observation #{inat_id} on December 18, 2023" } - ) - params = { inat_username: "anything", inat_ids: inat_id, consent: 1 } - - login - assert_no_difference( - "Observation.count", - "Imported an iNat obs which had been 'mirrored' from MO" - ) do - post(:create, params: params) - end - - # NOTE: 2024-09-04 jdc - # I'd prefer that the flash include links to both obss, - # and that this (or another) assertion check for that. - # At the moment, it's taking too long to figure out how. - assert_flash_text( - "iNat #{inat_id} is a “mirror” of " \ - "existing MO Observation #{mirrored_obs.id}" - ) - end - - def test_create_strip_inat_username - user = users(:rolf) - inat_username = " rolf " - inat_import = inat_imports(:rolf_inat_import) - assert_equal("Unstarted", inat_import.state, - "Need a Unstarted inat_import fixture") - - stub_request(:any, authorization_url) - login(user.login) - - assert_no_difference( - "Observation.count", - "Authorization request to iNat shouldn't create MO Observation(s)" - ) do - post(:create, - params: { inat_ids: 123_456_789, inat_username: inat_username, - consent: 1 }) - end - - assert_response(:redirect) - assert_equal( - "rolf", inat_import.reload.inat_username, - "It should strip leading/trailing whitespace from inat_username" - ) - end - - def test_create_authorization_request - user = users(:rolf) - inat_username = "rolf" - inat_import = inat_imports(:rolf_inat_import) - assert_equal("Unstarted", inat_import.state, - "Need a Unstarted inat_import fixture") - - stub_request(:any, authorization_url) - login(user.login) - - assert_no_difference( - "Observation.count", - "Authorization request to iNat shouldn't create MO Observation(s)" - ) do - post(:create, - params: { inat_ids: 123_456_789, inat_username: inat_username, - consent: 1 }) - end - - assert_response(:redirect) - assert_equal("Authorizing", inat_import.reload.state, - "MO should be awaiting authorization from iNat") - assert_equal(inat_username, inat_import.inat_username, - "Failed to save InatImport.inat_username") - end - - def test_authorization_response_denied - login - - get(:authorization_response, params: authorization_denial_callback_params) - - assert_redirected_to(observations_path) - assert_flash_error - end - - def test_import_authorized - user = users(:rolf) - assert_blank(user.inat_username, - "Test needs user fixture without an iNat username") - inat_import = inat_imports(:rolf_inat_import) - - # empty id_list to prevent importing any observations in this test - inat_import.inat_ids = "" - inat_import.save - inat_authorization_callback_params = { code: "MockCode" } - - login(user.login) - - assert_difference( - "enqueued_jobs.size", 1, "Failed to enqueue background job" - ) do - assert_enqueued_with(job: InatImportJob) do - get(:authorization_response, - params: inat_authorization_callback_params) - end - end - - tracker = InatImportJobTracker.find_by(inat_import: inat_import) - assert_redirected_to(inat_import_job_tracker_path(tracker.id)) - end - - def test_inat_username_unchanged_if_authorization_denied - user = users(:rolf) - assert_blank(user.inat_username, - "Test needs user fixture without an iNat username") - inat_username = "inat_username" - inat_import = inat_imports(:rolf_inat_import) - inat_import.update( - inat_ids: "", # Blank id_list ito prevent importing any observations - inat_username: inat_username - ) - - login(user.login) - get(:authorization_response, - params: authorization_denial_callback_params) - - assert_blank( - user.reload.inat_username, - "User inat_username shouldn't change if user denies authorization to MO" - ) - end - - ########## Utilities - - # iNat url where user is sent in order to authorize MO access - # to iNat confidential data - # https://www.inaturalist.org/pages/api+reference#authorization_code_flow - def authorization_url - "https://www.inaturalist.org/oauthenticate/authorize?" \ - "client_id=#{Rails.application.credentials.inat.id}" \ - "&redirect_uri=#{REDIRECT_URI}" \ - "&response_type=code" - end - - def authorization_denial_callback_params - { error: "access_denied", - error_description: - "The resource owner or authorization server denied the request." } - end - end -end diff --git a/test/controllers/observations_controller/observations_controller_create_test.rb b/test/controllers/observations_controller/observations_controller_create_test.rb index f0a801adbb..1cdb6995ec 100644 --- a/test/controllers/observations_controller/observations_controller_create_test.rb +++ b/test/controllers/observations_controller/observations_controller_create_test.rb @@ -104,7 +104,7 @@ def test_create_new_observation assert_input_value(:herbarium_record_accession_number, "") assert_true(@response.body.include?("Albion, Mendocino Co., California")) assert_link_in_html(:create_observation_inat_import_link.l, - new_observations_inat_import_path) + new_inat_import_path) users(:rolf).update(location_format: "scientific") get(:new) diff --git a/test/integration/capybara/inat_imports_integration_test.rb b/test/integration/capybara/inat_imports_integration_test.rb index 654da60d64..5c2b746118 100644 --- a/test/integration/capybara/inat_imports_integration_test.rb +++ b/test/integration/capybara/inat_imports_integration_test.rb @@ -6,7 +6,7 @@ class InatImportsTest < CapybaraIntegrationTestCase def test_inat_import_no_imports_designated login(mary) - visit(new_observations_inat_import_path) + visit(new_inat_import_path) fill_in("inat_username", with: "anything") page.check("consent") diff --git a/test/jobs/inat_import_job_test.rb b/test/jobs/inat_import_job_test.rb index f8fbaf5bb8..c8991bbb18 100644 --- a/test/jobs/inat_import_job_test.rb +++ b/test/jobs/inat_import_job_test.rb @@ -25,9 +25,9 @@ def initialize(api:) end class InatImportJobTest < ActiveJob::TestCase - SITE = Observations::InatImportsController::SITE - REDIRECT_URI = Observations::InatImportsController::REDIRECT_URI - API_BASE = Observations::InatImportsController::API_BASE + SITE = InatImportsController::SITE + REDIRECT_URI = InatImportsController::REDIRECT_URI + API_BASE = InatImportsController::API_BASE PHOTO_BASE = "https://inaturalist-open-data.s3.amazonaws.com/photos" ICONIC_TAXA = InatImportJob::ICONIC_TAXA From e6e804d9014a2f0aa81d5cd2c42357c8c4daba1c Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Tue, 17 Dec 2024 21:16:36 -0800 Subject: [PATCH 10/37] ::RestClient::Request --- app/classes/inat/page_parser.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/classes/inat/page_parser.rb b/app/classes/inat/page_parser.rb index b601960f0e..5f6638725c 100644 --- a/app/classes/inat/page_parser.rb +++ b/app/classes/inat/page_parser.rb @@ -35,8 +35,8 @@ def next_page private def response_bad?(response) - response.is_a?(RestClient::RequestFailed) || - response.instance_of?(RestClient::Response) && response.code != 200 || + response.is_a?(::RestClient::RequestFailed) || + response.instance_of?(::RestClient::Response) && response.code != 200 || # RestClient was happy, but the user wasn't authorized response.is_a?(Hash) && response[:status] == 401 end @@ -55,10 +55,10 @@ def next_request(**args) # Nimmo 2024-06-19 jdc. Moving the request from the inat class to here. # RestClient::Request.execute wasn't available in the class headers = { authorization: "Bearer #{@importer.token}", accept: :json } - RestClient::Request.execute( + ::RestClient::Request.execute( method: :get, url: "#{API_BASE}/observations?#{query}", headers: headers ) - rescue RestClient::ExceptionWithResponse => e + rescue ::RestClient::ExceptionWithResponse => e @importer.add_response_error(e.response) e.response end From b056e65286f83777e3f065be324f02a76576330d Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Wed, 18 Dec 2024 06:56:23 -0800 Subject: [PATCH 11/37] Add :INAT_IMPORTS translation --- config/locales/en.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/config/locales/en.txt b/config/locales/en.txt index 2dfa559084..9e81f2d841 100644 --- a/config/locales/en.txt +++ b/config/locales/en.txt @@ -3067,6 +3067,7 @@ inat_details_list: "- MO will list most iNat data and/or map it to analogous MO fields.\n- Coordinates are hidden from the public if iNat obscured them.\n- Location is a rectangle with an MO-style name, rather than the iNat Place name." # /inat_import_job_tracker + INAT_IMPORTS: iNat Imports # hidden header in top navbar INAT_IMPORT_JOB_TRACKERS: iNat Import Trackers inat_import_tracker: iNat Import Tracker inat_import_tracker_status: "[:STATUS]" From ac563accb5f618d545db6d204366fb9175cf3a38 Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Wed, 18 Dec 2024 07:14:39 -0800 Subject: [PATCH 12/37] Fix translation string --- config/locales/en.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/en.txt b/config/locales/en.txt index 9e81f2d841..ea77a1b436 100644 --- a/config/locales/en.txt +++ b/config/locales/en.txt @@ -3067,7 +3067,7 @@ inat_details_list: "- MO will list most iNat data and/or map it to analogous MO fields.\n- Coordinates are hidden from the public if iNat obscured them.\n- Location is a rectangle with an MO-style name, rather than the iNat Place name." # /inat_import_job_tracker - INAT_IMPORTS: iNat Imports # hidden header in top navbar + INAT_IMPORTS: iNat Imports INAT_IMPORT_JOB_TRACKERS: iNat Import Trackers inat_import_tracker: iNat Import Tracker inat_import_tracker_status: "[:STATUS]" From 2b2f21d7ace91034b7f21dd502ccd0199a2d585a Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Wed, 18 Dec 2024 09:37:15 -0800 Subject: [PATCH 13/37] update redirect_uri env variable - Conforms it to new inat_imports_controller --- config/environments/development.rb | 4 +++- config/environments/production.rb | 4 +++- config/environments/test.rb | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/config/environments/development.rb b/config/environments/development.rb index 8d87459d26..4058a72847 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -62,8 +62,10 @@ # REDIRECT_URI (Callback URL) # iNat calls this after iNat user authorizes MO to access their data. + # Must match the redirect_uri in the iNat application settings for iNat's + # MushroomObserver Test app https://www.inaturalist.org/oauth/applications/851 config.redirect_uri = - "http://localhost:3000/observations/inat_imports/authorization_response" + "http://localhost:3000/inat_imports/authorization_response" # ---------------------------- # Rails configuration. diff --git a/config/environments/production.rb b/config/environments/production.rb index 8b9f3c494a..d8ba962e75 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -45,8 +45,10 @@ # REDIRECT_URI (Callback URL) # iNat calls this after iNat user authorizes MO to access their data. + # Must match the redirect_uri in the iNat application settings for iNat's + # Mushroom Observer app https://www.inaturalist.org/oauth/applications/857 config.redirect_uri = - "https://mushroomobserver.org/observations/inat_imports/authorization_response" + "https://mushroomobserver.org/inat_imports/authorization_response" # Disable Mission Control default HTTP Basic Authentication because # we specify AdminController as the base class for Mission Control diff --git a/config/environments/test.rb b/config/environments/test.rb index 970458a21d..ae20dea38e 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -28,8 +28,10 @@ # REDIRECT_URI (Callback URL) # iNat calls this after iNat user authorizes MO to access their data. + # Must match the redirect_uri in the iNat application settings for iNat's + # MushroomObserver Test app https://www.inaturalist.org/oauth/applications/851 config.redirect_uri = - "http://localhost:3000/observations/inat_imports/authorization_response" + "http://localhost:3000/inat_imports/authorization_response" # ---------------------------- # Rails configuration. From 091135259fbef3b9b28f11253409947b91a40e25 Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Wed, 18 Dec 2024 13:40:07 -0800 Subject: [PATCH 14/37] enhance comment --- app/controllers/inat_imports_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index 4f7b503dd9..50a7cc8238 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -20,6 +20,7 @@ # 4. MO continues in the authorization_response action # Reads the saved InatImport instance # Updates the InatImport instance with the code received from iNat +# Instantiates an InatImportJobTracker, passing in the InatImport instance # Enqueues an InatImportJob, passing in the InatImport instance # 5. The rest happens in the background. The InatImportJob: # Uses the `code` to obtain an oauth access_token From c6ca984a181a0d2c735302996cc88ac397b691c2 Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Thu, 19 Dec 2024 03:43:55 -0800 Subject: [PATCH 15/37] Move method after constant definitions --- app/controllers/inat_imports_controller.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index 50a7cc8238..d6d2d05873 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -22,6 +22,7 @@ # Updates the InatImport instance with the code received from iNat # Instantiates an InatImportJobTracker, passing in the InatImport instance # Enqueues an InatImportJob, passing in the InatImport instance +# Redirects to InatImport show page for that InatImport instance # 5. The rest happens in the background. The InatImportJob: # Uses the `code` to obtain an oauth access_token # Trades the oauth token for a JWT api_token @@ -42,11 +43,6 @@ class InatImportsController < ApplicationController before_action :login_required before_action :pass_query_params - def show - @tracker = InatImportJobTracker.find(params[:id]) - @inat_import = InatImport.find(@tracker.inat_import) - end - # Site for authorization and authentication requests SITE = "https://www.inaturalist.org" # iNat calls this after iNat user authorizes MO to access their data. @@ -56,6 +52,11 @@ def show # The iNat API. Not called here, but referenced in tests and ActiveJob API_BASE = "https://api.inaturalist.org/v1" + def show + @tracker = InatImportJobTracker.find(params[:id]) + @inat_import = InatImport.find(@tracker.inat_import) + end + def new; end def create @@ -99,12 +100,14 @@ def inat_authorization_url # iNat redirects here after user completes iNat authorization def authorization_response + debugger auth_code = params[:code] return not_authorized if auth_code.blank? @inat_import = InatImport.find_or_create_by(user: User.current) @inat_import.update(token: auth_code, state: "Authenticating") - InatImportJobTracker.create(inat_import: @inat_import.id) + tracker = InatImportJobTracker.create(inat_import: @inat_import.id) + debugger Rails.logger.info( "Enqueuing InatImportJob for InatImport id: #{@inat_import.id}" From ab253539947bfba7ea6de2bdb94e77112f112455 Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Thu, 19 Dec 2024 06:20:04 -0800 Subject: [PATCH 16/37] Reorder inat_imports routes - Moves get("inat_imports/authorization_response") ahead of the resource routes (For reasons I don't understand `http://localhost:3000/inat_imports/authorization_response` was being routed to `InatImports#show`. --- app/controllers/inat_imports_controller.rb | 2 -- config/routes.rb | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index d6d2d05873..c7fd8a6a79 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -100,14 +100,12 @@ def inat_authorization_url # iNat redirects here after user completes iNat authorization def authorization_response - debugger auth_code = params[:code] return not_authorized if auth_code.blank? @inat_import = InatImport.find_or_create_by(user: User.current) @inat_import.update(token: auth_code, state: "Authenticating") tracker = InatImportJobTracker.create(inat_import: @inat_import.id) - debugger Rails.logger.info( "Enqueuing InatImportJob for InatImport id: #{@inat_import.id}" diff --git a/config/routes.rb b/config/routes.rb index 85dc57f5ab..7e0257df73 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -440,12 +440,12 @@ def route_actions_hash end # ----- InatImports ---------------------------- - resources :inat_imports, only: [:show, :new, :create] do - resources :job_trackers, only: [:show] - end get("inat_imports/authorization_response", to: "inat_imports#authorization_response", as: "inat_import_authorization_response") + resources :inat_imports, only: [:show, :new, :create] do + resources :job_trackers, only: [:show] + end # ----- Info: no resources, just forms and pages ---------------------------- get("/info/how_to_help", to: "info#how_to_help") From e10607c9706041aebf2d6fbbf42af1c69202f5c8 Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Fri, 20 Dec 2024 09:31:06 -0700 Subject: [PATCH 17/37] Update documentation comment --- app/controllers/inat_imports_controller.rb | 22 ++++++++++++++----- .../controllers/inat_imports/show.html.erb | 20 +++++++++-------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index c7fd8a6a79..8c87f0f616 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -22,8 +22,15 @@ # Updates the InatImport instance with the code received from iNat # Instantiates an InatImportJobTracker, passing in the InatImport instance # Enqueues an InatImportJob, passing in the InatImport instance -# Redirects to InatImport show page for that InatImport instance -# 5. The rest happens in the background. The InatImportJob: +# Redirects to InatImport show page for that InatImport instance, +# passing the JobTracker.id as the tracker_id param. +# 5 The InatImport show page (app/views/controllers/inat_imports/show.html.erb) +# instantiates a Stimulus controller (inat-import-job_controller.js) +# with an endpoint of InatImportJobTrackerController#show +# 6 Two things then proceed in parallel: +# A background InatImportJob +# The Stimulus controller updates the InatImport show page +# 6a The InatImportJob: # Uses the `code` to obtain an oauth access_token # Trades the oauth token for a JWT api_token # Checks if the MO user is trying to import some else's obss @@ -36,6 +43,9 @@ # adds an MO Comment with a snapshot of the imported data # updates the iNat obs with a Mushroom Observer URL Observation Field # updates the iNat obs Notes +# Updates the InatImport instance with status of the Job +# 6a The Stimulus controller regulary polls the InatImport instance and +# via a Turbo stream, updates the InatImport show page # class InatImportsController < ApplicationController include Validators @@ -53,7 +63,7 @@ class InatImportsController < ApplicationController API_BASE = "https://api.inaturalist.org/v1" def show - @tracker = InatImportJobTracker.find(params[:id]) + @tracker = InatImportJobTracker.find(params[:tracker_id]) @inat_import = InatImport.find(@tracker.inat_import) end @@ -112,8 +122,10 @@ def authorization_response ) # InatImportJob.perform_now(@inat_import) # for manual testing InatImportJob.perform_later(@inat_import) - - redirect_to(inat_import_path(@inat_import.id)) + redirect_to( + inat_import_path(@inat_import.id, + params: { tracker_id: tracker.id }) + ) end # --------------------------------- diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index eb53b3327c..96d04cb8eb 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -12,15 +12,17 @@

Via Stimulus: <%= - tag.span(id: "status", class: "text-right", - data: { controller: "inat-import-job", - status: @tracker.status, - endpoint: inat_import_job_tracker_path( - inat_import_id: @inat_import.id, id: @tracker.id - ), - inat_import_job_target: "status" }) do - @tracker.status - end + tag.span( + id: "status", class: "text-right", + data: { controller: "inat-import-job", + status: @tracker.status, + endpoint: inat_import_job_tracker_path( + inat_import_id: @inat_import.id, id: @tracker.id + ), + inat_import_job_target: "status" } + ) do + @tracker.status + end %>

From 135b5acdb28160fcacf73b674aefaadd103a5593 Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Fri, 20 Dec 2024 16:10:41 -0700 Subject: [PATCH 18/37] Revert "Reorder inat_imports routes" This reverts commit ab253539947bfba7ea6de2bdb94e77112f112455. --- app/controllers/inat_imports_controller.rb | 2 ++ config/routes.rb | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index c7fd8a6a79..d6d2d05873 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -100,12 +100,14 @@ def inat_authorization_url # iNat redirects here after user completes iNat authorization def authorization_response + debugger auth_code = params[:code] return not_authorized if auth_code.blank? @inat_import = InatImport.find_or_create_by(user: User.current) @inat_import.update(token: auth_code, state: "Authenticating") tracker = InatImportJobTracker.create(inat_import: @inat_import.id) + debugger Rails.logger.info( "Enqueuing InatImportJob for InatImport id: #{@inat_import.id}" diff --git a/config/routes.rb b/config/routes.rb index 7e0257df73..85dc57f5ab 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -440,12 +440,12 @@ def route_actions_hash end # ----- InatImports ---------------------------- - get("inat_imports/authorization_response", - to: "inat_imports#authorization_response", - as: "inat_import_authorization_response") resources :inat_imports, only: [:show, :new, :create] do resources :job_trackers, only: [:show] end + get("inat_imports/authorization_response", + to: "inat_imports#authorization_response", + as: "inat_import_authorization_response") # ----- Info: no resources, just forms and pages ---------------------------- get("/info/how_to_help", to: "info#how_to_help") From bbd60db2ec61d1b9632be98bff231f60808ba15d Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Fri, 20 Dec 2024 16:13:15 -0700 Subject: [PATCH 19/37] Reapply "Reorder inat_imports routes" This reverts commit 135b5acdb28160fcacf73b674aefaadd103a5593. --- app/controllers/inat_imports_controller.rb | 2 -- config/routes.rb | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index 83d12cfb23..8c87f0f616 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -110,14 +110,12 @@ def inat_authorization_url # iNat redirects here after user completes iNat authorization def authorization_response - debugger auth_code = params[:code] return not_authorized if auth_code.blank? @inat_import = InatImport.find_or_create_by(user: User.current) @inat_import.update(token: auth_code, state: "Authenticating") tracker = InatImportJobTracker.create(inat_import: @inat_import.id) - debugger Rails.logger.info( "Enqueuing InatImportJob for InatImport id: #{@inat_import.id}" diff --git a/config/routes.rb b/config/routes.rb index 85dc57f5ab..7e0257df73 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -440,12 +440,12 @@ def route_actions_hash end # ----- InatImports ---------------------------- - resources :inat_imports, only: [:show, :new, :create] do - resources :job_trackers, only: [:show] - end get("inat_imports/authorization_response", to: "inat_imports#authorization_response", as: "inat_import_authorization_response") + resources :inat_imports, only: [:show, :new, :create] do + resources :job_trackers, only: [:show] + end # ----- Info: no resources, just forms and pages ---------------------------- get("/info/how_to_help", to: "info#how_to_help") From 42d552a935b01374d25c871abe0c2682d64ad373 Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Fri, 20 Dec 2024 16:13:32 -0700 Subject: [PATCH 20/37] Revert "Update documentation comment" This reverts commit e10607c9706041aebf2d6fbbf42af1c69202f5c8. --- app/controllers/inat_imports_controller.rb | 22 +++++-------------- .../controllers/inat_imports/show.html.erb | 20 ++++++++--------- 2 files changed, 14 insertions(+), 28 deletions(-) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index 8c87f0f616..c7fd8a6a79 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -22,15 +22,8 @@ # Updates the InatImport instance with the code received from iNat # Instantiates an InatImportJobTracker, passing in the InatImport instance # Enqueues an InatImportJob, passing in the InatImport instance -# Redirects to InatImport show page for that InatImport instance, -# passing the JobTracker.id as the tracker_id param. -# 5 The InatImport show page (app/views/controllers/inat_imports/show.html.erb) -# instantiates a Stimulus controller (inat-import-job_controller.js) -# with an endpoint of InatImportJobTrackerController#show -# 6 Two things then proceed in parallel: -# A background InatImportJob -# The Stimulus controller updates the InatImport show page -# 6a The InatImportJob: +# Redirects to InatImport show page for that InatImport instance +# 5. The rest happens in the background. The InatImportJob: # Uses the `code` to obtain an oauth access_token # Trades the oauth token for a JWT api_token # Checks if the MO user is trying to import some else's obss @@ -43,9 +36,6 @@ # adds an MO Comment with a snapshot of the imported data # updates the iNat obs with a Mushroom Observer URL Observation Field # updates the iNat obs Notes -# Updates the InatImport instance with status of the Job -# 6a The Stimulus controller regulary polls the InatImport instance and -# via a Turbo stream, updates the InatImport show page # class InatImportsController < ApplicationController include Validators @@ -63,7 +53,7 @@ class InatImportsController < ApplicationController API_BASE = "https://api.inaturalist.org/v1" def show - @tracker = InatImportJobTracker.find(params[:tracker_id]) + @tracker = InatImportJobTracker.find(params[:id]) @inat_import = InatImport.find(@tracker.inat_import) end @@ -122,10 +112,8 @@ def authorization_response ) # InatImportJob.perform_now(@inat_import) # for manual testing InatImportJob.perform_later(@inat_import) - redirect_to( - inat_import_path(@inat_import.id, - params: { tracker_id: tracker.id }) - ) + + redirect_to(inat_import_path(@inat_import.id)) end # --------------------------------- diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index 96d04cb8eb..eb53b3327c 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -12,17 +12,15 @@

Via Stimulus: <%= - tag.span( - id: "status", class: "text-right", - data: { controller: "inat-import-job", - status: @tracker.status, - endpoint: inat_import_job_tracker_path( - inat_import_id: @inat_import.id, id: @tracker.id - ), - inat_import_job_target: "status" } - ) do - @tracker.status - end + tag.span(id: "status", class: "text-right", + data: { controller: "inat-import-job", + status: @tracker.status, + endpoint: inat_import_job_tracker_path( + inat_import_id: @inat_import.id, id: @tracker.id + ), + inat_import_job_target: "status" }) do + @tracker.status + end %>

From 7cd4b5b6e7699ae698956973d751c113c34fd2f3 Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Sat, 21 Dec 2024 04:58:01 -0700 Subject: [PATCH 21/37] Refactor span - Separates call to Stimulus controller from data replaces by Stimulus --- .../controllers/inat_imports/show.html.erb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index eb53b3327c..96c624f83e 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -12,15 +12,16 @@

Via Stimulus: <%= + # Outer span calls Stimulus controller, passing in the endpoint tag.span(id: "status", class: "text-right", - data: { controller: "inat-import-job", - status: @tracker.status, - endpoint: inat_import_job_tracker_path( - inat_import_id: @inat_import.id, id: @tracker.id - ), - inat_import_job_target: "status" }) do - @tracker.status - end + data: { controller: "inat-import-job", + endpoint: inat_import_job_tracker_path( + inat_import_id: @inat_import.id, id: @tracker.id + ) }) do + # Inner span is replaced by Stimulus + tag.span(@tracker.status, data: { inat_import_job_target: "status", + status: @tracker.status }) + end %>

From d42d7f51c3e9b8798f06cb619707f5faaba99486 Mon Sep 17 00:00:00 2001 From: andrew nimmo Date: Sat, 21 Dec 2024 12:56:41 -0800 Subject: [PATCH 22/37] Update routes.rb Add `module` arg to make it hit the namespaced controller `InatImports::JobTrackersController` --- config/routes.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/routes.rb b/config/routes.rb index 7e0257df73..8b4f10ee4e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -444,7 +444,7 @@ def route_actions_hash to: "inat_imports#authorization_response", as: "inat_import_authorization_response") resources :inat_imports, only: [:show, :new, :create] do - resources :job_trackers, only: [:show] + resources :job_trackers, only: [:show], module: :inat_imports end # ----- Info: no resources, just forms and pages ---------------------------- From 6ee8e40f8be752133950c57c9c11d2c579cbfb8d Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Sun, 22 Dec 2024 08:43:57 -0700 Subject: [PATCH 23/37] fix indentation --- app/views/controllers/inat_imports/show.html.erb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index 96c624f83e..f5abe3b696 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -14,10 +14,10 @@ <%= # Outer span calls Stimulus controller, passing in the endpoint tag.span(id: "status", class: "text-right", - data: { controller: "inat-import-job", - endpoint: inat_import_job_tracker_path( - inat_import_id: @inat_import.id, id: @tracker.id - ) }) do + data: { controller: "inat-import-job", + endpoint: inat_import_job_tracker_path( + inat_import_id: @inat_import.id, id: @tracker.id + ) }) do # Inner span is replaced by Stimulus tag.span(@tracker.status, data: { inat_import_job_target: "status", status: @tracker.status }) From 2a0324297f44aeab0bb854b5266b02b71e32f094 Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Sun, 22 Dec 2024 09:13:32 -0700 Subject: [PATCH 24/37] Edit comment summarizing entire process --- app/controllers/inat_imports_controller.rb | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index c7fd8a6a79..7caf768db7 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -22,8 +22,21 @@ # Updates the InatImport instance with the code received from iNat # Instantiates an InatImportJobTracker, passing in the InatImport instance # Enqueues an InatImportJob, passing in the InatImport instance -# Redirects to InatImport show page for that InatImport instance -# 5. The rest happens in the background. The InatImportJob: +# Redirects to InatImport.show (for that InatImport instance) +# --------------------------------- +# InatImport.show view: +# Includes a `#status` element which: +# Instantiates a Stimulus controller (inat-import-job_controller) +# with an endpoint of InatImportJobTracker.show +# is updated by a TurboStream response from the endpoint +# --------------------------------- +# Stimulus controller (inat-import-job_controller): +# Makes a request every second to the InatImportJobTracker.show endpoint +# --------------------------------- +# The InatImportJobTracker.show endpoint action: +# returns the status of the InatImport as a TurboStream response +# --------------------------------- +# 5. The InatImportJob: # Uses the `code` to obtain an oauth access_token # Trades the oauth token for a JWT api_token # Checks if the MO user is trying to import some else's obss @@ -36,6 +49,8 @@ # adds an MO Comment with a snapshot of the imported data # updates the iNat obs with a Mushroom Observer URL Observation Field # updates the iNat obs Notes +# updates the InatImport instance attributes: +# state, importables, imported_count, response_errors # class InatImportsController < ApplicationController include Validators From e6cb05e1fcfd72ba215d01bfdf818625c4c5019a Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Sun, 22 Dec 2024 09:15:38 -0700 Subject: [PATCH 25/37] Update Trackers comment - Change text that was copied from FieldSlipJobTrackers --- app/controllers/inat_imports/job_trackers_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/inat_imports/job_trackers_controller.rb b/app/controllers/inat_imports/job_trackers_controller.rb index d61a457fd3..e8679e3dae 100644 --- a/app/controllers/inat_imports/job_trackers_controller.rb +++ b/app/controllers/inat_imports/job_trackers_controller.rb @@ -4,14 +4,14 @@ module InatImports class JobTrackersController < ApplicationController before_action :login_required - # This is only a Turbo endpoint updating the row of a particular tracker. + # This is only a Turbo endpoint updating the the status of a job. def show return unless (@tracker = InatImportJobTracker.find(params[:id])) respond_to do |format| format.turbo_stream do render(turbo_stream: turbo_stream.update( - :status, # id of div to replace + :status, # id of element to replace partial: "inat_imports/job_trackers/status", locals: { tracker: @tracker } )) From 140c83f170b543436da74ea12f32a814ccaa9db0 Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Sun, 22 Dec 2024 10:28:41 -0700 Subject: [PATCH 26/37] Fix params passed to Import#show and Tracker#show --- app/controllers/inat_imports/job_trackers_controller.rb | 2 +- app/controllers/inat_imports_controller.rb | 6 ++++-- app/views/controllers/inat_imports/show.html.erb | 4 +--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/inat_imports/job_trackers_controller.rb b/app/controllers/inat_imports/job_trackers_controller.rb index e8679e3dae..104e59cb79 100644 --- a/app/controllers/inat_imports/job_trackers_controller.rb +++ b/app/controllers/inat_imports/job_trackers_controller.rb @@ -4,7 +4,7 @@ module InatImports class JobTrackersController < ApplicationController before_action :login_required - # This is only a Turbo endpoint updating the the status of a job. + # This is only a Turbo endpoint updating the display of the status of a job. def show return unless (@tracker = InatImportJobTracker.find(params[:id])) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index 7caf768db7..15718eb2ac 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -68,7 +68,7 @@ class InatImportsController < ApplicationController API_BASE = "https://api.inaturalist.org/v1" def show - @tracker = InatImportJobTracker.find(params[:id]) + @tracker = InatImportJobTracker.find(params[:tracker_id]) @inat_import = InatImport.find(@tracker.inat_import) end @@ -128,7 +128,9 @@ def authorization_response # InatImportJob.perform_now(@inat_import) # for manual testing InatImportJob.perform_later(@inat_import) - redirect_to(inat_import_path(@inat_import.id)) + # redirect_to(inat_import_path(@inat_import.id)) + redirect_to(inat_import_path(@inat_import, + params: { tracker_id: tracker.id })) end # --------------------------------- diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index f5abe3b696..a4362002fd 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -15,9 +15,7 @@ # Outer span calls Stimulus controller, passing in the endpoint tag.span(id: "status", class: "text-right", data: { controller: "inat-import-job", - endpoint: inat_import_job_tracker_path( - inat_import_id: @inat_import.id, id: @tracker.id - ) }) do + endpoint: inat_import_job_tracker_path(@inat_import, @tracker) }) do # Inner span is replaced by Stimulus tag.span(@tracker.status, data: { inat_import_job_target: "status", status: @tracker.status }) From e823c2e52be2593491316394b13b2dc7ce939b0f Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Sun, 22 Dec 2024 12:14:20 -0700 Subject: [PATCH 27/37] Add newly required param to test expectation - InatImport#show now needs a `tracker_id` param - Fixes failure in prior commit --- test/controllers/inat_imports_controller_test.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/controllers/inat_imports_controller_test.rb b/test/controllers/inat_imports_controller_test.rb index 4c601d3132..753385090d 100644 --- a/test/controllers/inat_imports_controller_test.rb +++ b/test/controllers/inat_imports_controller_test.rb @@ -267,8 +267,10 @@ def test_import_authorized end end - # tracker = InatImportJobTracker.find_by(inat_import: inat_import) - assert_redirected_to(inat_import_path(inat_import.id)) + tracker = InatImportJobTracker.where(inat_import: inat_import.id).last + assert_redirected_to( + inat_import_path(inat_import, params: { tracker_id: tracker.id }) + ) end def test_inat_username_unchanged_if_authorization_denied From ce04f8aec21c4b0829c81778d3401107d4824865 Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Sun, 22 Dec 2024 12:45:46 -0700 Subject: [PATCH 28/37] Fix test param to match new Import#show requirement --- test/controllers/inat_imports_controller_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/controllers/inat_imports_controller_test.rb b/test/controllers/inat_imports_controller_test.rb index 753385090d..3520136e08 100644 --- a/test/controllers/inat_imports_controller_test.rb +++ b/test/controllers/inat_imports_controller_test.rb @@ -26,7 +26,8 @@ def test_show tracker = InatImportJobTracker.create(inat_import: import.id) login - get(:show, params: { id: tracker.id }) + + get(:show, params: { id: import.id, tracker_id: tracker.id }) assert_response(:success) assert_select("span#importables_count", /^\d+$/, From 6ecd082c4cae75a5faa76c877caae0491dd382bd Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Sun, 22 Dec 2024 15:38:10 -0700 Subject: [PATCH 29/37] Local vars instead of ivars in authorization_response - Don't need ivars because it redirects instead of rendering --- app/controllers/inat_imports_controller.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index 15718eb2ac..c63aa29717 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -118,9 +118,9 @@ def authorization_response auth_code = params[:code] return not_authorized if auth_code.blank? - @inat_import = InatImport.find_or_create_by(user: User.current) - @inat_import.update(token: auth_code, state: "Authenticating") - tracker = InatImportJobTracker.create(inat_import: @inat_import.id) + inat_import = InatImport.find_or_create_by(user: User.current) + inat_import.update(token: auth_code, state: "Authenticating") + tracker = InatImportJobTracker.create(inat_import: inat_import.id) Rails.logger.info( "Enqueuing InatImportJob for InatImport id: #{@inat_import.id}" @@ -128,8 +128,7 @@ def authorization_response # InatImportJob.perform_now(@inat_import) # for manual testing InatImportJob.perform_later(@inat_import) - # redirect_to(inat_import_path(@inat_import.id)) - redirect_to(inat_import_path(@inat_import, + redirect_to(inat_import_path(inat_import, params: { tracker_id: tracker.id })) end From 1498a76b229713baaf246d8f6d6fc6633b6439fd Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Sun, 22 Dec 2024 20:11:30 -0700 Subject: [PATCH 30/37] Forgot to switch 2 ivars to locals --- app/controllers/inat_imports_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index c63aa29717..9760479ab0 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -123,10 +123,10 @@ def authorization_response tracker = InatImportJobTracker.create(inat_import: inat_import.id) Rails.logger.info( - "Enqueuing InatImportJob for InatImport id: #{@inat_import.id}" + "Enqueuing InatImportJob for InatImport id: #{inat_import.id}" ) - # InatImportJob.perform_now(@inat_import) # for manual testing - InatImportJob.perform_later(@inat_import) + # InatImportJob.perform_now(@inat_import) # uncomment for manual testing + InatImportJob.perform_later(inat_import) # uncomment for production redirect_to(inat_import_path(inat_import, params: { tracker_id: tracker.id })) From 9dff9e31e56d0059854c70949d36eb3d1a565b1e Mon Sep 17 00:00:00 2001 From: "Joseph D. Cohen" Date: Tue, 31 Dec 2024 14:40:49 -0700 Subject: [PATCH 31/37] Switch one more ivar to local Should have been done in #6ecd082 --- app/controllers/inat_imports_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/inat_imports_controller.rb b/app/controllers/inat_imports_controller.rb index 9760479ab0..fc4d6a429d 100644 --- a/app/controllers/inat_imports_controller.rb +++ b/app/controllers/inat_imports_controller.rb @@ -125,7 +125,7 @@ def authorization_response Rails.logger.info( "Enqueuing InatImportJob for InatImport id: #{inat_import.id}" ) - # InatImportJob.perform_now(@inat_import) # uncomment for manual testing + # InatImportJob.perform_now(inat_import) # uncomment for manual testing InatImportJob.perform_later(inat_import) # uncomment for production redirect_to(inat_import_path(inat_import, From 25440744570019b8db8f66d8d84b2bd93175f471 Mon Sep 17 00:00:00 2001 From: Joe Cohen Date: Mon, 13 Jan 2025 09:28:40 -0800 Subject: [PATCH 32/37] Reformat template to make data attrs more visible --- app/views/controllers/inat_imports/show.html.erb | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index a4362002fd..cac768a367 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -13,12 +13,17 @@ Via Stimulus: <%= # Outer span calls Stimulus controller, passing in the endpoint - tag.span(id: "status", class: "text-right", - data: { controller: "inat-import-job", - endpoint: inat_import_job_tracker_path(@inat_import, @tracker) }) do + tag.span( + id: "status", class: "text-right", + data: { controller: "inat-import-job", + endpoint: inat_import_job_tracker_path(@inat_import, @tracker) } + ) do # Inner span is replaced by Stimulus - tag.span(@tracker.status, data: { inat_import_job_target: "status", - status: @tracker.status }) + tag.span( + @tracker.status, + data: { inat_import_job_target: "status", + status: @tracker.status } + ) end %>

From 591284b1dec10bfe35a9bb14504770b9fa1c3781 Mon Sep 17 00:00:00 2001 From: Joe Cohen Date: Mon, 13 Jan 2025 17:40:03 -0800 Subject: [PATCH 33/37] Move Stimulus-controlled stuff to partial - That's where FieldSlipJobTrackersController#show is rendering it. - Adds `id` to affected element. --- .../controllers/inat_imports/job_trackers/_status.erb | 8 +++++++- app/views/controllers/inat_imports/show.html.erb | 7 ++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/views/controllers/inat_imports/job_trackers/_status.erb b/app/views/controllers/inat_imports/job_trackers/_status.erb index bb0f99922c..b8a27e7e80 100644 --- a/app/views/controllers/inat_imports/job_trackers/_status.erb +++ b/app/views/controllers/inat_imports/job_trackers/_status.erb @@ -1 +1,7 @@ -<%= tracker.status %> +<%= + tag.span(@tracker.status, + id: "status", + data: { inat_import_job_target: "status", + status: @tracker.status } + ) +%> diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index cac768a367..640de06026 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -19,11 +19,8 @@ endpoint: inat_import_job_tracker_path(@inat_import, @tracker) } ) do # Inner span is replaced by Stimulus - tag.span( - @tracker.status, - data: { inat_import_job_target: "status", - status: @tracker.status } - ) + render(partial: "inat_imports/job_trackers/status", + locals: { tracker: @tracker }) end %>

From 4b8869d9aa87d415720567fa1b288f9ca09c0d60 Mon Sep 17 00:00:00 2001 From: Joe Cohen Date: Mon, 13 Jan 2025 18:04:21 -0800 Subject: [PATCH 34/37] Remove manually update status line from view --- app/views/controllers/inat_imports/job_trackers/_status.erb | 4 +--- app/views/controllers/inat_imports/show.html.erb | 5 ----- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/app/views/controllers/inat_imports/job_trackers/_status.erb b/app/views/controllers/inat_imports/job_trackers/_status.erb index b8a27e7e80..cc295290ba 100644 --- a/app/views/controllers/inat_imports/job_trackers/_status.erb +++ b/app/views/controllers/inat_imports/job_trackers/_status.erb @@ -1,7 +1,5 @@ <%= tag.span(@tracker.status, - id: "status", data: { inat_import_job_target: "status", - status: @tracker.status } - ) + status: @tracker.status }) %> diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index 640de06026..281bc2fdda 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -6,11 +6,6 @@

<%= :inat_import_tracker_status.t %>: - <%= @inat_import.state %> -

- -

- Via Stimulus: <%= # Outer span calls Stimulus controller, passing in the endpoint tag.span( From 7824fee7291dec6e4654a950967c51eab26af92d Mon Sep 17 00:00:00 2001 From: Joe Cohen Date: Tue, 14 Jan 2025 08:42:58 -0800 Subject: [PATCH 35/37] Rename partial updated by Stimulus --- app/controllers/inat_imports/job_trackers_controller.rb | 4 ++-- .../inat_imports/job_trackers/{_status.erb => _updates.erb} | 0 app/views/controllers/inat_imports/show.html.erb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename app/views/controllers/inat_imports/job_trackers/{_status.erb => _updates.erb} (100%) diff --git a/app/controllers/inat_imports/job_trackers_controller.rb b/app/controllers/inat_imports/job_trackers_controller.rb index 104e59cb79..7f0cf10058 100644 --- a/app/controllers/inat_imports/job_trackers_controller.rb +++ b/app/controllers/inat_imports/job_trackers_controller.rb @@ -11,8 +11,8 @@ def show respond_to do |format| format.turbo_stream do render(turbo_stream: turbo_stream.update( - :status, # id of element to replace - partial: "inat_imports/job_trackers/status", + :status, # id of element to change + partial: "inat_imports/job_trackers/updates", locals: { tracker: @tracker } )) end diff --git a/app/views/controllers/inat_imports/job_trackers/_status.erb b/app/views/controllers/inat_imports/job_trackers/_updates.erb similarity index 100% rename from app/views/controllers/inat_imports/job_trackers/_status.erb rename to app/views/controllers/inat_imports/job_trackers/_updates.erb diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index 281bc2fdda..ef4183805b 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -14,7 +14,7 @@ endpoint: inat_import_job_tracker_path(@inat_import, @tracker) } ) do # Inner span is replaced by Stimulus - render(partial: "inat_imports/job_trackers/status", + render(partial: "inat_imports/job_trackers/updates", locals: { tracker: @tracker }) end %> From 696c9c8f3742e654055292a3e30e6d836feaa90c Mon Sep 17 00:00:00 2001 From: Joe Cohen Date: Tue, 14 Jan 2025 09:32:52 -0800 Subject: [PATCH 36/37] Refresh button uses js instead of Rails request - The request throws an error. --- app/views/controllers/inat_imports/show.html.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index ef4183805b..86209a3191 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -39,9 +39,9 @@ <%# Prominently refresh button if job is incomplete%> <% if import_incomplete?(@inat_import) %>

- <%= link_to(:inat_import_tracker_refresh.l, - inat_import_job_tracker_path(@tracker), - { class: "btn btn-default" } ) %> +

<% end %> From c224b36dcf5f970a6601a16418a5551138ef1b6e Mon Sep 17 00:00:00 2001 From: Joe Cohen Date: Tue, 14 Jan 2025 12:13:01 -0800 Subject: [PATCH 37/37] Fix indentation & comment. --- app/views/controllers/inat_imports/show.html.erb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/controllers/inat_imports/show.html.erb b/app/views/controllers/inat_imports/show.html.erb index 86209a3191..fdf66ccfc0 100644 --- a/app/views/controllers/inat_imports/show.html.erb +++ b/app/views/controllers/inat_imports/show.html.erb @@ -36,13 +36,13 @@ <%= @tracker.created_at %>

-<%# Prominently refresh button if job is incomplete%> +<%# Display refresh button if job is incomplete%> <% if import_incomplete?(@inat_import) %> -

- -

+

+ +

<% end %>