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/app/components/facilities/discard_reason_component.rb b/app/components/facilities/discard_reason_component.rb index e978c750..d42f93a7 100644 --- a/app/components/facilities/discard_reason_component.rb +++ b/app/components/facilities/discard_reason_component.rb @@ -4,9 +4,11 @@ class Facilities::DiscardReasonComponent < ViewComponent::Base attr_reader :discard_reason VALID_REASONS = { + nil => "None", none: "None", closed: "Closed", - duplicated: "Duplicated" + duplicated: "Duplicated", + sync_removed: "Removed by Sync" }.freeze def initialize(discard_reason) @@ -20,7 +22,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..454996d6 100644 --- a/app/controllers/admin/facilities_controller.rb +++ b/app/controllers/admin/facilities_controller.rb @@ -104,13 +104,13 @@ def load_facilities ) end - facilities = facilities.order(updated_at: :asc) + facilities = facilities.with_associations.order(updated_at: :desc) @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..72c0f4c1 100644 --- a/app/controllers/admin/tools_controller.rb +++ b/app/controllers/admin/tools_controller.rb @@ -16,18 +16,40 @@ 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)}." + created = result.data[:created_count] || 0 + updated = result.data[:updated_count] || 0 + deleted = result.data[:deleted_count] || 0 + redirect_to admin_facilities_path(service: "water_fountain"), notice: "Sync complete: #{created} created, #{updated} updated, #{deleted} removed from #{External::ApiHelper.api_name(api_key)}." else error_messages = result.errors.join(", ") redirect_to admin_tools_path, alert: "Failed to import facilities: #{error_messages}" end end + def discard_facilities + api_key = params[:api] + + unless External::ApiHelper.supported_api?(api_key) + redirect_to admin_tools_path, alert: "Invalid API selected. Please choose from the supported APIs." + return + end + + result = External::VancouverCity::DiscardService.call(api_key: api_key) + + if result.success? + discarded_count = result.data[:discarded_count] || 0 + redirect_to admin_facilities_path(service: "water_fountain"), notice: "#{discarded_count} facilities discarded from #{External::ApiHelper.api_name(api_key)}." + else + error_messages = result.errors.join(", ") + redirect_to admin_tools_path, alert: "Failed to discard facilities: #{error_messages}" + end + end + # Helper method for the view helper_method :api_options_for_select 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..86b27d08 100644 --- a/app/models/facility.rb +++ b/app/models/facility.rb @@ -17,12 +17,13 @@ class Facility < ApplicationRecord 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/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/vancouver_city/discard_service.rb b/app/services/external/vancouver_city/discard_service.rb new file mode 100644 index 00000000..3b1afb66 --- /dev/null +++ b/app/services/external/vancouver_city/discard_service.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class External::VancouverCity::DiscardService < ApplicationService + attr_reader :api_key + + def initialize(api_key:) + super() + @api_key = api_key + end + + def call + return failure(["Unsupported API: #{api_key}"]) unless External::ApiHelper.supported_api?(api_key) + + discarded_count = 0 + + Facility.external.kept.find_each do |facility| + facility.discard_reason = :sync_removed + facility.discard! + discarded_count += 1 + end + + success({ discarded_count: discarded_count }) + end +end diff --git a/app/services/external/vancouver_city/facility_syncer.rb b/app/services/external/vancouver_city/facility_syncer.rb index 1ffbd830..6bf0e3d7 100644 --- a/app/services/external/vancouver_city/facility_syncer.rb +++ b/app/services/external/vancouver_city/facility_syncer.rb @@ -3,19 +3,20 @@ # 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 :record, :api_key, :current, :logger 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(record:, api_key:, current:, logger: Rails.logger) @record = record + @current = current @api_key = api_key @logger = logger + + super() end - # rubocop:enable Lint/MissingSuper # rubocop:disable Metrics/AbcSize, Metrics/MethodLength def call @@ -29,14 +30,7 @@ def call 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 + existing_facility = current operation = if existing_facility.blank? :create elsif existing_facility.external? @@ -84,12 +78,15 @@ def call private def update_internal_facility(internal_facility, built_facility) + internal_facility.undiscard if internal_facility.discarded? + add_missing_services(internal_facility, built_facility) end def update_external_facility(external_facility, built_facility) - add_missing_services(external_facility, built_facility) + external_facility.undiscard if external_facility.discarded? + add_missing_services(external_facility, built_facility) external_facility.update!(built_facility.attributes.slice("name", "address", "lat", "long", "verified")) end @@ -97,6 +94,7 @@ 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 diff --git a/app/services/external/vancouver_city/syncer.rb b/app/services/external/vancouver_city/syncer.rb index b8591faa..34fb6ce7 100644 --- a/app/services/external/vancouver_city/syncer.rb +++ b/app/services/external/vancouver_city/syncer.rb @@ -3,17 +3,19 @@ # 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 # 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 @@ -21,7 +23,40 @@ def initialize(api_key:, api_client:) def call return Result.new(data: nil, errors: errors) if invalid? + facilities, synced_external_ids, created_count, updated_count = sync_facilities_from_api + + discarded_count = discard_missing_facilities(synced_external_ids) + + Rails.logger.info "Successfully processed #{facilities.size} facilities from #{api_key} API" + + build_result(facilities, created_count, updated_count, discarded_count) + 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 + + # Syncs facilities from the API with pagination + # @return [Array] Array containing facilities, synced_external_ids, created_count, updated_count + def sync_facilities_from_api facilities = [] + synced_external_ids = [] + created_count = 0 + updated_count = 0 offset = 0 loop do @@ -34,8 +69,17 @@ 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) + batch_results.each do |result| + facilities << result.facility + synced_external_ids << result.facility.external_id if result.facility.respond_to?(:external_id) + case result.operation + when :create + created_count += 1 + when :external_update, :internal_update + updated_count += 1 + end + end # If we got fewer records than the limit, we've reached the end break if records.size < PAGE_SIZE @@ -50,52 +94,111 @@ def call end end - Rails.logger.info "Successfully processed #{facilities.size} facilities from #{api_key} API" + [facilities, synced_external_ids, created_count, updated_count] + end + # Builds the result object for the sync operation + # @param facilities [Array] Array of synced facilities + # @param created_count [Integer] Number of created facilities + # @param updated_count [Integer] Number of updated facilities + # @param discarded_count [Integer] Number of discarded facilities + # @return [ApplicationService::Result] Result object with data and errors + def build_result(facilities, created_count, updated_count, discarded_count) Result.new( data: { facilities: facilities, total_count: facilities.size, + created_count: created_count, + updated_count: updated_count, + deleted_count: discarded_count, api_key: api_key }, errors: errors ) end - # Validates the input parameters - # @return [Array] Array of error messages - def validate - @errors = [] + # # 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 = [] - add_error("Unsupported API: #{api_key}") unless External::ApiHelper.supported_api?(api_key) + # records.each do |record| + # syncer_result = External::VancouverCity::FacilitySyncer.call(record: record, api_key: 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 + # if syncer_result.success? + # facilities << syncer_result.data[:facility] + # else + # add_errors(syncer_result.errors) + # end + # end - private + # facilities + # end - # Process API records and convert them to Facility objects + # Process API records and return ResultData objects with operations # @param records [Array] Array of API response records - # @return [Array] Array of built Facility objects - def process_records(records) - facilities = [] + # @return [Array] Array of result data objects + def process_records_with_operations(records) + results = [] + + external_ids = records.pluck("mapid").compact + existing_facilities = Facility.with_associations + .with_discarded + .where(external_id: external_ids) + .to_a + # existing_facility = Facility.with_discarded + # .find_by(external_id: built_facility.external_id) + # Need to also load facilities that match by name in case external_id is missing or changed, but prefer matches by external_id when available + # Facility.with_discarded + # .where(name: built_facility.name) + # .order(Arel.sql("external_id IS NULL DESC, external_id")) + # .first records.each do |record| - syncer_result = External::VancouverCity::FacilitySyncer.call(record: record, api_key: api_key) + current_facility = existing_facilities.find { |f| f.external_id == record["mapid"] } + syncer_result = External::VancouverCity::FacilitySyncer.call(record: record, current: current_facility, api_key: api_key) if syncer_result.success? - facilities << syncer_result.data[:facility] + data = syncer_result.data + # Support both ResultData objects and legacy hash format + results << if data.respond_to?(:operation) + data + else + # Legacy hash format: { facility: ... } + External::VancouverCity::FacilitySyncer::ResultData.new( + operation: nil, + facility: data[:facility] + ) + end else add_errors(syncer_result.errors) end end - facilities + results + end + + # 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 0 unless full_sync + + # Only discard facilities that: + # 1. Are currently kept (not already discarded with deleted_at set) + # 2. Have not been previously marked as sync_removed + missing_facilities = Facility.external.kept + .where.not(external_id: synced_external_ids) + .where("discard_reason IS NULL OR discard_reason != ?", "sync_removed") + + count = 0 + missing_facilities.find_each do |facility| + facility.discard_reason = :sync_removed + facility.discard! + count += 1 + end + + count 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 %>