diff --git a/Gemfile b/Gemfile index 19a74ba86a..58846a8516 100644 --- a/Gemfile +++ b/Gemfile @@ -48,13 +48,14 @@ gem "jwt" gem "mechanize" gem "notifications-ruby-client" gem "okcomputer" -gem "omniauth_openid_connect" gem "omniauth-rails_csrf_protection" +gem "omniauth_openid_connect" gem "pagy" gem "phonelib" gem "pundit" gem "rails_semantic_logger" gem "rainbow" +gem "redis" gem "ruby-progressbar" gem "rubyzip" gem "sentry-rails" diff --git a/Gemfile.lock b/Gemfile.lock index d6cdaee172..c4d40315db 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -519,6 +519,8 @@ GEM erb psych (>= 4.0.0) redcarpet (3.6.1) + redis (5.4.1) + redis-client (>= 0.22.0) redis-client (0.25.2) connection_pool redis-prescription (2.6.0) @@ -819,6 +821,7 @@ DEPENDENCIES rails (~> 8.0.2) rails_semantic_logger rainbow + redis rladr rspec rspec-html-matchers diff --git a/app/components/app_imports_navigation_component.rb b/app/components/app_imports_navigation_component.rb index 2c8315d325..e6907c1316 100644 --- a/app/components/app_imports_navigation_component.rb +++ b/app/components/app_imports_navigation_component.rb @@ -1,8 +1,9 @@ # frozen_string_literal: true class AppImportsNavigationComponent < ViewComponent::Base - def initialize(active:) + def initialize(active:, team:) @active = active + @team = team end def call @@ -31,18 +32,22 @@ def call private - attr_reader :active + attr_reader :active, :team - delegate :import_issues_count, :policy, :policy_scope, to: :helpers + delegate :policy, :policy_scope, to: :helpers def issues_text - safe_join( - ["Import issues", " ", render(AppCountComponent.new(import_issues_count))] - ) + count = TeamCachedCounts.new(team).import_issues + text_with_count("Import issues", count) end def notices_text count = ImportantNotices.call(patient_scope: policy_scope(Patient)).length - safe_join(["Important notices", " ", render(AppCountComponent.new(count))]) + + text_with_count("Important notices", count) + end + + def text_with_count(text, count) + safe_join([text, " ", render(AppCountComponent.new(count))]) end end diff --git a/app/controllers/parent_interface/consent_forms_controller.rb b/app/controllers/parent_interface/consent_forms_controller.rb index e38620acbb..202d50ad57 100644 --- a/app/controllers/parent_interface/consent_forms_controller.rb +++ b/app/controllers/parent_interface/consent_forms_controller.rb @@ -55,6 +55,8 @@ def record session.delete(:consent_form_id) + TeamCachedCounts.new(@team).reset_unmatched_consent_responses! + send_consent_form_confirmation(@consent_form) ConsentFormMatchingJob.perform_later(@consent_form) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index cb49d39aa3..1e295a00d0 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -57,4 +57,6 @@ def manifest_link_tag(name, **options) def opengraph_image_tag(service_url, name) tag.meta(property: "og:image", content: "#{service_url}#{asset_path(name)}") end + + def cached_counts = TeamCachedCounts.new(current_team) end diff --git a/app/helpers/imports_helper.rb b/app/helpers/imports_helper.rb index a4a9b5f305..8d14eb39e2 100644 --- a/app/helpers/imports_helper.rb +++ b/app/helpers/imports_helper.rb @@ -12,15 +12,6 @@ module ImportsHelper %w[registration] => :registration }.freeze - def import_issues_count - vaccination_records_with_issues = - policy_scope(VaccinationRecord).with_pending_changes.pluck(:patient_id) - - patients_with_issues = policy_scope(Patient).with_pending_changes.pluck(:id) - - (vaccination_records_with_issues + patients_with_issues).uniq.length - end - def issue_categories_for(pending_changes) FIELD_GROUPS.filter_map do |(keys, group)| group.to_s.humanize if (pending_changes & keys).any? diff --git a/app/jobs/commit_patient_changesets_job.rb b/app/jobs/commit_patient_changesets_job.rb index c0c661e6a1..b25e5b3e34 100644 --- a/app/jobs/commit_patient_changesets_job.rb +++ b/app/jobs/commit_patient_changesets_job.rb @@ -29,6 +29,8 @@ def perform(import) import.postprocess_rows! + reset_counts(import) + import.update_columns( processed_at: Time.zone.now, status: :processed, @@ -165,4 +167,10 @@ def has_auto_confirmable_school_move?(school_move, import) academic_year: import.academic_year ) || school_move.patient.archived?(team: import.team) end + + def reset_counts(import) + cached_counts = TeamCachedCounts.new(import.team) + cached_counts.reset_import_issues! + cached_counts.reset_school_moves! + end end diff --git a/app/jobs/consent_form_matching_job.rb b/app/jobs/consent_form_matching_job.rb index 2398552a6e..050aa0fd07 100644 --- a/app/jobs/consent_form_matching_job.rb +++ b/app/jobs/consent_form_matching_job.rb @@ -59,6 +59,8 @@ def match_with_exact_nhs_number patient.update_from_pds!(pds_patient) send_parental_contact_warning_if_needed(patient, @consent_form) @consent_form.match_with_patient!(patient, current_user: nil) + reset_counts + true end def session_patients @@ -95,5 +97,10 @@ def match_patient(patient) send_parental_contact_warning_if_needed(patient, @consent_form) @consent_form.match_with_patient!(patient, current_user: nil) + reset_counts + end + + def reset_counts + TeamCachedCounts.new(@consent_form.team).reset_unmatched_consent_responses! end end diff --git a/app/lib/team_cached_counts.rb b/app/lib/team_cached_counts.rb new file mode 100644 index 0000000000..d441a0061d --- /dev/null +++ b/app/lib/team_cached_counts.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +class TeamCachedCounts + def initialize(team) + @team = team + end + + def import_issues + return nil if current_user.nil? + + Rails + .cache + .fetch(import_issues_key) do + vaccination_records_with_issues = + VaccinationRecordPolicy::Scope + .new(current_user, VaccinationRecord) + .resolve + .with_pending_changes + .pluck(:patient_id) + + patients_with_issues = + PatientPolicy::Scope + .new(current_user, Patient) + .resolve + .with_pending_changes + .pluck(:id) + + (vaccination_records_with_issues + patients_with_issues).uniq.length + end + end + + def reset_import_issues! + Rails.cache.delete(import_issues_key) + end + + def school_moves + return nil if current_user.nil? + + Rails + .cache + .fetch(school_moves_key) do + SchoolMovePolicy::Scope.new(current_user, SchoolMove).resolve.count + end + end + + def reset_school_moves! + Rails.cache.delete(school_moves_key) + end + + def unmatched_consent_responses + return nil if current_user.nil? + + Rails + .cache + .fetch(unmatched_consent_responses_key) do + ConsentFormPolicy::Scope + .new(current_user, ConsentForm) + .resolve + .unmatched + .recorded + .not_archived + .count + end + end + + def reset_unmatched_consent_responses! + Rails.cache.delete(unmatched_consent_responses_key) + end + + private + + attr_reader :team + + def import_issues_key = cache_key("import-issues") + + def school_moves_key = cache_key("school-moves") + + def unmatched_consent_responses_key = cache_key("unmatched-consent-responses") + + def current_user + # We can't use the policy_scope helper here as we're not in a controller. + # Instead, we can mock what a `User` looks like from the perspective of a + # controller to satisfy the policy scopes. + @current_user ||= + if team && (organisation = team.organisation) + OpenStruct.new(selected_team: team, selected_organisation: organisation) + end + end + + def cache_key(type) = "cached-counts/#{type}/#{team.id}" +end diff --git a/app/views/imports/index.html.erb b/app/views/imports/index.html.erb index af332cccfd..17c724785c 100644 --- a/app/views/imports/index.html.erb +++ b/app/views/imports/index.html.erb @@ -2,6 +2,6 @@ <%= govuk_button_to "Import records", imports_path, secondary: true, class: "nhsuk-u-margin-bottom-4" %> -<%= render AppImportsNavigationComponent.new(active: :index) %> +<%= render AppImportsNavigationComponent.new(active: :index, team: current_team) %> <%= render AppImportsTableComponent.new(team: current_team) %> diff --git a/app/views/imports/issues/index.html.erb b/app/views/imports/issues/index.html.erb index 4f11f96731..682175689f 100644 --- a/app/views/imports/issues/index.html.erb +++ b/app/views/imports/issues/index.html.erb @@ -2,7 +2,7 @@ <%= govuk_button_to "Import records", imports_path, secondary: true, class: "nhsuk-u-margin-bottom-4" %> -<%= render AppImportsNavigationComponent.new(active: :issues) %> +<%= render AppImportsNavigationComponent.new(active: :issues, team: current_team) %> <% if @import_issues.any? %>
diff --git a/app/views/imports/notices/index.html.erb b/app/views/imports/notices/index.html.erb index 3b4e2c4b0b..d604fb9b9c 100644 --- a/app/views/imports/notices/index.html.erb +++ b/app/views/imports/notices/index.html.erb @@ -2,7 +2,7 @@ <%= govuk_button_to "Import records", imports_path, secondary: true, class: "nhsuk-u-margin-bottom-4" %> -<%= render AppImportsNavigationComponent.new(active: :notices) %> +<%= render AppImportsNavigationComponent.new(active: :notices, team: current_team) %> <% if @notices.present? %> <%= render AppNoticesTableComponent.new(@notices) %> diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb index 49b60430c9..eb5334c574 100644 --- a/app/views/layouts/_header.html.erb +++ b/app/views/layouts/_header.html.erb @@ -46,14 +46,14 @@ t("consent_forms.index.title_short"), consent_forms_path, request_path: request.path, - count: policy_scope(ConsentForm).unmatched.recorded.not_archived.count + count: cached_counts.unmatched_consent_responses ) %> <%= render AppHeaderNavigationItemComponent.new( t("school_moves.index.title"), school_moves_path, request_path: request.path, - count: policy_scope(SchoolMove).count + count: cached_counts.school_moves ) %> <%= render AppHeaderNavigationItemComponent.new(t("vaccines.index.title"), vaccines_path, request_path: request.path) %> @@ -62,7 +62,7 @@ t("imports.index.title_short"), imports_path, request_path: request.path, - count: import_issues_count + count: cached_counts.import_issues ) %> <%= render AppHeaderNavigationItemComponent.new(t("teams.show.title"), team_path, request_path: request.path) %> diff --git a/config/environments/production.rb b/config/environments/production.rb index 7cdd797223..a822c3dddb 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -72,7 +72,21 @@ ) # Replace the default in-process memory cache store with a durable alternative. - # config.cache_store = :mem_cache_store + config.cache_store = + :redis_cache_store, + { + url: ENV["REDIS_CACHE_URL"], + error_handler: ->(method:, returning:, exception:) do + Sentry.capture_exception( + exception, + level: "warning", + tags: { + method:, + returning: + } + ) + end + } # Replace the default in-process and non-durable queuing backend for Active Job. # config.active_job.queue_adapter = :resque