diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 5c95dc4f..2904c492 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -11,7 +11,8 @@ updates: interval: "weekly" target-branch: "develop" labels: - - "ruby dependencies" + - "ruby" + - "dependencies" - package-ecosystem: "npm" directory: "/" @@ -19,7 +20,8 @@ updates: interval: "weekly" target-branch: "develop" labels: - - "javascript dependencies" + - "javascript" + - "dependencies" - package-ecosystem: "composer" directory: "/" diff --git a/.opencode/agents/rails-code-auditor.md b/.opencode/agents/rails-code-auditor.md index 8e40ed14..cc56bcb7 100644 --- a/.opencode/agents/rails-code-auditor.md +++ b/.opencode/agents/rails-code-auditor.md @@ -1,7 +1,7 @@ --- description: Review code for quality and Rails conventions (report + suggest on request) mode: subagent -model: minimax-coding-plan/MiniMax-M2.5 +model: minimax-coding-plan/MiniMax-M2.7 permission: skill: "rails-code-quality": "allow" diff --git a/.opencode/agents/rails-migration-manager.md b/.opencode/agents/rails-migration-manager.md index 86e1c41d..f84e53f2 100644 --- a/.opencode/agents/rails-migration-manager.md +++ b/.opencode/agents/rails-migration-manager.md @@ -1,7 +1,7 @@ --- description: Manage Rails migrations - create, run, rollback, and troubleshoot mode: subagent -model: minimax-coding-plan/MiniMax-M2.5 +model: minimax-coding-plan/MiniMax-M2.7 permission: skill: "rails-migrations": "allow" diff --git a/.opencode/agents/rails-refactor.md b/.opencode/agents/rails-refactor.md index 5e439bd8..ef9f8407 100644 --- a/.opencode/agents/rails-refactor.md +++ b/.opencode/agents/rails-refactor.md @@ -1,7 +1,7 @@ --- description: Refactor code following Rails and project conventions mode: subagent -model: minimax-coding-plan/MiniMax-M2.5 +model: minimax-coding-plan/MiniMax-M2.7 permission: skill: "rails-code-quality": "allow" diff --git a/.opencode/agents/rails-resource-builder.md b/.opencode/agents/rails-resource-builder.md index 7844740a..dea91293 100644 --- a/.opencode/agents/rails-resource-builder.md +++ b/.opencode/agents/rails-resource-builder.md @@ -1,7 +1,7 @@ --- description: Generate complete Rails resources (models, controllers, routes, tests) mode: subagent -model: minimax-coding-plan/MiniMax-M2.5 +model: minimax-coding-plan/MiniMax-M2.7 permission: skill: "rails-models": "allow" diff --git a/.opencode/agents/rails-test-runner.md b/.opencode/agents/rails-test-runner.md index 9f9819bd..c5906966 100644 --- a/.opencode/agents/rails-test-runner.md +++ b/.opencode/agents/rails-test-runner.md @@ -1,7 +1,7 @@ --- description: Execute tests and report results only mode: subagent -model: minimax-coding-plan/MiniMax-M2.5 +model: minimax-coding-plan/MiniMax-M2.7 permission: skill: "rspec-testing": "allow" diff --git a/.opencode/agents/ruby-gem-updater.md b/.opencode/agents/ruby-gem-updater.md index 1cb9680a..b5a0cc87 100644 --- a/.opencode/agents/ruby-gem-updater.md +++ b/.opencode/agents/ruby-gem-updater.md @@ -1,7 +1,7 @@ --- description: Execute Ruby gem updates with version checking, breaking change analysis, and testing mode: subagent -model: minimax-coding-plan/MiniMax-M2.5 +model: minimax-coding-plan/MiniMax-M2.7 permission: skill: "ruby-gem-update": "allow" diff --git a/Gemfile b/Gemfile index 81e83ad0..85153e05 100644 --- a/Gemfile +++ b/Gemfile @@ -4,7 +4,7 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" } ruby "3.4.5" # Bundle edge Rails instead: gem "rails", github: "rails/rails" -gem "rails", "~> 8.1.0" +gem "rails", "~> 8.1.3" # Use postgresql as the database for Active Record gem "pg", "~> 1.6.2" # Use Puma as the app server @@ -68,8 +68,8 @@ gem "faker", "~> 3.6", groups: [:development, :test].tap { |groups| group :development do # Access an interactive console on exception pages or by calling "console" anywhere in the code. - gem "web-console", "~> 4.2.1" - gem "listen", "~> 3.9.0" + gem "web-console", "~> 4.3.0" + gem "listen", "~> 3.10.0" # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring # gem "spring" @@ -106,7 +106,7 @@ end gem "tzinfo-data", platforms: [:mri, :windows] # Pagination -gem "pagy", "~> 43.4" +gem "pagy", "~> 43.5" # Alternative approach to web apps development. # https://github.com/hotwired/hotwire-rails @@ -135,7 +135,7 @@ gem "importmap-rails" # gem "rack-timeout" # Http client for making API requests -gem "faraday", "~> 2.14.0" +gem "faraday", "~> 2.14.2" # OpenStruct for easy data modeling - removed from standard library since ruby 3.5 gem "ostruct" diff --git a/Gemfile.lock b/Gemfile.lock index 2c28e615..9f7f05a0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,31 +7,31 @@ GIT GEM remote: https://rubygems.org/ specs: - action_text-trix (2.1.17) + action_text-trix (2.1.18) railties - actioncable (8.1.2) - actionpack (= 8.1.2) - activesupport (= 8.1.2) + actioncable (8.1.3) + actionpack (= 8.1.3) + activesupport (= 8.1.3) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (8.1.2) - actionpack (= 8.1.2) - activejob (= 8.1.2) - activerecord (= 8.1.2) - activestorage (= 8.1.2) - activesupport (= 8.1.2) + actionmailbox (8.1.3) + actionpack (= 8.1.3) + activejob (= 8.1.3) + activerecord (= 8.1.3) + activestorage (= 8.1.3) + activesupport (= 8.1.3) mail (>= 2.8.0) - actionmailer (8.1.2) - actionpack (= 8.1.2) - actionview (= 8.1.2) - activejob (= 8.1.2) - activesupport (= 8.1.2) + actionmailer (8.1.3) + actionpack (= 8.1.3) + actionview (= 8.1.3) + activejob (= 8.1.3) + activesupport (= 8.1.3) mail (>= 2.8.0) rails-dom-testing (~> 2.2) - actionpack (8.1.2) - actionview (= 8.1.2) - activesupport (= 8.1.2) + actionpack (8.1.3) + actionview (= 8.1.3) + activesupport (= 8.1.3) nokogiri (>= 1.8.5) rack (>= 2.2.4) rack-session (>= 1.0.1) @@ -39,36 +39,36 @@ GEM rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) useragent (~> 0.16) - actiontext (8.1.2) + actiontext (8.1.3) action_text-trix (~> 2.1.15) - actionpack (= 8.1.2) - activerecord (= 8.1.2) - activestorage (= 8.1.2) - activesupport (= 8.1.2) + actionpack (= 8.1.3) + activerecord (= 8.1.3) + activestorage (= 8.1.3) + activesupport (= 8.1.3) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (8.1.2) - activesupport (= 8.1.2) + actionview (8.1.3) + activesupport (= 8.1.3) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) - activejob (8.1.2) - activesupport (= 8.1.2) + activejob (8.1.3) + activesupport (= 8.1.3) globalid (>= 0.3.6) - activemodel (8.1.2) - activesupport (= 8.1.2) - activerecord (8.1.2) - activemodel (= 8.1.2) - activesupport (= 8.1.2) + activemodel (8.1.3) + activesupport (= 8.1.3) + activerecord (8.1.3) + activemodel (= 8.1.3) + activesupport (= 8.1.3) timeout (>= 0.4.0) - activestorage (8.1.2) - actionpack (= 8.1.2) - activejob (= 8.1.2) - activerecord (= 8.1.2) - activesupport (= 8.1.2) + activestorage (8.1.3) + actionpack (= 8.1.3) + activejob (= 8.1.3) + activerecord (= 8.1.3) + activesupport (= 8.1.3) marcel (~> 1.0) - activesupport (8.1.2) + activesupport (8.1.3) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.3.1) @@ -81,7 +81,7 @@ GEM securerandom (>= 0.3) tzinfo (~> 2.0, >= 2.0.5) uri (>= 0.13.1) - addressable (2.8.9) + addressable (2.9.0) public_suffix (>= 2.0.2, < 8.0) ast (2.4.3) base64 (0.3.0) @@ -90,7 +90,7 @@ GEM erubi (>= 1.0.0) rack (>= 0.9.0) rouge (>= 1.0.0) - bigdecimal (4.0.1) + bigdecimal (4.1.2) bindex (0.8.1) binding_of_caller (2.0.0) debug_inspector (>= 1.2.0) @@ -143,20 +143,20 @@ GEM railties (>= 5.0.0) faker (3.6.1) i18n (>= 1.8.11, < 2) - faraday (2.14.1) + faraday (2.14.2) faraday-net_http (>= 2.0, < 3.5) json logger faraday-net_http (3.4.2) net-http (~> 0.5) - ffi (1.17.3-aarch64-linux-gnu) - ffi (1.17.3-aarch64-linux-musl) - ffi (1.17.3-arm-linux-gnu) - ffi (1.17.3-arm-linux-musl) - ffi (1.17.3-arm64-darwin) - ffi (1.17.3-x86_64-darwin) - ffi (1.17.3-x86_64-linux-gnu) - ffi (1.17.3-x86_64-linux-musl) + ffi (1.17.4-aarch64-linux-gnu) + ffi (1.17.4-aarch64-linux-musl) + ffi (1.17.4-arm-linux-gnu) + ffi (1.17.4-arm-linux-musl) + ffi (1.17.4-arm64-darwin) + ffi (1.17.4-x86_64-darwin) + ffi (1.17.4-x86_64-linux-gnu) + ffi (1.17.4-x86_64-linux-musl) geo_coord (0.2.0) geocoder (1.8.6) base64 (>= 0.1.0) @@ -203,15 +203,13 @@ GEM prism (>= 1.3.0) rdoc (>= 4.0.0) reline (>= 0.4.2) - json (2.19.2) - json-schema (6.2.0) - addressable (~> 2.8) - bigdecimal (>= 3.1, < 5) + json (2.19.5) jwt (3.1.2) base64 language_server-protocol (3.17.0.5) lint_roller (1.1.0) - listen (3.9.0) + listen (3.10.0) + logger rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) logger (1.7.0) @@ -226,12 +224,10 @@ GEM net-smtp marcel (1.1.0) matrix (0.4.3) - mcp (0.9.0) - json-schema (>= 4.1) memory_profiler (1.1.0) method_source (1.1.0) mini_mime (1.1.5) - minitest (6.0.2) + minitest (6.0.4) drb (~> 2.0) prism (~> 1.5) msgpack (1.8.0) @@ -247,30 +243,30 @@ GEM net-smtp (0.5.1) net-protocol nio4r (2.7.5) - nokogiri (1.19.2-aarch64-linux-gnu) + nokogiri (1.19.3-aarch64-linux-gnu) racc (~> 1.4) - nokogiri (1.19.2-aarch64-linux-musl) + nokogiri (1.19.3-aarch64-linux-musl) racc (~> 1.4) - nokogiri (1.19.2-arm-linux-gnu) + nokogiri (1.19.3-arm-linux-gnu) racc (~> 1.4) - nokogiri (1.19.2-arm-linux-musl) + nokogiri (1.19.3-arm-linux-musl) racc (~> 1.4) - nokogiri (1.19.2-arm64-darwin) + nokogiri (1.19.3-arm64-darwin) racc (~> 1.4) - nokogiri (1.19.2-x86_64-darwin) + nokogiri (1.19.3-x86_64-darwin) racc (~> 1.4) - nokogiri (1.19.2-x86_64-linux-gnu) + nokogiri (1.19.3-x86_64-linux-gnu) racc (~> 1.4) - nokogiri (1.19.2-x86_64-linux-musl) + nokogiri (1.19.3-x86_64-linux-musl) racc (~> 1.4) orm_adapter (0.5.0) ostruct (0.6.3) - pagy (43.4.2) + pagy (43.5.1) json uri yaml - parallel (1.27.0) - parser (3.3.10.2) + parallel (2.0.1) + parser (3.3.11.1) ast (~> 2.4.1) racc pg (1.6.3) @@ -284,7 +280,7 @@ GEM prettyprint prettyprint (0.2.0) prism (1.9.0) - propshaft (1.3.1) + propshaft (1.3.2) actionpack (>= 7.0.0) activesupport (>= 7.0.0) rack @@ -311,33 +307,33 @@ GEM puma (7.2.0) nio4r (~> 2.0) racc (1.8.1) - rack (3.2.5) + rack (3.2.6) rack-cors (3.0.0) logger rack (>= 3.0.14) rack-mini-profiler (4.0.1) rack (>= 1.2.0) - rack-session (2.1.1) + rack-session (2.1.2) base64 (>= 0.1.0) rack (>= 3.0.0) rack-test (2.2.0) rack (>= 1.3) rackup (2.3.1) rack (>= 3) - rails (8.1.2) - actioncable (= 8.1.2) - actionmailbox (= 8.1.2) - actionmailer (= 8.1.2) - actionpack (= 8.1.2) - actiontext (= 8.1.2) - actionview (= 8.1.2) - activejob (= 8.1.2) - activemodel (= 8.1.2) - activerecord (= 8.1.2) - activestorage (= 8.1.2) - activesupport (= 8.1.2) + rails (8.1.3) + actioncable (= 8.1.3) + actionmailbox (= 8.1.3) + actionmailer (= 8.1.3) + actionpack (= 8.1.3) + actiontext (= 8.1.3) + actionview (= 8.1.3) + activejob (= 8.1.3) + activemodel (= 8.1.3) + activerecord (= 8.1.3) + activestorage (= 8.1.3) + activesupport (= 8.1.3) bundler (>= 1.15.0) - railties (= 8.1.2) + railties (= 8.1.3) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) actionview (>= 5.0.1.rc1) @@ -349,9 +345,9 @@ GEM rails-html-sanitizer (1.7.0) loofah (~> 2.25) nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) - railties (8.1.2) - actionpack (= 8.1.2) - activesupport (= 8.1.2) + railties (8.1.3) + actionpack (= 8.1.3) + activesupport (= 8.1.3) irb (~> 1.13) rackup (>= 1.0.0) rake (>= 12.2) @@ -371,7 +367,7 @@ GEM redis-client (>= 0.22.0) redis-client (0.28.0) connection_pool - regexp_parser (2.11.3) + regexp_parser (2.12.0) reline (0.6.3) io-console (~> 0.5) requestjs-rails (0.0.14) @@ -397,12 +393,11 @@ GEM rspec-mocks (>= 3.13.0, < 5.0.0) rspec-support (>= 3.13.0, < 5.0.0) rspec-support (3.13.7) - rubocop (1.85.1) + rubocop (1.86.1) json (~> 2.3) language_server-protocol (~> 3.17.0.2) lint_roller (~> 1.1.0) - mcp (~> 0.6) - parallel (~> 1.10) + parallel (>= 1.10) parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 2.9.3, < 3.0) @@ -480,11 +475,10 @@ GEM concurrent-ruby (~> 1) warden (1.2.9) rack (>= 2.0.9) - web-console (4.2.1) - actionview (>= 6.0.0) - activemodel (>= 6.0.0) + web-console (4.3.0) + actionview (>= 8.0.0) bindex (>= 0.4.0) - railties (>= 6.0.0) + railties (>= 8.0.0) websocket-driver (0.8.0) base64 websocket-extensions (>= 0.1.0) @@ -519,7 +513,7 @@ DEPENDENCIES dotenv-rails factory_bot_rails (~> 6.4.3) faker (~> 3.6) - faraday (~> 2.14.0) + faraday (~> 2.14.2) geo_coord geocoder (~> 1.8) haversine! @@ -527,10 +521,10 @@ DEPENDENCIES importmap-rails inline_svg jwt - listen (~> 3.9.0) + listen (~> 3.10.0) memory_profiler ostruct - pagy (~> 43.4) + pagy (~> 43.5) pg (~> 1.6.2) propshaft pry (~> 0.16.0) @@ -541,7 +535,7 @@ DEPENDENCIES puma (~> 7.2) rack-cors rack-mini-profiler (~> 4.0) - rails (~> 8.1.0) + rails (~> 8.1.3) rails-controller-testing redis (~> 5.4.1) requestjs-rails @@ -557,7 +551,7 @@ DEPENDENCIES turbo-rails tzinfo-data view_component - web-console (~> 4.2.1) + web-console (~> 4.3.0) RUBY VERSION ruby 3.4.5p51 diff --git a/app/components/facilities/discard_reason_component.rb b/app/components/facilities/discard_reason_component.rb index e978c750..fbc6c2ff 100644 --- a/app/components/facilities/discard_reason_component.rb +++ b/app/components/facilities/discard_reason_component.rb @@ -4,9 +4,10 @@ class Facilities::DiscardReasonComponent < ViewComponent::Base attr_reader :discard_reason VALID_REASONS = { - none: "None", + nil => "None", closed: "Closed", - duplicated: "Duplicated" + duplicated: "Duplicated", + sync_removed: "Removed by Sync" }.freeze def initialize(discard_reason) @@ -20,7 +21,7 @@ def self.select_options end def call - text = VALID_REASONS[discard_reason] || "Unsupported value '#{discard_reason}'" + text = VALID_REASONS[discard_reason.presence] || "Unsupported value '#{discard_reason}'" tag.span(text) end end diff --git a/app/components/facilities/show_component/details_card_component.html.erb b/app/components/facilities/show_component/details_card_component.html.erb index 23f029d8..09420267 100644 --- a/app/components/facilities/show_component/details_card_component.html.erb +++ b/app/components/facilities/show_component/details_card_component.html.erb @@ -95,6 +95,10 @@
+ + + + diff --git a/app/controllers/admin/facilities_controller.rb b/app/controllers/admin/facilities_controller.rb index 1f64e6ec..72391b08 100644 --- a/app/controllers/admin/facilities_controller.rb +++ b/app/controllers/admin/facilities_controller.rb @@ -75,42 +75,55 @@ def switch_status def load_facilities facilities = Facility.all + facilities = filter_by_status(facilities) + facilities = filter_by_service(facilities) + facilities = filter_by_welcome(facilities) + facilities = filter_by_search(facilities) + facilities = facilities.with_associations.order(updated_at: :desc) + @pagy, @facilities = pagy(facilities) + end + + def filter_by_status(facilities) case params[:status] - when "live" - facilities = facilities.live - when "pending_reviews" - facilities = facilities.pending_reviews - when "discarded" - facilities = facilities.discarded + when "live" then facilities.live + when "pending_reviews" then facilities.pending_reviews + when "discarded" then facilities.discarded + else facilities end + end + def filter_by_service(facilities) if params[:service] == "none" - facilities = facilities.without_services + facilities.without_services elsif params[:service].present? - facilities = facilities.with_service(params[:service]) + facilities.with_service(params[:service]) + else + facilities end + end + def filter_by_welcome(facilities) if params[:welcome_customer] == "none" - facilities = facilities.without_welcomes + facilities.without_welcomes elsif params[:welcome_customer].present? - facilities = facilities.joins(:facility_welcomes) - .where(facility_welcomes: { customer: params[:welcome_customer] }) + facilities.joins(:facility_welcomes) + .where(facility_welcomes: { customer: params[:welcome_customer] }) + else + facilities end + end + def filter_by_search(facilities) if params[:q].present? - facilities = facilities.name_search(params[:q]).or( - facilities.address_search(params[:q]) - ) + facilities.name_search(params[:q]).or(facilities.address_search(params[:q])) + else + facilities end - - facilities = facilities.order(updated_at: :asc) - - @pagy, @facilities = pagy(facilities) end def load_facility - @facility = Facility.find(params[:id]) + @facility = Facility.with_associations.find(params[:id]) end def load_services_dropdown diff --git a/app/controllers/admin/tools_controller.rb b/app/controllers/admin/tools_controller.rb index 0448a6e1..b6db530b 100644 --- a/app/controllers/admin/tools_controller.rb +++ b/app/controllers/admin/tools_controller.rb @@ -8,7 +8,6 @@ def index; end def import_facilities api_key = params[:api] - # Validate that both parameters are present and supported unless External::ApiHelper.supported_api?(api_key) redirect_to admin_tools_path, alert: "Invalid API selected. Please choose from the supported APIs." return @@ -16,23 +15,108 @@ def import_facilities result = External::VancouverCity::Syncer.call( api_key: api_key, - api_client: External::VancouverCity.default_client + api_client: External::VancouverCity.default_client, + full_sync: true ) - if result.success? - total_count = result.data[:total_count] || 0 - redirect_to admin_facilities_path(service: "water_fountain"), notice: "#{total_count} Facilities imported successfully from #{External::ApiHelper.api_name(api_key)}." + result_data = collect_result_data(result) + handle_import_result(result, result_data, api_key) + end + + def discard_facilities + successes = 0 + errors = [] + Facility.external.kept.find_each do |facility| + result = External::VancouverCity::FacilityDiscarder.call(facility) + if result.success? + successes += 1 + else + Rails.logger.error "Failed to discard facility #{facility.id} (#{facility.name}): #{result.errors.join(', ')}" + errors << result.error_messages + end + end + + handle_discard_result(successes, errors) + end + + def handle_discard_result(successes, errors) + if errors.empty? + Rails.logger.info "Successfully discarded #{successes} external facilities." + redirect_to admin_facilities_path(service: "water_fountain"), + notice: "#{successes} external facilities discarded." + elsif successes.positive? + Rails.logger.warn "Partial discard: #{successes} facilities discarded, but #{errors.flatten.size} failed to discard with errors: #{errors.flatten.join('; ')}" + success_message = "#{successes} external facilities discarded." + failure_message = "However, #{errors.flatten.size} facilities failed to discard with errors: #{errors.flatten.join('; ')}" + redirect_to admin_tools_path(service: "water_fountain"), + alert: "#{success_message} #{failure_message}." else - error_messages = result.errors.join(", ") - redirect_to admin_tools_path, alert: "Failed to import facilities: #{error_messages}" + Rails.logger.error "Failed to discard any facilities. Errors: #{errors.flatten.join('; ')}" + error_messages = errors.flatten.join("; ") + redirect_to admin_tools_path, alert: "Failed to discard #{errors.size} facilities: #{error_messages}" end end + SyncReport = Struct.new( + :create_operation, :update_operation, :delete_operation, + :success_count, :failure_count, + :success_created, :success_updated, :success_deleted, + :failure_created, :failure_updated, :failure_deleted, + :errors_messages, + keyword_init: true + ) + # Helper method for the view helper_method :api_options_for_select private + def collect_result_data(sync_result) + create_op = sync_result.data.select { |e| e.operation == External::SyncOperations.create } + update_op = sync_result.data.select { |e| e.operation.name == "update" } + delete_op = sync_result.data.select { |e| e.operation == External::SyncOperations.discard } + + SyncReport.new( + create_operation: create_op, + update_operation: update_op, + delete_operation: delete_op, + success_count: sync_result.data.count(&:success?), + failure_count: sync_result.data.count(&:failed?), + success_created: create_op.count(&:success?), + success_updated: update_op.count(&:success?), + success_deleted: delete_op.count(&:success?), + failure_created: create_op.count(&:failed?), + failure_updated: update_op.count(&:failed?), + failure_deleted: delete_op.count(&:failed?), + errors_messages: sync_result.errors.flatten + ) + end + + def handle_import_result(result, result_data, api_key) + if result.partial_failed? + handle_partial_failure(result_data, api_key) + elsif result.success? + handle_full_success(result_data, api_key) + else + handle_full_failure(result_data, api_key) + end + end + + def handle_partial_failure(result_data, _api_key) + Rails.logger.warn "Partial sync: #{result_data.success_count} facilities synced successfully, but #{result_data.failure_count} failed with errors: #{result_data.errors_messages.join(', ')}" + redirect_to admin_tools_path, alert: "Sync completed with some errors: #{result_data.success_count} succeeded (#{result_data.success_created} created, #{result_data.success_updated} updated, #{result_data.success_deleted} deleted), but #{result_data.failure_count} failed (#{result_data.failure_created} create failures, #{result_data.failure_updated} update failures, #{result_data.failure_deleted} delete failures). Please check logs for details." + end + + def handle_full_success(result_data, api_key) + Rails.logger.info "Successfully imported #{result_data.success_count} facilities (#{result_data.success_created} created, #{result_data.success_updated} updated, #{result_data.success_deleted} deleted) from #{api_key} API." + redirect_to admin_facilities_path(service: "water_fountain"), notice: "Successfully imported #{result_data.success_count} facilities (#{result_data.success_created} created, #{result_data.success_updated} updated, #{result_data.success_deleted} deleted) from #{api_key} API." + end + + def handle_full_failure(result_data, api_key) + Rails.logger.error "Failed to sync facilities from #{api_key} API: #{result_data.errors_messages.join(', ')}" + redirect_to admin_tools_path, alert: "Failed to sync facilities from #{api_key} API: #{result_data.errors_messages.join(', ')}" + end + def api_options_for_select External::ApiHelper.api_options end diff --git a/app/controllers/api/facilities_controller.rb b/app/controllers/api/facilities_controller.rb index 885eaed7..cc77c144 100644 --- a/app/controllers/api/facilities_controller.rb +++ b/app/controllers/api/facilities_controller.rb @@ -65,12 +65,7 @@ def load_facilities end # Includes related objects to avoid N+1 queries - @facilities = @facilities.includes( - :zone, - :facility_welcomes, - { facility_services: [:service] }, - { schedules: [:time_slots] } - ) + @facilities = @facilities.with_associations end def register_impressions diff --git a/app/javascript/controllers/index.js b/app/javascript/controllers/index.js index 5d960599..696e17b0 100644 --- a/app/javascript/controllers/index.js +++ b/app/javascript/controllers/index.js @@ -18,3 +18,9 @@ application.register("navigate", NavigateController) import PagyController from "controllers/pagy_controller" application.register("pagy", PagyController) + +import ProgressController from "controllers/progress_controller" +application.register("progress", ProgressController) + +import TabsController from "controllers/tabs_controller" +application.register("tabs", TabsController) diff --git a/app/javascript/controllers/progress_controller.js b/app/javascript/controllers/progress_controller.js new file mode 100644 index 00000000..951c9a77 --- /dev/null +++ b/app/javascript/controllers/progress_controller.js @@ -0,0 +1,36 @@ +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static targets = ["indicator", "message"] + + connect() { + console.log("Progress Controller Connected", this.element) + } + + submit(event) { + const form = event.currentTarget + const button = form.querySelector('input[type="submit"], button[type="submit"]') + const operation = form.dataset.operation + + if (this.hasMessageTarget) { + this.messageTarget.textContent = this.getMessage(operation) + } + + if (button) { + button.classList.add("is-loading") + button.disabled = true + } + + if (this.hasIndicatorTarget) { + this.indicatorTarget.classList.remove("is-hidden") + } + } + + getMessage(operation) { + const messages = { + import: "Importing facilities from Vancouver City API...", + discard: "Discarding facilities from Vancouver City API..." + } + return messages[operation] || "Processing..." + } +} diff --git a/app/javascript/controllers/tabs_controller.js b/app/javascript/controllers/tabs_controller.js new file mode 100644 index 00000000..8a031982 --- /dev/null +++ b/app/javascript/controllers/tabs_controller.js @@ -0,0 +1,52 @@ +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static targets = ["link", "content"] + + initialize() { + this.activeTab = this.element.dataset.activeTab || "sync" + } + + connect() { + console.log("Tabs Controller Connected", this.element); + + this.boundSwitchTab = this.switchTab.bind(this) + this.linkTargets.forEach((link) => { + link.addEventListener('click', this.boundSwitchTab) + }) + this.activateTab(this.activeTab) + } + + disconnect() { + this.linkTargets.forEach((link) => { + link.removeEventListener('click', this.boundSwitchTab) + }) + } + + switchTab(event) { + event.preventDefault() + + const tab = event.currentTarget.dataset.tab + this.activateTab(tab) + } + + activateTab(tab) { + this.linkTargets.forEach((link) => { + if (link.dataset.tab === tab) { + link.closest("li").classList.add("is-active") + } else { + link.closest("li").classList.remove("is-active") + } + }) + + this.contentTargets.forEach((content) => { + if (content.id === tab) { + content.classList.remove("is-hidden") + } else { + content.classList.add("is-hidden") + } + }) + + this.activeTab = tab + } +} diff --git a/app/jobs/facilities_static_generator_job.rb b/app/jobs/facilities_static_generator_job.rb deleted file mode 100644 index d35bb852..00000000 --- a/app/jobs/facilities_static_generator_job.rb +++ /dev/null @@ -1,11 +0,0 @@ -# frozen_string_literal: true - -class FacilitiesStaticGeneratorJob - def perform - jsonfile = "public/facilities.json" - facilities_hash = { - v1: { facilities: Facility.is_verified.as_json } - } - File.write(jsonfile, JSON.pretty_generate(facilities_hash)) - end -end diff --git a/app/models/facility.rb b/app/models/facility.rb index 450cc5e3..bb6328c9 100644 --- a/app/models/facility.rb +++ b/app/models/facility.rb @@ -10,19 +10,20 @@ class Facility < ApplicationRecord belongs_to :user, optional: true belongs_to :zone, optional: true - has_many :facility_welcomes, dependent: :destroy - has_many :facility_services, dependent: :destroy + has_many :facility_welcomes, dependent: :destroy, autosave: true + has_many :facility_services, dependent: :destroy, autosave: true has_many :services, through: :facility_services - has_many :schedules, class_name: "FacilitySchedule", dependent: :destroy + has_many :schedules, class_name: "FacilitySchedule", dependent: :destroy, autosave: true has_many :time_slots, through: :schedules enum :discard_reason, { - none: nil, closed: "closed", - duplicated: "duplicated" - }, prefix: true, default: :none + duplicated: "duplicated", + sync_removed: "sync_removed" + }, prefix: true, default: nil validates :name, presence: true + validates :external_id, uniqueness: { allow_nil: true } validate :validate_website with_options if: :verified? do @@ -42,23 +43,30 @@ class Facility < ApplicationRecord scope :without_welcomes, -> { where.not(facility_welcomes: FacilityWelcome.all) } scope :external, -> { where.not(external_id: nil) } scope :not_external, -> { where(external_id: nil) } + scope :with_associations, lambda { + includes( + :zone, + :facility_welcomes, + { facility_services: [:service] }, + { schedules: [:time_slots] } + ) + } + + def discard_reason_none? + discard_reason.nil? + end def managed_by?(user_or_user_id) - return false if user_or_user_id.blank? + f_user = if user_or_user_id.respond_to? :id + user_or_user_id + else + User.find_by(id: user_or_user_id) + end - f_user_id = if user_or_user_id.respond_to? :id - user_or_user_id.id - else - user_or_user_id - end + return false if f_user.blank? # Case Facility's User is the same - return true if user_id == f_user_id - # Case Zone of the Facility has the user as admin - return true if User.find(f_user_id).manages.any? - - # Otherwise return FALSE - false + user_id == f_user.id || f_user.manages.exists?(id: id) end def self.managed_by(user) @@ -157,7 +165,7 @@ def clean_data end # handles discard - self.discard_reason = :none if undiscarded? + self.discard_reason = nil if undiscarded? end def distance(to_coord: nil, to_lat: nil, to_long: nil, to_facility: nil) diff --git a/app/models/facility_schedule.rb b/app/models/facility_schedule.rb index 156e7495..1fe22e21 100644 --- a/app/models/facility_schedule.rb +++ b/app/models/facility_schedule.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class FacilitySchedule < ApplicationRecord - belongs_to :facility, touch: true + belongs_to :facility, touch: true, inverse_of: :schedules has_many :time_slots, class_name: "FacilityTimeSlot", dependent: :destroy SLOT_TIME_PRESENCE_ERROR = "must not be present if facility availability is %s all day for %s" diff --git a/app/models/facility_service.rb b/app/models/facility_service.rb index bfd3c59c..1d16a376 100644 --- a/app/models/facility_service.rb +++ b/app/models/facility_service.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true class FacilityService < ApplicationRecord - belongs_to :facility, touch: true - belongs_to :service + belongs_to :facility, touch: true, inverse_of: :facility_services + belongs_to :service, inverse_of: :facility_services validates :service, uniqueness: { scope: :facility } diff --git a/app/models/facility_time_slot.rb b/app/models/facility_time_slot.rb index daae60ea..680c898a 100644 --- a/app/models/facility_time_slot.rb +++ b/app/models/facility_time_slot.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class FacilityTimeSlot < ApplicationRecord - belongs_to :facility_schedule, touch: true + belongs_to :facility_schedule, touch: true, inverse_of: :time_slots has_one :facility, through: :facility_schedule validates :from_hour, :to_hour, presence: true, diff --git a/app/models/facility_welcome.rb b/app/models/facility_welcome.rb index b7ca2c6b..befa0293 100644 --- a/app/models/facility_welcome.rb +++ b/app/models/facility_welcome.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class FacilityWelcome < ApplicationRecord - belongs_to :facility, touch: true + belongs_to :facility, touch: true, inverse_of: :facility_welcomes validates :customer, presence: true, uniqueness: { scope: :facility } diff --git a/app/services/application_service.rb b/app/services/application_service.rb index cb9d3869..b6bc76f5 100644 --- a/app/services/application_service.rb +++ b/app/services/application_service.rb @@ -31,6 +31,14 @@ def invalid? !valid? end + def success(data = {}) + Result.new(data: data, errors: []) + end + + def failure(errors) + Result.new(data: nil, errors: Array(errors)) + end + private def errors diff --git a/app/services/external/sync_operations.rb b/app/services/external/sync_operations.rb new file mode 100644 index 00000000..3fca0116 --- /dev/null +++ b/app/services/external/sync_operations.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +class External::SyncOperations + class << self + def unexpected_error + @unexpected_error ||= UnexpectedError.new + end + + def unknown + @unknown ||= Unknown.new + end + + def api_call + @api_call ||= ApiCall.new + end + + def create + @create ||= Create.new + end + + def external_update + @external_update ||= ExternalUpdate.new + end + + def internal_update + @internal_update ||= InternalUpdate.new + end + + def discard + @discard ||= Discard.new + end + end + + class Base + def name + throw NotImplementedError, "Subclasses must implement the name method" + end + end + + class Create < Base + def name + "create" + end + end + + class ExternalUpdate < Base + def name + "update" + end + end + + class InternalUpdate < Base + def name + "update" + end + end + + class Discard < Base + def name + "discard" + end + end + + class ApiCall < Base + def name + "api call" + end + end + + class Unknown < Base + def name + "unknown" + end + end + + class UnexpectedError < Base + def name + "unexpected error" + end + end +end diff --git a/app/services/external/vancouver_city/facility_builder.rb b/app/services/external/vancouver_city/facility_builder.rb index 8203373b..63b5cbed 100644 --- a/app/services/external/vancouver_city/facility_builder.rb +++ b/app/services/external/vancouver_city/facility_builder.rb @@ -3,7 +3,7 @@ # Service for building facility objects from Vancouver City Open Data API records # Inherits from ApplicationService and handles record validation and error recovery class External::VancouverCity::FacilityBuilder < ApplicationService - attr_reader :record, :api_key + attr_reader :facility, :record, :api_key, :mapper ResultData = Struct.new(:facility, keyword_init: true) do def blank? @@ -14,10 +14,12 @@ def blank? # Initialize the builder with required parameters # @param record [Hash] Single API response record # @param api_key [String] One of the supported API keys from External::ApiHelper - def initialize(record:, api_key:) + def initialize(facility:, record:, api_key:) super() + @facility = facility @record = record @api_key = api_key + @mapper = ::External::VancouverCity::FacilityMapper.new(record) end # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity @@ -26,36 +28,29 @@ def initialize(record:, api_key:) def call return Result.new(data: ResultData.new, errors: errors) if invalid? - begin - facility = build_facility_from_record - - # Build facility services - service_builder = External::VancouverCity::FacilityServiceBuilder.new(facility: facility, fields: record, api_key: api_key) - service_result = service_builder.call - service_result.errors.each { |error| add_error(error) } unless service_result.success? - - # Build facility welcomes - welcome_builder = External::VancouverCity::FacilityWelcomeBuilder.new(facility: facility, fields: record) - welcome_result = welcome_builder.call - welcome_result.errors.each { |error| add_error(error) } unless welcome_result.success? - - # Build facility schedules - schedule_builder = External::VancouverCity::FacilityScheduleBuilder.new(facility: facility, fields: record) - schedule_result = schedule_builder.call - schedule_result.errors.each { |error| add_error(error) } unless schedule_result.success? - - if facility&.valid? - Result.new(data: ResultData.new(facility: facility), errors: errors) - else - # rubocop:disable Style/SafeNavigationChainLength - add_error("Facility #{facility&.name} is invalid: #{facility&.errors&.full_messages&.join(', ')}") - # rubocop:enable Style/SafeNavigationChainLength - Result.new(data: ResultData.new, errors: errors) - end - rescue StandardError => e - add_error("Failed to build facility from record: #{e.message}") - Rails.logger.warn "Failed to build facility from record: #{e.message}" - Rails.logger.warn "Record data: #{record.inspect}" + facility.assign_attributes(facility_data_from_record) + + # Build facility services + service_builder = ::External::VancouverCity::FacilityServiceBuilder.new(facility: facility, fields: record, api_key: api_key) + service_result = service_builder.call + service_result.errors.each { |error| add_error(error) } unless service_result.success? + + # Build facility welcomes + welcome_builder = ::External::VancouverCity::FacilityWelcomeBuilder.new(facility: facility, fields: record) + welcome_result = welcome_builder.call + welcome_result.errors.each { |error| add_error(error) } unless welcome_result.success? + + # Build facility schedules + schedule_builder = ::External::VancouverCity::FacilityScheduleBuilder.new(facility: facility, fields: record) + schedule_result = schedule_builder.call + schedule_result.errors.each { |error| add_error(error) } unless schedule_result.success? + + if facility&.valid? + Result.new(data: ResultData.new(facility: facility), errors: errors) + else + # rubocop:disable Style/SafeNavigationChainLength + add_error("Facility '#{facility&.name}' is invalid: #{facility&.errors&.full_messages&.join(', ')}") + # rubocop:enable Style/SafeNavigationChainLength Result.new(data: ResultData.new, errors: errors) end end @@ -70,6 +65,8 @@ def validate add_error("Record is required") elsif !record.is_a?(Hash) add_error("Record must be a Hash") + elsif mapper.external_id(api_key).blank? + add_error("Record is missing external_id for API key '#{api_key}'") elsif !valid_geometry? add_error("Geometry should be either Array with 2 elements or Hash with 'lat' and 'lon' keys") end @@ -77,102 +74,28 @@ def validate private + def coords + mapper.coordinates.presence || mapper.geo_point_2d + end + def valid_geometry? - coordinates.present? || geo_point_2d.present? + coords.present? end # Build a Facility object from an API record # @param record [Hash] Single API response record # @return [Facility, nil] Built Facility object or nil if invalid - def build_facility_from_record - coords = coordinates.presence || geo_point_2d - - facility_data = { - name: extract_name(record), - address: extract_address(record), - phone: extract_phone(record), - website: extract_website(record), - notes: extract_notes(record), - lat: coords[:lat], - long: coords[:long], + def facility_data_from_record + { + name: mapper.name, + address: mapper.address, + phone: mapper.phone, + website: mapper.website, + notes: mapper.notes, + lat: coords.lat, + long: coords.long, verified: true, - external_id: record["mapid"] || "#{api_key}-unknown-id" + external_id: mapper.external_id(api_key) }.compact - - Facility.new(facility_data) - end - - # Extract facility name from fields - # @param fields [Hash] API record fields - # @return [String, nil] Facility name - def extract_name(fields) - name = fields["name"] - return nil unless name - - # Replace special characters with whitespace and clean up - name.gsub("\\n", " ").tr("\n", " ").gsub(/\s+/, " ").strip.presence - end - - # Extract address from fields - # @param fields [Hash] API record fields - # @return [String, nil] Facility address - def extract_address(fields) - # For drinking fountains, use the location field and geo_local_area - location = fields["location"] - area = fields["geo_local_area"] - - [location, area].compact.join(", ").presence - end - - # Extract phone number from fields - # @param fields [Hash] API record fields - # @return [String, nil] Phone number - def extract_phone(fields) - fields["phone"] || fields["phone_number"] || fields["contact_phone"] - end - - # Extract website from fields - # @param fields [Hash] API record fields - # @return [String, nil] Website URL - def extract_website(fields) - fields["website"] || fields["url"] || fields["web_site"] - end - - # Extract notes/description from fields - # @param fields [Hash] API record fields - # @return [String, nil] Notes or description - def extract_notes(fields) - notes_parts = [] - - # Include maintainer info - notes_parts << "Maintained by: #{fields['maintainer']}" if fields["maintainer"].present? - - # Include operation info - notes_parts << "Operation: #{fields['in_operation']}" if fields["in_operation"].present? - - # Include pet friendly info - notes_parts << "Pet friendly: #{fields['pet_friendly']}" if fields["pet_friendly"].present? - - notes_parts.join(". ").presence - end - - # Extract coordinates from geometry - # @return [Hash] Hash with :lat and :long keys - def coordinates - coords = record.dig("geom", "geometry", "coordinates").presence || [] - return {} unless coords.size == 2 - - # GeoJSON coordinates are [longitude, latitude] - { lat: coords[1], long: coords[0] } - end - - # Extract coordinates from geo_point_2d field - # @return [Hash] Hash with :lat and :long keys - def geo_point_2d - geo_point = record["geo_point_2d"].presence || {} - return {} unless geo_point.is_a?(Hash) - return {} unless geo_point.key?("lat") && geo_point.key?("lon") - - { lat: geo_point["lat"], long: geo_point["lon"] } end end diff --git a/app/services/external/vancouver_city/facility_discarder.rb b/app/services/external/vancouver_city/facility_discarder.rb new file mode 100644 index 00000000..b3d1d5a4 --- /dev/null +++ b/app/services/external/vancouver_city/facility_discarder.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +class External::VancouverCity::FacilityDiscarder < ApplicationService + attr_reader :facility + + def initialize(facility) + @facility = facility + super() + end + + def call + # Return early if the facility is already discarded + return build_result if facility.discarded? + + facility.discard_reason = :sync_removed + unless facility.discard + # discard failed, collect errors + add_errors(facility.errors.full_messages) + end + + build_result + end + + private + + # Builds the result object for this discard operation + def build_result + ::External::VancouverCity::Syncer::SyncResultDataEntry.new( + operation: External::SyncOperations.discard, + facility: facility, + errors: errors + ) + end +end diff --git a/app/services/external/vancouver_city/facility_mapper.rb b/app/services/external/vancouver_city/facility_mapper.rb new file mode 100644 index 00000000..51a1d8bf --- /dev/null +++ b/app/services/external/vancouver_city/facility_mapper.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +class External::VancouverCity::FacilityMapper < ApplicationService + attr_reader :record + + def initialize(record) + super() + @record = record + end + + # Extract external_id or fallback to a generated ID using the provided prefix + # @param record [Hash] API response record + # @param prefix [String] Prefix to use if mapid is not present + # @return [String] Generated external_id + def self.external_id(record, prefix = nil) + result = record["mapid"] + generated_id = nil + if result.blank? && prefix.present? + generated_id = "#{prefix}-#{name(record)}".parameterize + Rails.logger.warn "Record is missing 'mapid', generated external_id '#{generated_id}' using prefix '#{prefix}' and name '#{record['name']}'" + end + + result.presence || generated_id + end + + # Extract external_id + # @param prefix [String] Prefix to use if mapid is not present + # @return [String] Generated external_id + def external_id(prefix) + self.class.external_id(record, prefix) + end + + # Extract facility name + # @return [String, nil] Facility name + def self.name(record) + name = record["name"].to_s + return nil if name.blank? + + strip_special_chars(name) + end + + # Replace special characters with whitespace and clean up + def self.strip_special_chars(value) + value.to_s.gsub("\\n", " ").tr("\n", " ").gsub(/\s+/, " ").strip.presence + end + + # Extract facility name + # @return [String, nil] Facility name + def name + self.class.name(record) + end + + # Extract address + # @return [String, nil] Facility address + def address + # For drinking fountains, use the location field and geo_local_area + location = self.class.strip_special_chars(record["location"]) + area = self.class.strip_special_chars(record["geo_local_area"]) + + [location, area].compact.join(", ").presence + end + + # Extract phone number + # @return [String, nil] Phone number + def phone + record["phone"] || record["phone_number"] || record["contact_phone"] + end + + # Extract website + # @return [String, nil] Website URL + def website + record["website"] || record["url"] || record["web_site"] + end + + # Extract notes/description + # @return [String, nil] Notes or description + def notes + notes_parts = [] + + # Include maintainer info + notes_parts << "Maintained by: #{record['maintainer']}" if record["maintainer"].present? + + # Include operation info + notes_parts << "Operation: #{record['in_operation']}" if record["in_operation"].present? + + # Include pet friendly info + notes_parts << "Pet friendly: #{record['pet_friendly']}" if record["pet_friendly"].present? + + notes_parts.join(". ").presence + end + + Coord = Struct.new(:lat, :long, keyword_init: true) + + # Extract coordinates from geometry + # @return [Hash] Hash with :lat and :long keys + def coordinates + coords = record.dig("geom", "geometry", "coordinates").presence || [] + return {} unless coords.size == 2 + + # GeoJSON coordinates are [longitude, latitude] + Coord.new(lat: coords[1], long: coords[0]) + end + + # Extract coordinates from geo_point_2d + # @return [Hash] Hash with :lat and :long keys + def geo_point_2d + geo_point = record["geo_point_2d"].presence || {} + return {} unless geo_point.is_a?(Hash) + return {} unless geo_point.key?("lat") && geo_point.key?("lon") + + Coord.new(lat: geo_point["lat"], long: geo_point["lon"]) + end +end diff --git a/app/services/external/vancouver_city/facility_schedule_builder.rb b/app/services/external/vancouver_city/facility_schedule_builder.rb index 91adc6d0..3ec4e76e 100644 --- a/app/services/external/vancouver_city/facility_schedule_builder.rb +++ b/app/services/external/vancouver_city/facility_schedule_builder.rb @@ -53,7 +53,10 @@ def validate # Add schedules to facility based on business requirements # Creates open-all-day schedules for all weekdays def add_facility_schedules + current_schedules = facility.schedules.index_by(&:week_day) FacilitySchedule.week_days.each_key do |day| + next if current_schedules.key?(day) + facility.schedules.build( week_day: day, closed_all_day: false, diff --git a/app/services/external/vancouver_city/facility_service_builder.rb b/app/services/external/vancouver_city/facility_service_builder.rb index 5394b727..62ff5132 100644 --- a/app/services/external/vancouver_city/facility_service_builder.rb +++ b/app/services/external/vancouver_city/facility_service_builder.rb @@ -66,7 +66,13 @@ def add_facility_services service = Service.find_by(key: service_key) return if service.blank? + return if facility.facility_services.any? { |fs| fs.service == service } + # Build FacilityService association without saving - facility.facility_services.build(service: service) + if facility.new_record? + facility.facility_services.build(service: service) + else + facility.facility_services.create!(service: service) + end end end diff --git a/app/services/external/vancouver_city/facility_syncer.rb b/app/services/external/vancouver_city/facility_syncer.rb index 1ffbd830..e6b96797 100644 --- a/app/services/external/vancouver_city/facility_syncer.rb +++ b/app/services/external/vancouver_city/facility_syncer.rb @@ -3,102 +3,72 @@ # Service for syncing facility data from Vancouver City Open Data API # Inherits from ApplicationService and handles pagination to fetch all facilities class External::VancouverCity::FacilitySyncer < ApplicationService - attr_reader :record, :api_key, :logger + attr_reader :operation, :record, :api_key, :current, :log - ResultData = Struct.new(:operation, :facility, keyword_init: true) do - delegate :present?, :blank?, to: :facility - end - - # rubocop:disable Lint/MissingSuper - def initialize(record:, api_key:, logger: Rails.logger) + def initialize(operation:, record:, api_key:, current:, logger: Rails.logger) + @operation = operation @record = record + @current = current @api_key = api_key - @logger = logger + @log = logger + + super() end - # rubocop:enable Lint/MissingSuper - # rubocop:disable Metrics/AbcSize, Metrics/MethodLength + # rubocop:disable Metrics/AbcSize def call - builder_result = External::VancouverCity::FacilityBuilder.call(record: record, api_key: api_key) + external_id = ::External::VancouverCity::FacilityMapper.external_id(record) + log.info "Processing facility record with external_id '#{external_id}' and name '#{record['name']}' using operation '#{operation.name}'" + builder_result = External::VancouverCity::FacilityBuilder.call(facility: current || Facility.new, record: record, api_key: api_key) if builder_result.failed? + log.warn "FacilityBuilder failed for record with external_id '#{external_id}': #{builder_result.errors.join(', ')}" add_errors(builder_result.errors) return Result.new( - data: ResultData.new(operation: nil, facility: nil), + data: nil, errors: errors ) end - built_facility = builder_result.data[:facility] - existing_facility = Facility.find_by(external_id: built_facility.external_id) - - # If no external_id match, look for name match but prefer internal facilities - if existing_facility.blank? - existing_facility = Facility.where(name: built_facility.name) - .order(Arel.sql("external_id IS NULL DESC, external_id")) - .first - end - operation = if existing_facility.blank? - :create - elsif existing_facility.external? - :external_update - else - :internal_update - end + built_facility = builder_result.data.facility result_facility = nil ApplicationRecord.transaction do case operation - when :external_update - logger.info "Facility with external_id '#{existing_facility.external_id}' already exists, updating services" - update_external_facility(existing_facility, built_facility) - result_facility = existing_facility - when :internal_update - logger.warn "Facility with name '#{existing_facility.name}' already exists internally, adding services" - update_internal_facility(existing_facility, built_facility) - result_facility = existing_facility - when :create - logger.info "Creating new facility with external_id '#{built_facility.external_id}'" - if built_facility.invalid? - add_errors(built_facility.errors) - result_facility = nil - else - built_facility.save! - result_facility = built_facility - end + when External::SyncOperations::ExternalUpdate + log.info "Facility with external_id '#{built_facility.external_id}' already exists, updating services" + update_facility(built_facility) + result_facility = built_facility + when External::SyncOperations::InternalUpdate + log.warn "Facility with name '#{built_facility.name}' already exists internally, adding services" + update_facility(built_facility) + result_facility = built_facility + when External::SyncOperations::Create + log.info "Creating new facility with external_id '#{built_facility.external_id}'" + create_facility(built_facility) + result_facility = built_facility + else + throw ArgumentError.new("Unsupported operation: #{operation}") end rescue ActiveRecord::RecordInvalid => e add_error("Failed to save facility: #{e.message}") result_facility = nil - rescue StandardError => e - add_error("Unexpected error during facility sync: #{e.message}") - result_facility = nil end Result.new( - data: ResultData.new(operation: operation, facility: result_facility), + data: result_facility, errors: errors ) end - # rubocop:enable Metrics/AbcSize, Metrics/MethodLength + # rubocop:enable Metrics/AbcSize private - def update_internal_facility(internal_facility, built_facility) - add_missing_services(internal_facility, built_facility) + def update_facility(facility) + facility.undiscard if facility.discarded? + facility.save! end - def update_external_facility(external_facility, built_facility) - add_missing_services(external_facility, built_facility) - - external_facility.update!(built_facility.attributes.slice("name", "address", "lat", "long", "verified")) - end - - def add_missing_services(existing_facility, built_facility) - built_services = built_facility.facility_services.map(&:service).uniq - existing_services = existing_facility.facility_services.map(&:service).uniq - new_services = built_services - existing_services - new_services.each do |service| - existing_facility.facility_services.create!(service: service) - end + def create_facility(facility) + facility.save! end end diff --git a/app/services/external/vancouver_city/facility_welcome_builder.rb b/app/services/external/vancouver_city/facility_welcome_builder.rb index d585133b..50e58af8 100644 --- a/app/services/external/vancouver_city/facility_welcome_builder.rb +++ b/app/services/external/vancouver_city/facility_welcome_builder.rb @@ -55,7 +55,17 @@ def add_facility_welcomes welcomes = FacilityWelcome.all_customers welcomes.each do |customer_type| + add_welcomes(customer_type) + end + end + + def add_welcomes(customer_type) + return if facility.facility_welcomes.any? { |fw| fw.customer == customer_type.value } + + if facility.new_record? facility.facility_welcomes.build(customer: customer_type.value) + else + facility.facility_welcomes.create!(customer: customer_type.value) end end end diff --git a/app/services/external/vancouver_city/syncer.rb b/app/services/external/vancouver_city/syncer.rb index b8591faa..8600db53 100644 --- a/app/services/external/vancouver_city/syncer.rb +++ b/app/services/external/vancouver_city/syncer.rb @@ -3,25 +3,114 @@ # Service for syncing facility data from Vancouver City Open Data API # Inherits from ApplicationService and handles pagination to fetch all facilities class External::VancouverCity::Syncer < ApplicationService - attr_reader :api_key, :api_client + attr_reader :api_key, :api_client, :full_sync PAGE_SIZE = 50 # Maximum records per request allowed by the API + SyncResultDataEntry = Struct.new(:operation, :facility, :errors, keyword_init: true) do + def success? + errors.blank? + end + + def failed? + errors.present? + end + + # Returns a human-readable error message for this sync result entry + def error_messages + return [] if success? + return "#{operation.name} failed: #{errors.join(', ')}" if facility.blank? + + "#{operation.name} failed for facility '#{facility.name}': #{errors.join(', ')}" + end + end + + SyncResult = Struct.new(:result, keyword_init: true) do + delegate :failed?, :success?, :data, :errors, to: :result + + def partial_failed? + result_errors.flatten.empty? && entry_error_messages.flatten.any? + end + + def error_messages + (result_errors + entry_error_messages).flatten.compact.uniq + end + + private + + def result_errors + result.errors.presence || [] + end + + def entry_error_messages + data.presence&.map(&:error_messages) || [] + end + end # Initialize the syncer with required parameters # @param api_key [String] One of the supported API keys from External::ApiHelper # @param api_client [VancouverApiClient] The API client instance - def initialize(api_key:, api_client:) + # @param full_sync [Boolean] Whether to perform a full sync (discard missing facilities) + def initialize(api_key:, api_client:, full_sync: true) super() @api_key = api_key @api_client = api_client + @full_sync = full_sync end # Main method that performs the sync operation # @return [ApplicationService::Result] Result object with data and errors def call - return Result.new(data: nil, errors: errors) if invalid? + return build_result([], errors) if invalid? + + Rails.logger.info "Starting sync for #{api_key} API (full_sync: #{full_sync})" + + sync_results = sync_facilities_from_api + synced_external_ids = sync_results + .select(&:success?) + .map { |result| result.facility.external_id } + success_count = synced_external_ids.size + failed_count = sync_results.size - success_count + + discard_results = discard_missing_facilities(synced_external_ids) + discard_success_count = discard_results.count { |result| result.errors.blank? } + discard_failed_count = discard_results.size - discard_success_count + + Rails.logger.info "Finished sync for #{api_key} API: #{success_count} succeeded, #{failed_count} failed, #{discard_success_count} discarded, #{discard_failed_count} failed to discard" + + build_result(sync_results + discard_results, errors) + end + + # Validates the input parameters + # @return [Array] Array of error messages + def validate + @errors = [] + + add_error("Unsupported API: #{api_key}") unless External::ApiHelper.supported_api?(api_key) + + if api_client.nil? + add_error("API client is required") + elsif !api_client.is_a?(External::VancouverCity::VancouverApiClient) + add_error("API client must be an instance of VancouverApiClient") + end + + errors + end + + private - facilities = [] + def build_result(result_data, result_errors) + SyncResult.new( + result: Result.new( + data: result_data, + errors: result_errors + ) + ) + end + + # Syncs facilities from the API with pagination + # @return [Array] Array containing facilities, synced_external_ids, created_count, updated_count + def sync_facilities_from_api + sync_results = [] offset = 0 loop do @@ -34,68 +123,106 @@ def call break if records.empty? # Process each record and build Facility objects - batch_facilities = process_records(records) - facilities.concat(batch_facilities) + batch_results = process_records_with_operations(records) + sync_results += batch_results # If we got fewer records than the limit, we've reached the end break if records.size < PAGE_SIZE offset += PAGE_SIZE rescue External::VancouverCity::VancouverApiError => e - add_error("API request failed: #{e.message}") + sync_results << SyncResultDataEntry.new( + operation: External::SyncOperations.api_call, + facility: nil, + errors: ["API request failed: #{e.message}"] + ) break rescue StandardError => e - add_error("Unexpected error during sync: #{e.message}") + sync_results << SyncResultDataEntry.new( + operation: External::SyncOperations.unexpected_error, + facility: nil, + errors: ["Unexpected error during sync; #{e.class}: #{e.message}"] + ) + # raise break end end - Rails.logger.info "Successfully processed #{facilities.size} facilities from #{api_key} API" - - Result.new( - data: { - facilities: facilities, - total_count: facilities.size, - api_key: api_key - }, - errors: errors - ) + sync_results end - # Validates the input parameters - # @return [Array] Array of error messages - def validate - @errors = [] - - add_error("Unsupported API: #{api_key}") unless External::ApiHelper.supported_api?(api_key) + # Process API records and return ResultData objects with operations + # @param records [Array] Array of API response records + # @return [Array] Array of result data objects + def process_records_with_operations(records) + external_ids = records.filter_map { |record| ::External::VancouverCity::FacilityMapper.external_id(record) } + names = records.filter_map { |record| ::External::VancouverCity::FacilityMapper.name(record) } + existing_facilities = Facility.with_associations + .with_discarded + .where(external_id: external_ids) + .to_a + existing_facilities_by_name = Facility.with_associations + .with_discarded + .where(name: names) + .to_a + + records.map do |record| + process_single_record(record.with_indifferent_access, existing_facilities, existing_facilities_by_name) + end + end - if api_client.nil? - add_error("API client is required") - elsif !api_client.is_a?(External::VancouverCity::VancouverApiClient) - add_error("API client must be an instance of VancouverApiClient") + def process_single_record(record, existing_facilities, existing_facilities_by_name) + external_id = ::External::VancouverCity::FacilityMapper.external_id(record) + if external_id.nil? + return SyncResultDataEntry.new( + operation: External::SyncOperations.unknown, + facility: nil, + errors: ["Missing external_id for record with name '#{record['name']}'"] + ) end - errors + current_facility = existing_facilities.find { |f| f.external_id == external_id } || + existing_facilities_by_name.find { |f| f.name == ::External::VancouverCity::FacilityMapper.name(record) } + operation = if current_facility.blank? + External::SyncOperations.create + elsif current_facility.external? + External::SyncOperations.external_update + else + External::SyncOperations.internal_update + end + + syncer_result = External::VancouverCity::FacilitySyncer.call( + operation: operation, + record: record, + current: current_facility, + api_key: api_key, + logger: Rails.logger + ) + SyncResultDataEntry.new( + operation: operation, + facility: syncer_result.data, + errors: syncer_result.errors + ) end - private + # Discard facilities that were not in the API response (full sync only) + # @param synced_external_ids [Array] Array of external_ids that were in the response + # @return [Integer] Number of facilities discarded + def discard_missing_facilities(synced_external_ids) + return [] unless full_sync - # Process API records and convert them to Facility objects - # @param records [Array] Array of API response records - # @return [Array] Array of built Facility objects - def process_records(records) - facilities = [] + results = [] + External::SyncOperations.discard - records.each do |record| - syncer_result = External::VancouverCity::FacilitySyncer.call(record: record, api_key: api_key) + # Only discard facilities that are currently kept + # (not already discarded with deleted_at set) + missing_facilities = Facility.external.kept + .where.not(external_id: synced_external_ids) - if syncer_result.success? - facilities << syncer_result.data[:facility] - else - add_errors(syncer_result.errors) - end + missing_facilities.find_each do |facility| + results << External::VancouverCity::FacilityDiscarder.call(facility) end - facilities + results end end diff --git a/app/views/admin/tools/index.html.erb b/app/views/admin/tools/index.html.erb index 72f3707e..20078bfa 100644 --- a/app/views/admin/tools/index.html.erb +++ b/app/views/admin/tools/index.html.erb @@ -10,13 +10,32 @@
-
-
- <%= render Shared::CardComponent.new(title: "Vancouver City API") do |card| %> +

Vancouver City API

+ + +
+
+ +
+ + +
+ <%= render Shared::CardComponent.new(title: "Import Facilities") do |card| %>

Choose which facilities to import from Vancouver City's Open Data API.

- <%= form_with url: import_facilities_admin_tools_path, method: :post, class: "form", id: "import-form" do |form| %> + <%= form_with url: import_facilities_admin_tools_path, method: :post, class: "form", id: "import-form", data: { action: "progress#submit", operation: "import" } do |form| %>
<%= form.label :api, "API Endpoint", class: "label" %>
@@ -31,48 +50,66 @@
- <%= form.submit "Import Facilities", class: "button is-primary", id: "import-button", data: { disable_with: "Importing..." } %> + <%= form.submit "Import Facilities", + class: "button is-primary", + id: "import-button", + data: { disable_with: "Importing..." } %>
+
+

Note: Import performs a full sync, which removes any facilities that no longer exist in the API.

+
<% end %> +
+ <% end %> +
- -
External ID:<%= facility.external_id %>
Website: <%= link_to_website %>