diff --git a/app/services/permitted_form_fields_service.rb b/app/services/permitted_form_fields_service.rb index 9b81e224d4..d5afa09153 100644 --- a/app/services/permitted_form_fields_service.rb +++ b/app/services/permitted_form_fields_service.rb @@ -4,9 +4,9 @@ # This service handles computing the permitted fields for a given role, # based on the forms associated with that role. The query is optionally cached. -# The similarly named PermittedFieldService uses this service to compuyte the full list of permitted fields +# The similarly named PermittedFieldService uses this service to compute the full list of permitted fields class PermittedFormFieldsService - attr_accessor :fields, :field_names, :with_cache + attr_accessor :with_cache PERMITTED_WRITEABLE_FIELD_TYPES = [ Field::TEXT_FIELD, Field::TEXT_AREA, Field::RADIO_BUTTON, Field::TICK_BOX, @@ -15,7 +15,7 @@ class PermittedFormFieldsService Field::SUBFORM, Field::TALLY_FIELD, Field::CALCULATED ].freeze - # TODO: Primero is assuming that these forms exist in the configuration. If they + # TODO: Primero is assuming that these forms exist in the configuration. PERMITTED_SUBFORMS_FOR_ACTION = { Permission::ADD_NOTE => 'notes_section', Permission::INCIDENT_DETAILS_FROM_CASE => 'incident_details', @@ -30,17 +30,27 @@ def initialize(with_cache = false) self.with_cache = with_cache end - def rebuild_cache(roles, record_type, module_unique_id, writeable, force = false) - return unless force || fields.nil? + def generate_cache_key(roles, record_type, module_unique_id, writeable) + role_keys = roles.map(&:cache_key_with_version) # The assumption here is that the cache will be updated if any changes took place to Forms, or Roles - role_keys = roles.map(&:cache_key_with_version) - cache_key = "permitted_form_fields_service/#{role_keys.join('/')}/#{module_unique_id}/#{record_type}/#{writeable}" - self.fields = Rails.cache.fetch(cache_key, expires_in: 48.hours) do + "permitted_form_fields_service/#{role_keys.join('/')}/#{module_unique_id}/#{record_type}/#{writeable}" + end + + def permitted_fields_from_cache(roles, record_type, module_unique_id, writeable, force = false) + cache_key = generate_cache_key(roles, record_type, module_unique_id, writeable) + + Rails.cache.fetch(cache_key, expires_in: 48.hours, force:) do permitted_fields_from_forms(roles, record_type, module_unique_id, writeable).to_a end - # TODO: This can be cached too - self.field_names = fields.map(&:name).uniq + end + + def permitted_field_names_from_cache(roles, record_type, module_unique_id, writeable, force = false) + cache_key = generate_cache_key(roles, record_type, module_unique_id, writeable) + + Rails.cache.fetch("#{cache_key}/names", expires_in: 48.hours, force:) do + permitted_fields_from_cache(roles, record_type, module_unique_id, writeable, force).map(&:name).uniq + end end def permitted_fields_from_forms(roles, record_type, module_unique_id, writeable, visible_only = false) @@ -56,8 +66,7 @@ def permitted_fields_from_forms(roles, record_type, module_unique_id, writeable, def permitted_fields(roles, record_type, module_unique_id, writeable) if with_cache? - rebuild_cache(roles, record_type, module_unique_id, writeable) - fields + permitted_fields_from_cache(roles, record_type, module_unique_id, writeable) else permitted_fields_from_forms(roles, record_type, module_unique_id, writeable).to_a end @@ -65,8 +74,7 @@ def permitted_fields(roles, record_type, module_unique_id, writeable) def permitted_field_names(roles, record_type, module_unique_id, writeable) if with_cache? - rebuild_cache(roles, record_type, module_unique_id, writeable) - field_names + permitted_field_names_from_cache(roles, record_type, module_unique_id, writeable) else permitted_fields_from_forms(roles, record_type, module_unique_id, writeable).map(&:name).uniq end diff --git a/app/services/subform_summary_fields_service.rb b/app/services/subform_summary_fields_service.rb index 1a4a4167dd..e943a28c27 100644 --- a/app/services/subform_summary_fields_service.rb +++ b/app/services/subform_summary_fields_service.rb @@ -1,11 +1,13 @@ # frozen_string_literal: true # Copyright (c) 2014 - 2023 UNICEF. All rights reserved. + +# This service retrieves the summary fields. The query is optionally cached. class SubformSummaryFieldsService - attr_accessor :fields, :with_cache + attr_accessor :with_cache def self.instance - @instance ||= new(Rails.configuration.use_app_cache) + new(Rails.configuration.use_app_cache) end alias with_cache? with_cache @@ -14,27 +16,24 @@ def initialize(with_cache = false) self.with_cache = with_cache end - def rebuild_cache(record_type, force = false) - return unless force || fields.nil? - + def subform_summary_fields_from_cache(record_type, force = false) # The assumption here is that the cache will be updated if any changes took place to Fields max_updated_at = Field.maximum(:updated_at).as_json cache_key = "subform_summary_fields_service/fields/#{max_updated_at}/#{record_type}" - self.fields = Rails.cache.fetch(cache_key, expires_in: 48.hours) do - loaded_fields(record_type).to_a + Rails.cache.fetch(cache_key, expires_in: 48.hours, force:) do + subform_summary_fields_from_forms(record_type).to_a end end def subform_summary_fields(record_type) if with_cache? - rebuild_cache(record_type) - fields + subform_summary_fields_from_cache(record_type) else - loaded_fields(record_type) + subform_summary_fields_from_forms(record_type) end end - def loaded_fields(record_type) + def subform_summary_fields_from_forms(record_type) Field.joins(:form_section) .where(form_section: { parent_form: record_type }, visible: true) .where.not(subform_summary: nil) diff --git a/spec/services/permitted_form_fields_service_spec.rb b/spec/services/permitted_form_fields_service_spec.rb index 2679fa92d3..006acf6ce7 100644 --- a/spec/services/permitted_form_fields_service_spec.rb +++ b/spec/services/permitted_form_fields_service_spec.rb @@ -355,7 +355,7 @@ describe '#permitted_fields' do it 'lists all writeable fields' do - permitted_fields = service.permitted_fields(role, 'case', 'primeromodule-cpa', true) + permitted_fields = service.permitted_fields([role], 'case', 'primeromodule-cpa', true) expect(permitted_fields.size).to eq(11) expect(permitted_fields.map(&:name)).to match_array( %w[name age sex national_id_no consent_for_services current_address protection_concerns @@ -364,7 +364,7 @@ end it 'lists all readable fields' do - permitted_fields = service.permitted_fields(role, 'case', 'primeromodule-cpa', false) + permitted_fields = service.permitted_fields([role], 'case', 'primeromodule-cpa', false) expect(permitted_fields.size).to eq(14) expect(permitted_fields.map(&:name)).to match_array( %w[name age sex national_id_no consent_for_services current_address protection_concerns @@ -373,7 +373,7 @@ end it 'includes action subforms when writeable for the module' do - permitted_fields = service.permitted_fields(role_with_actions, 'case', 'primeromodule-cpa', true) + permitted_fields = service.permitted_fields([role_with_actions], 'case', 'primeromodule-cpa', true) expect(permitted_fields.size).to eq(13) expect(permitted_fields.map(&:name)).to match_array( %w[name age sex national_id_no consent_for_services current_address protection_concerns other_documents @@ -382,37 +382,57 @@ end it 'excludes action subforms when readable' do - permitted_fields = service.permitted_fields(role_with_actions, 'case', 'primeromodule-cpa', false) + permitted_fields = service.permitted_fields([role_with_actions], 'case', 'primeromodule-cpa', false) expect(permitted_fields.size).to eq(14) expect(permitted_fields.map(&:name)).to_not include(:notes_section, :services_section) end it 'includes mrm subforms when writeable' do - permitted_fields = service.permitted_fields(role_mrm, 'incident', PrimeroModule::MRM, true) + permitted_fields = service.permitted_fields([role_mrm], 'incident', PrimeroModule::MRM, true) expect(permitted_fields.size).to eq(2) expect(permitted_fields.map(&:name)).to match_array(%w[another_field killing]) end context 'when a user has access to the notes_section field and can add notes' do it 'returns fields for the specified parent_form and module' do - permitted_fields = service.permitted_fields(role_with_notes, 'case', 'primeromodule-cpa', true) + permitted_fields = service.permitted_fields([role_with_notes], 'case', 'primeromodule-cpa', true) expect(permitted_fields.size).to eq(1) expect(permitted_fields.map { |field| field.form_section.parent_form }).to match_array(%w[case]) expect(permitted_fields.map(&:name)).to match_array(%w[notes_section]) end it 'returns the notes_section fields for the cp module' do - permitted_fields = service.permitted_fields(role_cp, 'case', PrimeroModule::CP, true) + permitted_fields = service.permitted_fields([role_cp], 'case', PrimeroModule::CP, true) expect(permitted_fields.size).to eq(1) expect(permitted_fields.first.subform.fields.map(&:name)).to match_array(%w[name comment]) end it 'returns the notes_section fields for the gbv module' do - permitted_fields = service.permitted_fields(role_gbv, 'case', PrimeroModule::GBV, true) + permitted_fields = service.permitted_fields([role_gbv], 'case', PrimeroModule::GBV, true) expect(permitted_fields.size).to eq(1) expect(permitted_fields.first.subform.fields.map(&:name)).to match_array(%w[name age]) end end + + describe 'when with_cache?' do + let(:service_with_cache) { PermittedFormFieldsService.new(true) } + + it 'lists all writeable and readable fields' do + permitted_writeable_fields = service_with_cache.permitted_fields([role], 'case', 'primeromodule-cpa', true) + permitted_readable_fields = service_with_cache.permitted_fields([role], 'case', 'primeromodule-cpa', false) + + expect(permitted_writeable_fields.size).to eq(11) + expect(permitted_writeable_fields.map(&:name)).to match_array( + %w[name age sex national_id_no consent_for_services current_address protection_concerns + registration_date created_on family_details other_documents] + ) + expect(permitted_readable_fields.size).to eq(14) + expect(permitted_readable_fields.map(&:name)).to match_array( + %w[name age sex national_id_no consent_for_services current_address protection_concerns + registration_date created_on separator1 other_documents family_details interview_date consent_for_referral] + ) + end + end end describe '#permitted_field_names' do diff --git a/spec/services/subform_summary_fields_service_spec.rb b/spec/services/subform_summary_fields_service_spec.rb new file mode 100644 index 0000000000..3a37eec8d7 --- /dev/null +++ b/spec/services/subform_summary_fields_service_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +# Copyright (c) 2014 - 2023 UNICEF. All rights reserved. + +require 'rails_helper' + +describe SubformSummaryFieldsService do + before :each do + clean_data(Field, FormSection) + end + + let(:service) { SubformSummaryFieldsService.new } + + let!(:form_section1) do + score_section = FormSection.create!( + unique_id: 'scores_section', parent_form: 'case', name_en: 'Scores', is_nested: true, + fields: [ + Field.new(name: 'date', type: Field::DATE_FIELD, display_name_en: 'Date'), + Field.new(name: 'score', type: Field::NUMERIC_FIELD, display_name_en: 'Score') + ] + ) + + FormSection.create!( + unique_id: 'form_section1', parent_form: 'case', name_en: 'form_section1', + fields: [ + Field.new(name: 'name', type: Field::TEXT_FIELD, display_name_en: 'Name'), + Field.new(name: 'age', type: Field::NUMERIC_FIELD, display_name_en: 'Age'), + Field.new(name: 'sex', type: Field::SELECT_BOX, display_name_en: 'Sex'), + Field.new( + name: 'most_recent_score', + type: Field::NUMERIC_FIELD, + display_name_en: 'Most Recent Score', + subform_summary: { + subform_field_name: 'scores_section', + first: { + field_name: 'score', + order_by: 'date', + order: 'asc' + } + } + ), + Field.new( + name: 'last_score', + type: Field::NUMERIC_FIELD, + display_name_en: 'Last Score', + visible: false, + subform_summary: { + subform_field_name: 'scores_section', + first: { + field_name: 'score', + order_by: 'date', + order: 'asc' + } + } + ), + Field.new(name: 'scores_section', display_name_en: 'Scores', type: Field::SUBFORM, subform: score_section) + ] + ) + end + + it 'returns all the subform summary fields' do + summary_fields = SubformSummaryFieldsService.instance.subform_summary_fields('case') + + expect(summary_fields.size).to eq(1) + expect(summary_fields.map(&:name)).to match_array(%w[most_recent_score]) + end + + describe 'when with_cache?' do + let(:service) { SubformSummaryFieldsService.new(true) } + + it 'returns all the subform summary fields' do + summary_fields = SubformSummaryFieldsService.instance.subform_summary_fields('case') + + expect(summary_fields.size).to eq(1) + expect(summary_fields.map(&:name)).to match_array(%w[most_recent_score]) + end + end +end