From 9ceb6354cc15dbe841d4badb512d5fe483fd6941 Mon Sep 17 00:00:00 2001 From: Sagar Shinde Date: Mon, 9 Feb 2026 11:25:04 +0530 Subject: [PATCH 01/12] SIMPLEBACK-34: Add sync API for patient scores (read-only) - Add migration to create patient_scores table (with existence check) - Add PatientScore model with Mergeable concern - Add Api::V4::PatientScoresController with sync_to_user only - Add PatientScoreTransformer for response transformation - Add patient_score schema to Api::V4::Models - Add GET route for /patient_scores/sync - Add factory and controller spec for patient_scores (sync_to_user only) --- .../api/v4/patient_scores_controller.rb | 11 ++++ app/models/patient_score.rb | 13 +++++ app/schema/api/v4/models.rb | 16 ++++++ .../api/v4/patient_score_transformer.rb | 19 +++++++ config/routes.rb | 4 ++ .../20260209112204_create_patient_scores.rb | 24 +++++++++ db/structure.sql | 50 ++++++++++++++++++- .../api/v4/patient_scores_controller_spec.rb | 27 ++++++++++ spec/factories/patient_scores.rb | 10 ++++ 9 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 app/controllers/api/v4/patient_scores_controller.rb create mode 100644 app/models/patient_score.rb create mode 100644 app/transformers/api/v4/patient_score_transformer.rb create mode 100644 db/migrate/20260209112204_create_patient_scores.rb create mode 100644 spec/controllers/api/v4/patient_scores_controller_spec.rb create mode 100644 spec/factories/patient_scores.rb diff --git a/app/controllers/api/v4/patient_scores_controller.rb b/app/controllers/api/v4/patient_scores_controller.rb new file mode 100644 index 0000000000..ec8d1a1d3b --- /dev/null +++ b/app/controllers/api/v4/patient_scores_controller.rb @@ -0,0 +1,11 @@ +class Api::V4::PatientScoresController < Api::V4::SyncController + def sync_to_user + __sync_to_user__("patient_scores") + end + + private + + def transform_to_response(patient_score) + Api::V4::PatientScoreTransformer.to_response(patient_score) + end +end diff --git a/app/models/patient_score.rb b/app/models/patient_score.rb new file mode 100644 index 0000000000..e43576bc00 --- /dev/null +++ b/app/models/patient_score.rb @@ -0,0 +1,13 @@ +class PatientScore < ApplicationRecord + include Mergeable + include Discard::Model + + belongs_to :patient, optional: true + + validates :device_created_at, presence: true + validates :device_updated_at, presence: true + validates :score_type, presence: true + validates :score_value, presence: true, numericality: true + + scope :for_sync, -> { with_discarded } +end diff --git a/app/schema/api/v4/models.rb b/app/schema/api/v4/models.rb index caac4c712c..7b70e2418e 100644 --- a/app/schema/api/v4/models.rb +++ b/app/schema/api/v4/models.rb @@ -117,6 +117,20 @@ def patient_attribute required: %w[id patient_id height weight created_at updated_at]} end + def patient_score + {type: :object, + properties: { + id: {"$ref" => "#/definitions/uuid"}, + patient_id: {"$ref" => "#/definitions/uuid"}, + score_type: {"$ref" => "#/definitions/non_empty_string"}, + score_value: {type: :number}, + deleted_at: {"$ref" => "#/definitions/nullable_timestamp"}, + created_at: {"$ref" => "#/definitions/timestamp"}, + updated_at: {"$ref" => "#/definitions/timestamp"} + }, + required: %w[id patient_id score_type score_value created_at updated_at]} + end + def patient_phone_number { type: :object, @@ -458,6 +472,8 @@ def definitions patient: patient, patient_attribute: patient_attribute, patient_attributes: Api::CommonDefinitions.array_of("patient_attribute"), + patient_score: patient_score, + patient_scores: Api::CommonDefinitions.array_of("patient_score"), patient_business_identifier: Api::V3::Models.patient_business_identifier, patient_business_identifiers: Api::CommonDefinitions.array_of("patient_business_identifier"), phone_number: Api::V3::Models.phone_number, diff --git a/app/transformers/api/v4/patient_score_transformer.rb b/app/transformers/api/v4/patient_score_transformer.rb new file mode 100644 index 0000000000..93dc2c4711 --- /dev/null +++ b/app/transformers/api/v4/patient_score_transformer.rb @@ -0,0 +1,19 @@ +class Api::V4::PatientScoreTransformer < Api::V4::Transformer + class << self + def to_response(payload) + super(payload) + .merge({ + "score_type" => payload["score_type"], + "score_value" => payload["score_value"].to_f + }) + end + + def from_request(payload) + super(payload) + .merge({ + "score_type" => payload["score_type"], + "score_value" => payload["score_value"].to_f + }) + end + end +end diff --git a/config/routes.rb b/config/routes.rb index d48555010f..1fe42b775a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -71,6 +71,10 @@ get "sync", to: "cvd_risks#sync_to_user" post "sync", to: "cvd_risks#sync_from_user" end + + scope :patient_scores do + get "sync", to: "patient_scores#sync_to_user" + end end namespace :webview do diff --git a/db/migrate/20260209112204_create_patient_scores.rb b/db/migrate/20260209112204_create_patient_scores.rb new file mode 100644 index 0000000000..d6b687bf9b --- /dev/null +++ b/db/migrate/20260209112204_create_patient_scores.rb @@ -0,0 +1,24 @@ +class CreatePatientScores < ActiveRecord::Migration[6.1] + def change + unless table_exists?(:patient_scores) + create_table :patient_scores, id: :uuid do |t| + t.references :patient, null: false, foreign_key: true, type: :uuid + t.string :score_type, null: false, limit: 100 + t.decimal :score_value, precision: 5, scale: 2, null: false + t.datetime :device_created_at, null: false + t.datetime :device_updated_at, null: false + t.datetime :deleted_at + + t.timestamps + end + end + + unless index_exists?(:patient_scores, [:patient_id, :score_type]) + add_index :patient_scores, [:patient_id, :score_type] + end + + unless index_exists?(:patient_scores, :updated_at) + add_index :patient_scores, :updated_at + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 921856ce3c..b7610bb8d7 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1959,6 +1959,23 @@ CREATE TABLE public.patient_phone_numbers ( ); +-- +-- Name: patient_scores; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.patient_scores ( + id uuid NOT NULL, + patient_id uuid NOT NULL, + score_type character varying(100) NOT NULL, + score_value numeric(5,2) NOT NULL, + device_created_at timestamp without time zone NOT NULL, + device_updated_at timestamp without time zone NOT NULL, + deleted_at timestamp without time zone, + created_at timestamp(6) without time zone NOT NULL, + updated_at timestamp(6) without time zone NOT NULL +); + + -- -- Name: prescription_drugs; Type: TABLE; Schema: public; Owner: - -- @@ -6596,6 +6613,14 @@ ALTER TABLE ONLY public.patient_phone_numbers ADD CONSTRAINT patient_phone_numbers_pkey PRIMARY KEY (id); +-- +-- Name: patient_scores patient_scores_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.patient_scores + ADD CONSTRAINT patient_scores_pkey PRIMARY KEY (id); + + -- -- Name: patients patients_pkey; Type: CONSTRAINT; Schema: public; Owner: - -- @@ -7699,6 +7724,20 @@ CREATE INDEX index_patient_phone_numbers_on_dnd_status ON public.patient_phone_n CREATE INDEX index_patient_phone_numbers_on_patient_id ON public.patient_phone_numbers USING btree (patient_id); +-- +-- Name: index_patient_scores_on_patient_id_and_score_type; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_patient_scores_on_patient_id_and_score_type ON public.patient_scores USING btree (patient_id, score_type); + + +-- +-- Name: index_patient_scores_on_updated_at; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_patient_scores_on_updated_at ON public.patient_scores USING btree (updated_at); + + -- -- Name: index_patient_registrations_per_day_per_facilities; Type: INDEX; Schema: public; Owner: - -- @@ -8242,6 +8281,14 @@ ALTER TABLE ONLY public.patient_phone_numbers ADD CONSTRAINT fk_rails_0145dd0b05 FOREIGN KEY (patient_id) REFERENCES public.patients(id); +-- +-- Name: patient_scores fk_rails_0209112204; Type: FK CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.patient_scores + ADD CONSTRAINT fk_rails_0209112204 FOREIGN KEY (patient_id) REFERENCES public.patients(id); + + -- -- Name: facility_groups fk_rails_0ba9e6af98; Type: FK CONSTRAINT; Schema: public; Owner: - -- @@ -8802,5 +8849,6 @@ INSERT INTO "schema_migrations" (version) VALUES ('20251211154907'), ('20251215113615'), ('20251219061210'), -('20260128094448'); +('20260128094448'), +('20260209112204'); diff --git a/spec/controllers/api/v4/patient_scores_controller_spec.rb b/spec/controllers/api/v4/patient_scores_controller_spec.rb new file mode 100644 index 0000000000..c153ec671e --- /dev/null +++ b/spec/controllers/api/v4/patient_scores_controller_spec.rb @@ -0,0 +1,27 @@ +require "rails_helper" + +describe Api::V4::PatientScoresController, type: :controller do + let(:request_user) { create(:user) } + let(:request_facility_group) { request_user.facility.facility_group } + let(:request_facility) { create(:facility, facility_group: request_facility_group) } + let(:model) { PatientScore } + + def create_record(options = {}) + facility = create(:facility, facility_group: request_facility_group) + patient = create(:patient, registration_facility: facility) + create(:patient_score, options.merge(patient: patient)) + end + + def create_record_list(n, options = {}) + facility = create(:facility, facility_group_id: request_facility_group.id) + patient = create(:patient, registration_facility_id: facility.id) + create_list(:patient_score, n, options.merge(patient: patient)) + end + + it_behaves_like "a sync controller that authenticates user requests: sync_to_user" + it_behaves_like "a sync controller that audits the data access: sync_to_user" + + describe "GET sync: send data from server to device;" do + it_behaves_like "a working V3 sync controller sending records" + end +end diff --git a/spec/factories/patient_scores.rb b/spec/factories/patient_scores.rb new file mode 100644 index 0000000000..7e82fb102f --- /dev/null +++ b/spec/factories/patient_scores.rb @@ -0,0 +1,10 @@ +FactoryBot.define do + factory :patient_score do + id { SecureRandom.uuid } + patient + score_type { "risk_score" } + score_value { 75.50 } + device_created_at { Time.current } + device_updated_at { Time.current } + end +end From 19c7acf560e26a2ab22e5aa46c94fbc35057b37e Mon Sep 17 00:00:00 2001 From: Sagar Shinde Date: Mon, 9 Mar 2026 11:31:41 +0530 Subject: [PATCH 02/12] Fixed.The issue was that db/structure.sql had a premature semicolon at line 9724, which terminated the INSERT INTO schema_migrations statement early. The migration versions on lines 9725-9729 were left as orphaned SQL, causing the syntax error. --- db/structure.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/structure.sql b/db/structure.sql index 80addf2f40..64151dcfb7 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9721,7 +9721,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20251215113615'), ('20251219061210'), ('20260128094448'), -('20260209112204'); +('20260209112204'), ('20260212195326'), ('20260205110957'), ('20260224063659'), From 4d43f018a833be1c4f04b2b0ef1a8b71e2869ae8 Mon Sep 17 00:00:00 2001 From: Sagar Shinde Date: Tue, 17 Mar 2026 11:32:21 +0530 Subject: [PATCH 03/12] Add district/facility filter to patient merge screen --- .../admin/deduplicate_patients_controller.rb | 48 ++++++++++++++++++- .../admin/deduplicate_patients/show.html.erb | 32 +++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/deduplicate_patients_controller.rb b/app/controllers/admin/deduplicate_patients_controller.rb index 0cbd71d60e..f6bf01abe8 100644 --- a/app/controllers/admin/deduplicate_patients_controller.rb +++ b/app/controllers/admin/deduplicate_patients_controller.rb @@ -1,18 +1,28 @@ class Admin::DeduplicatePatientsController < AdminController skip_before_action :verify_authenticity_token + before_action :set_filter_options, only: [:show] DUPLICATE_LIMIT = 250 def show facilities = current_admin.accessible_facilities(:manage) authorize { facilities.any? } + # Apply facility filter if selected + filtered_facilities = if @selected_facility.present? + facilities.where(id: @selected_facility.id) + elsif @selected_district.present? + facilities.where(id: @selected_district.facilities) + else + facilities + end + # Scoping by facilities is costly for users who have a lot of facilities - duplicate_patient_ids = if current_admin.accessible_organizations(:manage).any? + duplicate_patient_ids = if current_admin.accessible_organizations(:manage).any? && !@selected_district.present? && !@selected_facility.present? PatientDeduplication::Strategies.identifier_excluding_full_name_match(limit: DUPLICATE_LIMIT) else PatientDeduplication::Strategies.identifier_excluding_full_name_match_for_facilities( limit: DUPLICATE_LIMIT, - facilities: facilities + facilities: filtered_facilities ) end @@ -44,4 +54,38 @@ def can_admin_deduplicate_patients?(patients) .where(id: patients.pluck(:assigned_facility_id)) .any? end + + private + + def set_filter_options + @accessible_facilities = current_admin.accessible_facilities(:manage) + populate_districts + set_selected_district + populate_facilities + set_selected_facility + end + + def populate_districts + @districts = Region.district_regions + .joins("INNER JOIN regions facility_region ON regions.path @> facility_region.path") + .where("facility_region.source_id" => @accessible_facilities.map(&:id)) + .distinct(:slug) + .order(:name) + end + + def set_selected_district + @selected_district = if params[:district_slug].present? + @districts.find_by(slug: params[:district_slug]) + elsif @districts.present? + @districts.first + end + end + + def populate_facilities + @facilities = @accessible_facilities.where(id: @selected_district&.facilities).order(:name) + end + + def set_selected_facility + @selected_facility = @facilities.find_by(id: params[:facility_id]) if params[:facility_id].present? + end end diff --git a/app/views/admin/deduplicate_patients/show.html.erb b/app/views/admin/deduplicate_patients/show.html.erb index 5532fb1bd9..529beeb9b0 100644 --- a/app/views/admin/deduplicate_patients/show.html.erb +++ b/app/views/admin/deduplicate_patients/show.html.erb @@ -1,4 +1,36 @@

Merge duplicate patients

+ +<%= bootstrap_form_with(url: admin_deduplication_path, method: :get, layout: :horizontal, class: "mb-4") do |form| %> + <% html_select_options = { onchange: "this.form.submit();" } + searchable_select_options = html_select_options.merge(class: "selectpicker", data: {live_search: true}) %> +
+
+ <%= form.select :district_slug, + @districts.order(:name).map { |district| [district.name, district.slug] }, + { + hide_label: true, + selected: @selected_district&.slug, + wrapper: false + }, + searchable_select_options + %> +
+
+ <%= form.select :facility_id, + @facilities.order(:name).map { |facility| [facility.label_with_district, facility.id] }, + { + hide_label: true, + include_blank: "All facilities", + selected: @selected_facility&.id, + wrapper: false + }, + searchable_select_options + %> +
+
+ <%= form.submit "Filter", style: "display: none" %> +<% end %> +

<% if @duplicate_count == Admin::DeduplicatePatientsController::DUPLICATE_LIMIT %> From cac5c00ec5875472ef65fa1d99166ed99d710a38 Mon Sep 17 00:00:00 2001 From: Sagar Shinde Date: Thu, 19 Mar 2026 14:14:53 +0530 Subject: [PATCH 04/12] Add sync methods and timestamps to patient scores API --- .../api/v4/patient_scores_controller.rb | 20 +++++++++++++++++++ .../api/v4/patient_score_transformer.rb | 5 ++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v4/patient_scores_controller.rb b/app/controllers/api/v4/patient_scores_controller.rb index ec8d1a1d3b..e2c50c9b96 100644 --- a/app/controllers/api/v4/patient_scores_controller.rb +++ b/app/controllers/api/v4/patient_scores_controller.rb @@ -3,6 +3,26 @@ def sync_to_user __sync_to_user__("patient_scores") end + def current_facility_records + @current_facility_records ||= + PatientScore + .for_sync + .where(patient: current_facility.prioritized_patients.select(:id)) + .updated_on_server_since(current_facility_processed_since, limit) + end + + def other_facility_records + other_facilities_limit = limit - current_facility_records.size + @other_facility_records ||= + PatientScore + .for_sync + .where(patient_id: current_sync_region + .syncable_patients + .where.not(registration_facility: current_facility) + .select(:id)) + .updated_on_server_since(other_facilities_processed_since, other_facilities_limit) + end + private def transform_to_response(patient_score) diff --git a/app/transformers/api/v4/patient_score_transformer.rb b/app/transformers/api/v4/patient_score_transformer.rb index 93dc2c4711..46e3cf8612 100644 --- a/app/transformers/api/v4/patient_score_transformer.rb +++ b/app/transformers/api/v4/patient_score_transformer.rb @@ -1,10 +1,13 @@ class Api::V4::PatientScoreTransformer < Api::V4::Transformer class << self def to_response(payload) + current_time = Time.current.iso8601 super(payload) .merge({ "score_type" => payload["score_type"], - "score_value" => payload["score_value"].to_f + "score_value" => payload["score_value"].to_f, + "created_at" => current_time, + "updated_at" => current_time }) end From 3ef858ac5b513e2189c3d0a681ff7b4fd78bb4cb Mon Sep 17 00:00:00 2001 From: Sagar Shinde Date: Mon, 13 Apr 2026 12:06:11 +0530 Subject: [PATCH 05/12] fix: Use actual record timestamps in PatientScoreTransformer Remove hardcoded Time.current for created_at/updated_at and let the base transformer use the actual device_created_at/device_updated_at from the record. This fixes pagination issues in sync. --- app/transformers/api/v4/patient_score_transformer.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/transformers/api/v4/patient_score_transformer.rb b/app/transformers/api/v4/patient_score_transformer.rb index 46e3cf8612..93dc2c4711 100644 --- a/app/transformers/api/v4/patient_score_transformer.rb +++ b/app/transformers/api/v4/patient_score_transformer.rb @@ -1,13 +1,10 @@ class Api::V4::PatientScoreTransformer < Api::V4::Transformer class << self def to_response(payload) - current_time = Time.current.iso8601 super(payload) .merge({ "score_type" => payload["score_type"], - "score_value" => payload["score_value"].to_f, - "created_at" => current_time, - "updated_at" => current_time + "score_value" => payload["score_value"].to_f }) end From daa7ab6797efdeab7f65898e83008d924a8025c4 Mon Sep 17 00:00:00 2001 From: Sagar Shinde Date: Mon, 13 Apr 2026 12:28:37 +0530 Subject: [PATCH 06/12] fix: Remove duplicate migration entry in structure.sql Remove duplicate entry for migration 20260212195326 and fix chronological ordering of migration entries. --- db/structure.sql | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/db/structure.sql b/db/structure.sql index ac341c5186..7c708197e8 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -9723,9 +9723,8 @@ INSERT INTO "schema_migrations" (version) VALUES ('20260120115014'), ('20260127150000'), ('20260128094448'), -('20260209112204'), -('20260212195326'), ('20260205110957'), +('20260209112204'), ('20260212195326'), ('20260224063659'), ('20260316093605'), From 8489fba3dce61540e8400a5c1c092f394f8960de Mon Sep 17 00:00:00 2001 From: Sagar Shinde Date: Tue, 14 Apr 2026 10:33:47 +0530 Subject: [PATCH 07/12] fix: Spread clustered PatientScore updated_at timestamps Add data migration to fix sync pagination issue where bulk-inserted PatientScores share the same updated_at timestamp, causing the process_token to never advance. The migration: 1. Finds timestamps with >1000 records clustered 2. If device_updated_at is well-distributed, uses that 3. Otherwise spreads by id with millisecond offsets --- ...103256_spread_patient_scores_updated_at.rb | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 db/data/20260414103256_spread_patient_scores_updated_at.rb diff --git a/db/data/20260414103256_spread_patient_scores_updated_at.rb b/db/data/20260414103256_spread_patient_scores_updated_at.rb new file mode 100644 index 0000000000..f299a8b6bd --- /dev/null +++ b/db/data/20260414103256_spread_patient_scores_updated_at.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +class SpreadPatientScoresUpdatedAt < ActiveRecord::Migration[6.1] + def up + # Find the clustered timestamp that's causing sync pagination issues + # This happens when bulk inserts share the same updated_at + clustered_timestamps = PatientScore.group(:updated_at) + .having("count(*) > 1000") + .count + .keys + + return if clustered_timestamps.empty? + + clustered_timestamps.each do |clustered_timestamp| + Rails.logger.info "Spreading updated_at for PatientScores with timestamp: #{clustered_timestamp}" + + # First, check if device_updated_at is well-distributed + device_updated_distribution = PatientScore + .where(updated_at: clustered_timestamp) + .group(:device_updated_at) + .count + .sort_by { |_, n| -n } + .first(5) + + max_device_cluster = device_updated_distribution.first&.last || 0 + + if max_device_cluster < 1000 + # device_updated_at is well-distributed, use it + Rails.logger.info "Using device_updated_at (max cluster: #{max_device_cluster})" + PatientScore + .where(updated_at: clustered_timestamp) + .update_all("updated_at = device_updated_at") + else + # device_updated_at is also clustered, spread by id with millisecond offsets + Rails.logger.info "Spreading by id with millisecond offsets (device_updated_at max cluster: #{max_device_cluster})" + ActiveRecord::Base.connection.execute(<<-SQL.squish) + UPDATE patient_scores ps + SET updated_at = '#{clustered_timestamp}'::timestamp + + (sub.row_num * interval '1 millisecond') + FROM ( + SELECT id, row_number() OVER (ORDER BY id) AS row_num + FROM patient_scores + WHERE updated_at = '#{clustered_timestamp}' + ) sub + WHERE ps.id = sub.id + SQL + end + end + end + + def down + # This migration cannot be reversed as we don't track original values + raise ActiveRecord::IrreversibleMigration + end +end From 9d36154daa286b04a51d417b6bdc0b7befcc9a93 Mon Sep 17 00:00:00 2001 From: Sagar Shinde Date: Tue, 14 Apr 2026 15:08:20 +0530 Subject: [PATCH 08/12] Implement keyset pagination for PatientScore sync - Replace timestamp-only pagination with (updated_at, id) keyset pagination - Add process token tracking for last_id to ensure no records are skipped - Prevents duplicate/missed records when multiple scores share same timestamp --- .../api/v4/patient_scores_controller.rb | 84 ++++++++++++++++--- .../api/v4/patient_scores_controller_spec.rb | 25 ++++++ 2 files changed, 98 insertions(+), 11 deletions(-) diff --git a/app/controllers/api/v4/patient_scores_controller.rb b/app/controllers/api/v4/patient_scores_controller.rb index e2c50c9b96..0432423530 100644 --- a/app/controllers/api/v4/patient_scores_controller.rb +++ b/app/controllers/api/v4/patient_scores_controller.rb @@ -5,22 +5,30 @@ def sync_to_user def current_facility_records @current_facility_records ||= - PatientScore - .for_sync - .where(patient: current_facility.prioritized_patients.select(:id)) - .updated_on_server_since(current_facility_processed_since, limit) + keyset_paginate( + PatientScore + .for_sync + .where(patient: current_facility.prioritized_patients.select(:id)), + current_facility_processed_since, + current_facility_last_id, + limit + ) end def other_facility_records other_facilities_limit = limit - current_facility_records.size @other_facility_records ||= - PatientScore - .for_sync - .where(patient_id: current_sync_region - .syncable_patients - .where.not(registration_facility: current_facility) - .select(:id)) - .updated_on_server_since(other_facilities_processed_since, other_facilities_limit) + keyset_paginate( + PatientScore + .for_sync + .where(patient_id: current_sync_region + .syncable_patients + .where.not(registration_facility: current_facility) + .select(:id)), + other_facilities_processed_since, + other_facilities_last_id, + other_facilities_limit + ) end private @@ -28,4 +36,58 @@ def other_facility_records def transform_to_response(patient_score) Api::V4::PatientScoreTransformer.to_response(patient_score) end + + def keyset_paginate(scope, since_time, since_id, batch_limit) + return scope.none if batch_limit <= 0 + + ordered = scope.order(:updated_at, :id).limit(batch_limit) + + if since_id.present? + ordered.where( + "(patient_scores.updated_at, patient_scores.id) > (?, ?)", + since_time, since_id + ) + else + ordered.where("patient_scores.updated_at >= ?", since_time) + end + end + + def processed_until(records) + return nil if records.empty? + last = records.last + { + timestamp: last.updated_at.strftime(APIController::TIME_WITHOUT_TIMEZONE_FORMAT), + id: last.id + } + end + + def response_process_token + current_cursor = processed_until(current_facility_records) + other_cursor = processed_until(other_facility_records) + + { + current_facility_id: current_facility.id, + current_facility_processed_since: + current_cursor&.dig(:timestamp) || process_token[:current_facility_processed_since], + current_facility_last_id: + current_cursor&.dig(:id) || process_token[:current_facility_last_id], + other_facilities_processed_since: + other_cursor&.dig(:timestamp) || process_token[:other_facilities_processed_since], + other_facilities_last_id: + other_cursor&.dig(:id) || process_token[:other_facilities_last_id], + resync_token: resync_token, + sync_region_id: current_sync_region.id + } + end + + def current_facility_last_id + return nil if force_resync? + return nil if process_token[:current_facility_id] != current_facility.id + process_token[:current_facility_last_id] + end + + def other_facilities_last_id + return nil if force_resync? + process_token[:other_facilities_last_id] + end end diff --git a/spec/controllers/api/v4/patient_scores_controller_spec.rb b/spec/controllers/api/v4/patient_scores_controller_spec.rb index c153ec671e..cbceb6b9ba 100644 --- a/spec/controllers/api/v4/patient_scores_controller_spec.rb +++ b/spec/controllers/api/v4/patient_scores_controller_spec.rb @@ -23,5 +23,30 @@ def create_record_list(n, options = {}) describe "GET sync: send data from server to device;" do it_behaves_like "a working V3 sync controller sending records" + + it "advances the process_token when many records share one updated_at" do + set_authentication_headers + shared_ts = 5.minutes.ago + facility = create(:facility, facility_group: request_facility_group) + patient = create(:patient, registration_facility: facility) + records = create_list(:patient_score, 5, patient: patient, updated_at: shared_ts) + expected_ids = records.map(&:id).to_set + + received_ids = Set.new + process_token = nil + + 3.times do + reset_controller + set_authentication_headers + get :sync_to_user, params: {limit: 2, process_token: process_token}.compact + + body = JSON(response.body) + body["patient_scores"].each { |r| received_ids << r["id"] } + process_token = body["process_token"] + break if body["patient_scores"].empty? + end + + expect(received_ids).to eq(expected_ids) + end end end From 7ca61286e8eb8f0a9da51e59775fc03ff473dd3e Mon Sep 17 00:00:00 2001 From: Sagar Shinde Date: Tue, 14 Apr 2026 16:01:28 +0530 Subject: [PATCH 09/12] fix: Eager load PatientScore pagination results to fix cursor tracking --- app/controllers/api/v4/patient_scores_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v4/patient_scores_controller.rb b/app/controllers/api/v4/patient_scores_controller.rb index 0432423530..b3e0933a4f 100644 --- a/app/controllers/api/v4/patient_scores_controller.rb +++ b/app/controllers/api/v4/patient_scores_controller.rb @@ -12,7 +12,7 @@ def current_facility_records current_facility_processed_since, current_facility_last_id, limit - ) + ).to_a end def other_facility_records @@ -28,7 +28,7 @@ def other_facility_records other_facilities_processed_since, other_facilities_last_id, other_facilities_limit - ) + ).to_a end private From 19be76e0e79514ba4a29b9cb8c8a44072a17e2f2 Mon Sep 17 00:00:00 2001 From: Sagar Shinde Date: Tue, 14 Apr 2026 16:22:11 +0530 Subject: [PATCH 10/12] fix: Parse process_token timestamps in UTC for keyset pagination Timestamps stored in process_token are UTC strings (ending with 'Z'),but the inherited to_time method converts them to local timezone.This causes incorrect comparisons when the server timezone differs from UTC, resulting in 0 records returned on subsequent sync requests. Override other_facilities_processed_since and current_facility_processed_since to explicitly parse timestamps in UTC using to_time(:utc). --- .../api/v4/patient_scores_controller.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/app/controllers/api/v4/patient_scores_controller.rb b/app/controllers/api/v4/patient_scores_controller.rb index b3e0933a4f..9c04b73624 100644 --- a/app/controllers/api/v4/patient_scores_controller.rb +++ b/app/controllers/api/v4/patient_scores_controller.rb @@ -90,4 +90,22 @@ def other_facilities_last_id return nil if force_resync? process_token[:other_facilities_last_id] end + + def other_facilities_processed_since + return Time.new(0) if force_resync? + process_token[:other_facilities_processed_since]&.to_time(:utc) || Time.new(0) + end + + def current_facility_processed_since + if force_resync? + Time.new(0) + elsif process_token[:current_facility_processed_since].blank? + other_facilities_processed_since + elsif process_token[:current_facility_id] != current_facility.id + [process_token[:current_facility_processed_since].to_time(:utc), + other_facilities_processed_since].min + else + process_token[:current_facility_processed_since].to_time(:utc) + end + end end From b958b09e4e593f17b5f7cfadbdbad95cc4ac9f19 Mon Sep 17 00:00:00 2001 From: sshinde-rtsl Date: Tue, 14 Apr 2026 17:52:43 +0530 Subject: [PATCH 11/12] refactor: Use next_page token for PatientScore sync, drop other-facility bucket Replace keyset pagination with simple OFFSET-based next_page counter in the process_token. Patient scores now sync only for the current facility's prioritized patients; the other-facilities bucket is dropped. Updates specs to match the new token contract. --- .../api/v4/patient_scores_controller.rb | 96 +++---------------- .../api/v4/patient_scores_controller_spec.rb | 71 +++++++++++--- 2 files changed, 72 insertions(+), 95 deletions(-) diff --git a/app/controllers/api/v4/patient_scores_controller.rb b/app/controllers/api/v4/patient_scores_controller.rb index 9c04b73624..eb812f04d9 100644 --- a/app/controllers/api/v4/patient_scores_controller.rb +++ b/app/controllers/api/v4/patient_scores_controller.rb @@ -5,30 +5,17 @@ def sync_to_user def current_facility_records @current_facility_records ||= - keyset_paginate( - PatientScore - .for_sync - .where(patient: current_facility.prioritized_patients.select(:id)), - current_facility_processed_since, - current_facility_last_id, - limit - ).to_a + PatientScore + .for_sync + .where(patient: current_facility.prioritized_patients.select(:id)) + .order(:updated_at, :id) + .limit(limit) + .offset((current_page - 1) * limit) + .to_a end def other_facility_records - other_facilities_limit = limit - current_facility_records.size - @other_facility_records ||= - keyset_paginate( - PatientScore - .for_sync - .where(patient_id: current_sync_region - .syncable_patients - .where.not(registration_facility: current_facility) - .select(:id)), - other_facilities_processed_since, - other_facilities_last_id, - other_facilities_limit - ).to_a + [] end private @@ -37,75 +24,18 @@ def transform_to_response(patient_score) Api::V4::PatientScoreTransformer.to_response(patient_score) end - def keyset_paginate(scope, since_time, since_id, batch_limit) - return scope.none if batch_limit <= 0 - - ordered = scope.order(:updated_at, :id).limit(batch_limit) - - if since_id.present? - ordered.where( - "(patient_scores.updated_at, patient_scores.id) > (?, ?)", - since_time, since_id - ) - else - ordered.where("patient_scores.updated_at >= ?", since_time) - end - end - - def processed_until(records) - return nil if records.empty? - last = records.last - { - timestamp: last.updated_at.strftime(APIController::TIME_WITHOUT_TIMEZONE_FORMAT), - id: last.id - } + def current_page + page = process_token[:next_page].to_i + page < 1 ? 1 : page end def response_process_token - current_cursor = processed_until(current_facility_records) - other_cursor = processed_until(other_facility_records) - + has_more_pages = current_facility_records.size == limit { current_facility_id: current_facility.id, - current_facility_processed_since: - current_cursor&.dig(:timestamp) || process_token[:current_facility_processed_since], - current_facility_last_id: - current_cursor&.dig(:id) || process_token[:current_facility_last_id], - other_facilities_processed_since: - other_cursor&.dig(:timestamp) || process_token[:other_facilities_processed_since], - other_facilities_last_id: - other_cursor&.dig(:id) || process_token[:other_facilities_last_id], + next_page: has_more_pages ? current_page + 1 : 1, resync_token: resync_token, sync_region_id: current_sync_region.id } end - - def current_facility_last_id - return nil if force_resync? - return nil if process_token[:current_facility_id] != current_facility.id - process_token[:current_facility_last_id] - end - - def other_facilities_last_id - return nil if force_resync? - process_token[:other_facilities_last_id] - end - - def other_facilities_processed_since - return Time.new(0) if force_resync? - process_token[:other_facilities_processed_since]&.to_time(:utc) || Time.new(0) - end - - def current_facility_processed_since - if force_resync? - Time.new(0) - elsif process_token[:current_facility_processed_since].blank? - other_facilities_processed_since - elsif process_token[:current_facility_id] != current_facility.id - [process_token[:current_facility_processed_since].to_time(:utc), - other_facilities_processed_since].min - else - process_token[:current_facility_processed_since].to_time(:utc) - end - end end diff --git a/spec/controllers/api/v4/patient_scores_controller_spec.rb b/spec/controllers/api/v4/patient_scores_controller_spec.rb index cbceb6b9ba..03e4df8662 100644 --- a/spec/controllers/api/v4/patient_scores_controller_spec.rb +++ b/spec/controllers/api/v4/patient_scores_controller_spec.rb @@ -7,14 +7,12 @@ let(:model) { PatientScore } def create_record(options = {}) - facility = create(:facility, facility_group: request_facility_group) - patient = create(:patient, registration_facility: facility) + patient = create(:patient, registration_facility: request_facility) create(:patient_score, options.merge(patient: patient)) end def create_record_list(n, options = {}) - facility = create(:facility, facility_group_id: request_facility_group.id) - patient = create(:patient, registration_facility_id: facility.id) + patient = create(:patient, registration_facility: request_facility) create_list(:patient_score, n, options.merge(patient: patient)) end @@ -22,24 +20,32 @@ def create_record_list(n, options = {}) it_behaves_like "a sync controller that audits the data access: sync_to_user" describe "GET sync: send data from server to device;" do - it_behaves_like "a working V3 sync controller sending records" + before { set_authentication_headers } - it "advances the process_token when many records share one updated_at" do - set_authentication_headers + it "returns only current facility patient scores" do + expected = create_record_list(3) + other_patient = create(:patient, registration_facility: create(:facility, facility_group: request_facility_group)) + create(:patient_score, patient: other_patient) + + get :sync_to_user + + body = JSON(response.body) + expect(body["patient_scores"].map { |r| r["id"] }.to_set) + .to eq(expected.map(&:id).to_set) + end + + it "paginates via next_page token across multiple requests with a shared updated_at" do shared_ts = 5.minutes.ago - facility = create(:facility, facility_group: request_facility_group) - patient = create(:patient, registration_facility: facility) - records = create_list(:patient_score, 5, patient: patient, updated_at: shared_ts) + records = create_record_list(5, updated_at: shared_ts) expected_ids = records.map(&:id).to_set received_ids = Set.new process_token = nil - 3.times do + 4.times do reset_controller set_authentication_headers get :sync_to_user, params: {limit: 2, process_token: process_token}.compact - body = JSON(response.body) body["patient_scores"].each { |r| received_ids << r["id"] } process_token = body["process_token"] @@ -48,5 +54,46 @@ def create_record_list(n, options = {}) expect(received_ids).to eq(expected_ids) end + + it "advances next_page while full pages are returned and resets to 1 on the final page" do + create_record_list(5, updated_at: 5.minutes.ago) + + get :sync_to_user, params: {limit: 2} + token1 = parse_process_token(JSON(response.body)) + expect(token1[:next_page]).to eq(2) + + reset_controller + set_authentication_headers + get :sync_to_user, params: {limit: 2, process_token: JSON(response.body)["process_token"]} + token2 = parse_process_token(JSON(response.body)) + expect(token2[:next_page]).to eq(3) + + reset_controller + set_authentication_headers + get :sync_to_user, params: {limit: 2, process_token: JSON(response.body)["process_token"]} + body3 = JSON(response.body) + token3 = parse_process_token(body3) + expect(body3["patient_scores"].size).to eq(1) + expect(token3[:next_page]).to eq(1) + end + + it "returns an empty list and next_page=1 when there is nothing to sync" do + get :sync_to_user + + body = JSON(response.body) + token = parse_process_token(body) + expect(body["patient_scores"]).to eq([]) + expect(token[:next_page]).to eq(1) + end + + it "returns discarded records" do + records = create_record_list(3, updated_at: 5.minutes.ago) + records.first.patient.discard_data(reason: nil) + + get :sync_to_user + + body = JSON(response.body) + expect(body["patient_scores"].size).to eq(3) + end end end From b364838dc8cdb4f8c3a1736910b8ac170bc67257 Mon Sep 17 00:00:00 2001 From: sshinde-rtsl Date: Tue, 14 Apr 2026 19:09:20 +0530 Subject: [PATCH 12/12] fix: Advance next_page on every non-empty page, reset only on empty Previously the token reset next_page to 1 as soon as a partial page was returned, so calling the endpoint again with the returned token re-fetched the same records instead of returning empty. Now next_page advances on every non-empty response (full or partial) and resets to 1 only when the query returns zero records, giving clients a clear 'sync complete' signal. --- .../api/v4/patient_scores_controller.rb | 3 +-- .../api/v4/patient_scores_controller_spec.rb | 26 ++++++++++++------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/app/controllers/api/v4/patient_scores_controller.rb b/app/controllers/api/v4/patient_scores_controller.rb index eb812f04d9..13e6432e0b 100644 --- a/app/controllers/api/v4/patient_scores_controller.rb +++ b/app/controllers/api/v4/patient_scores_controller.rb @@ -30,10 +30,9 @@ def current_page end def response_process_token - has_more_pages = current_facility_records.size == limit { current_facility_id: current_facility.id, - next_page: has_more_pages ? current_page + 1 : 1, + next_page: current_facility_records.empty? ? 1 : current_page + 1, resync_token: resync_token, sync_region_id: current_sync_region.id } diff --git a/spec/controllers/api/v4/patient_scores_controller_spec.rb b/spec/controllers/api/v4/patient_scores_controller_spec.rb index 03e4df8662..ee3e216fb5 100644 --- a/spec/controllers/api/v4/patient_scores_controller_spec.rb +++ b/spec/controllers/api/v4/patient_scores_controller_spec.rb @@ -55,26 +55,34 @@ def create_record_list(n, options = {}) expect(received_ids).to eq(expected_ids) end - it "advances next_page while full pages are returned and resets to 1 on the final page" do + it "advances next_page on every non-empty page and resets to 1 only on an empty page" do create_record_list(5, updated_at: 5.minutes.ago) get :sync_to_user, params: {limit: 2} - token1 = parse_process_token(JSON(response.body)) - expect(token1[:next_page]).to eq(2) + body1 = JSON(response.body) + expect(body1["patient_scores"].size).to eq(2) + expect(parse_process_token(body1)[:next_page]).to eq(2) reset_controller set_authentication_headers - get :sync_to_user, params: {limit: 2, process_token: JSON(response.body)["process_token"]} - token2 = parse_process_token(JSON(response.body)) - expect(token2[:next_page]).to eq(3) + get :sync_to_user, params: {limit: 2, process_token: body1["process_token"]} + body2 = JSON(response.body) + expect(body2["patient_scores"].size).to eq(2) + expect(parse_process_token(body2)[:next_page]).to eq(3) reset_controller set_authentication_headers - get :sync_to_user, params: {limit: 2, process_token: JSON(response.body)["process_token"]} + get :sync_to_user, params: {limit: 2, process_token: body2["process_token"]} body3 = JSON(response.body) - token3 = parse_process_token(body3) expect(body3["patient_scores"].size).to eq(1) - expect(token3[:next_page]).to eq(1) + expect(parse_process_token(body3)[:next_page]).to eq(4) + + reset_controller + set_authentication_headers + get :sync_to_user, params: {limit: 2, process_token: body3["process_token"]} + body4 = JSON(response.body) + expect(body4["patient_scores"]).to eq([]) + expect(parse_process_token(body4)[:next_page]).to eq(1) end it "returns an empty list and next_page=1 when there is nothing to sync" do