From 047e05c047a81422fa749aff870f78163bdf04a7 Mon Sep 17 00:00:00 2001 From: Daniel Dye Date: Fri, 5 Apr 2024 16:12:40 +0100 Subject: [PATCH 1/4] Update the search_form partial to be more generic Use `name` for the search field name, as standard form field helpers use. Use `value` for the search field input value, as standard form field helpers use. Also default to `params[name]`. The `label` argument now accepts a hash with `text` and `size`. This is to allow the customisation of the label font size. --- app/controllers/claims/support/schools_controller.rb | 1 - .../placements/support/organisations_controller.rb | 1 - app/views/claims/support/schools/index.html.erb | 5 +++-- .../placements/support/organisations/_filter.html.erb | 2 +- .../placements/support/organisations/index.html.erb | 7 ++++--- app/views/shared/_search_form.html.erb | 10 +++++----- config/locales/en/claims/support/claims.yml | 1 + config/locales/en/claims/support/schools.yml | 1 + config/locales/en/placements/support/organisations.yml | 1 + config/locales/en/shared/search_form.yml | 5 ++--- 10 files changed, 18 insertions(+), 16 deletions(-) diff --git a/app/controllers/claims/support/schools_controller.rb b/app/controllers/claims/support/schools_controller.rb index a058b2ed6..2b2c4bc84 100644 --- a/app/controllers/claims/support/schools_controller.rb +++ b/app/controllers/claims/support/schools_controller.rb @@ -5,7 +5,6 @@ class Claims::Support::SchoolsController < Claims::Support::ApplicationControlle def index @pagy, @schools = pagy(schools) - @search_param = params[:name_or_postcode] end def show; end diff --git a/app/controllers/placements/support/organisations_controller.rb b/app/controllers/placements/support/organisations_controller.rb index 2eb7bf984..c087e7392 100644 --- a/app/controllers/placements/support/organisations_controller.rb +++ b/app/controllers/placements/support/organisations_controller.rb @@ -2,7 +2,6 @@ class Placements::Support::OrganisationsController < Placements::Support::Applic def index @pagy, @organisations = pagy(organisations) @filters = filters_param - @search_param = search_param end def new diff --git a/app/views/claims/support/schools/index.html.erb b/app/views/claims/support/schools/index.html.erb index 617656408..3aeeb3b81 100644 --- a/app/views/claims/support/schools/index.html.erb +++ b/app/views/claims/support/schools/index.html.erb @@ -10,7 +10,8 @@ <%= render partial: "shared/search_form", locals: { url: claims_support_schools_path, - search_param: @search_param, + label: { text: t(".search_label"), size: "m" }, + name: :name_or_postcode, clear_search_url: claims_support_schools_path, } %> @@ -30,7 +31,7 @@ <% else %>

- <%= t("no_results", for: @search_param) %> + <%= t("no_results", for: params[:name_or_postcode]) %>

<% end %> diff --git a/app/views/placements/support/organisations/_filter.html.erb b/app/views/placements/support/organisations/_filter.html.erb index a7bc7b6b6..cbd9a5aae 100644 --- a/app/views/placements/support/organisations/_filter.html.erb +++ b/app/views/placements/support/organisations/_filter.html.erb @@ -48,7 +48,7 @@ checked: filters.include?(org_type)) %> <% end %> <% end %> - <%= form.hidden_field :name_or_postcode, value: @search_param %> + <%= form.hidden_field :name_or_postcode, value: search_param %> <% end %> diff --git a/app/views/placements/support/organisations/index.html.erb b/app/views/placements/support/organisations/index.html.erb index 8d5f7c93c..5681a9212 100644 --- a/app/views/placements/support/organisations/index.html.erb +++ b/app/views/placements/support/organisations/index.html.erb @@ -9,12 +9,13 @@
- <%= render partial: "filter", locals: { filters: @filters, search_param: @search_param } %> + <%= render partial: "filter", locals: { filters: @filters, search_param: params[:name_or_postcode] } %>
<%= render partial: "shared/search_form", locals: { url: placements_support_organisations_path, - search_param: @search_param, + label: { text: t(".search_label"), size: "m" }, + name: :name_or_postcode, clear_search_url: placements_support_organisations_path(filters: @filters), filters: @filters, } %> @@ -27,7 +28,7 @@ <%= govuk_pagination(pagy: @pagy) %>

<%= t("pagination_info", from: @pagy.from, to: @pagy.to, count: @pagy.count) %>

<% else %> -

<%= no_results(@search_param, @filters) %>

+

<%= no_results(params[:name_or_postcode], @filters) %>

<% end %>
diff --git a/app/views/shared/_search_form.html.erb b/app/views/shared/_search_form.html.erb index e77b3698d..e9ce6adaa 100644 --- a/app/views/shared/_search_form.html.erb +++ b/app/views/shared/_search_form.html.erb @@ -1,4 +1,4 @@ -<%# locals: (url:, search_param:, clear_search_url: "", filters: []) -%> +<%# locals: (url:, name:, value: params[name], clear_search_url: "", label:, filters: []) -%>
<%= form_with( @@ -7,8 +7,8 @@ class: "search-form", data: { turbo: false }, ) do |f| %> - <%= f.govuk_text_field :name_or_postcode, type: :search, value: search_param, label: { text: t(".search-header"), size: "m" } %> - <%= f.govuk_submit t(".search") %> + <%= f.govuk_text_field name, type: :search, value:, label: %> + <%= f.govuk_submit t(".submit") %> <% if filters.present? %> <% filters.each do |filter| %> <%= f.hidden_field :filters, multiple: true, value: filter %> @@ -17,8 +17,8 @@ <% end %>
-<% unless search_param.blank? %> +<% unless value.blank? %>

- <%= govuk_link_to t(".clear_search"), clear_search_url, no_visited_state: true %> + <%= govuk_link_to t(".clear"), clear_search_url, no_visited_state: true %>

<% end %> diff --git a/config/locales/en/claims/support/claims.yml b/config/locales/en/claims/support/claims.yml index 9c06db4af..1159153b4 100644 --- a/config/locales/en/claims/support/claims.yml +++ b/config/locales/en/claims/support/claims.yml @@ -5,6 +5,7 @@ en: index: heading: Claims download_csv: Download claims + search_label: Search by claim reference show: heading: Claim status: Status diff --git a/config/locales/en/claims/support/schools.yml b/config/locales/en/claims/support/schools.yml index 6ba788c5d..ada21bbc3 100644 --- a/config/locales/en/claims/support/schools.yml +++ b/config/locales/en/claims/support/schools.yml @@ -18,6 +18,7 @@ en: index: heading: Organisations (%{records}) add_organisation: Add organisation + search_label: Search by organisation name or postcode new: caption: Add organisation cancel: Cancel diff --git a/config/locales/en/placements/support/organisations.yml b/config/locales/en/placements/support/organisations.yml index 9ece89ad3..6578cceea 100644 --- a/config/locales/en/placements/support/organisations.yml +++ b/config/locales/en/placements/support/organisations.yml @@ -24,3 +24,4 @@ en: lead_school: Lead partner no_organisations: There are no onboarded organisations organisation_type: Organisation type + search_label: Search by organisation name or postcode diff --git a/config/locales/en/shared/search_form.yml b/config/locales/en/shared/search_form.yml index 536f314a1..83c6aee04 100644 --- a/config/locales/en/shared/search_form.yml +++ b/config/locales/en/shared/search_form.yml @@ -1,6 +1,5 @@ en: shared: search_form: - search-header: Search by organisation name or postcode - search: Search - clear_search: Clear search + submit: Search + clear: Clear search From 1072a0a303c5687622f1ae6721218b351269e21b Mon Sep 17 00:00:00 2001 From: Daniel Dye Date: Fri, 5 Apr 2024 16:41:47 +0100 Subject: [PATCH 2/4] Default money localisation to current locale This defaults money formatting to use commas for thousands separators with the default `en` locale. --- config/initializers/money.rb | 2 +- spec/system/claims/schools/claims/create_claim_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/initializers/money.rb b/config/initializers/money.rb index 9b01ec9d8..d7328eb57 100644 --- a/config/initializers/money.rb +++ b/config/initializers/money.rb @@ -22,5 +22,5 @@ } end -Money.locale_backend = nil +Money.locale_backend = :i18n Money.rounding_mode = BigDecimal::ROUND_HALF_UP diff --git a/spec/system/claims/schools/claims/create_claim_spec.rb b/spec/system/claims/schools/claims/create_claim_spec.rb index 2bcdf593a..e5bcfe267 100644 --- a/spec/system/claims/schools/claims/create_claim_spec.rb +++ b/spec/system/claims/schools/claims/create_claim_spec.rb @@ -154,7 +154,7 @@ def then_i_check_my_answers within("dl.govuk-summary-list:nth(3)") do within(".govuk-summary-list__row:nth(1)") do expect(page).to have_content("Claim amount") - expect(page).to have_content("£1715.20") + expect(page).to have_content("£1,715.20") end end end From 093fc879774e5a19d04b51975ad468de3550c4a2 Mon Sep 17 00:00:00 2001 From: Daniel Dye Date: Fri, 5 Apr 2024 16:43:23 +0100 Subject: [PATCH 3/4] Update Claim::StatusTagComponent to use the govuk_tag helper Compose upon the govuk tag component. --- app/components/claim/status_tag_component.rb | 16 ++++++++++------ .../claims/schools/claims/view_claim_spec.rb | 4 ++-- .../support/schools/claims/view_claim_spec.rb | 2 +- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/components/claim/status_tag_component.rb b/app/components/claim/status_tag_component.rb index dbf21a399..cd0bd5351 100644 --- a/app/components/claim/status_tag_component.rb +++ b/app/components/claim/status_tag_component.rb @@ -8,19 +8,23 @@ def initialize(claim:, classes: [], html_attributes: {}) end def call - content_tag(:p, claim.status.humanize, class: "govuk-tag #{css_class}") + govuk_tag(text: claim.status.humanize, colour:) end private - def css_class - style_status_classes.fetch(claim.status) + def default_attributes + super.merge!(class: super.class) end - def style_status_classes + def colour + status_colours.fetch(claim.status) + end + + def status_colours { - draft: "govuk-tag--grey", - submitted: "govuk-tag--blue", + draft: "grey", + submitted: "blue", }.with_indifferent_access end end diff --git a/spec/system/claims/schools/claims/view_claim_spec.rb b/spec/system/claims/schools/claims/view_claim_spec.rb index d02ead2d5..20a1bad39 100644 --- a/spec/system/claims/schools/claims/view_claim_spec.rb +++ b/spec/system/claims/schools/claims/view_claim_spec.rb @@ -68,7 +68,7 @@ def then_i_can_then_see_the_submitted_claim_details expect(page).to have_content("Accredited providerBest Practice Network") expect(page).to have_content("Mentors\nBarry Garlow") expect(page).to have_content("Hours of training") - expect(page).to have_content("Status\nSubmitted") + expect(page).to have_content("StatusSubmitted") expect(page).to have_content("Submitted by#{anne.full_name}") expect(page).to have_content("Date submitted 5 March 2024") expect(page).to have_content("Barry Garlow#{mentor_training.hours_completed} hours") @@ -81,7 +81,7 @@ def then_i_can_then_see_the_draft_claim_details expect(page).to have_content("Accredited providerBest Practice Network") expect(page).to have_content("Mentors\nBarry Garlow") expect(page).to have_content("Hours of training") - expect(page).to have_content("Status\nDraft") + expect(page).to have_content("StatusDraft") expect(page).to have_content("Submitted by-") expect(page).to have_content("Date submitted-") expect(page).to have_content("Barry Garlow#{draft_mentor_training.hours_completed} hours") diff --git a/spec/system/claims/support/schools/claims/view_claim_spec.rb b/spec/system/claims/support/schools/claims/view_claim_spec.rb index 71fd5e9e7..5eb66f81d 100644 --- a/spec/system/claims/support/schools/claims/view_claim_spec.rb +++ b/spec/system/claims/support/schools/claims/view_claim_spec.rb @@ -54,7 +54,7 @@ def then_i_can_then_see_the_claim_details expect(page).to have_content("Submitted by#{colin.full_name}") expect(page).to have_content("Date submitted 5 March 2024") expect(page).to have_content("Hours of training") - expect(page).to have_content("Status\nSubmitted") + expect(page).to have_content("StatusSubmitted") expect(page).to have_content("Barry Garlow#{mentor_training.hours_completed} hours") expect(page).to have_content("Claim amount£321.60") end From 264517ac09b32babb696137aaf8d4273f53e450c Mon Sep 17 00:00:00 2001 From: Daniel Dye Date: Fri, 5 Apr 2024 16:45:07 +0100 Subject: [PATCH 4/4] Update Support Claims page - Add Claim::CardComponent - Add pagination count to Support Claims page heading and title - Add search bar to filter claims by reference number --- app/assets/stylesheets/components/_all.scss | 1 + .../stylesheets/components/claim/card.scss | 46 +++++++++++++++++++ app/components/claim/card_component.html.erb | 20 ++++++++ app/components/claim/card_component.rb | 9 ++++ app/queries/claims/claims_query.rb | 7 +++ .../claims/support/claims/index.html.erb | 46 +++++++++++-------- config/locales/en/claims/support/claims.yml | 2 +- spec/components/claim/card_component_spec.rb | 27 +++++++++++ .../previews/claim/card_component_preview.rb | 11 +++++ spec/queries/claims/claims_query_spec.rb | 11 +++++ .../support/claims/view_a_claim_spec.rb | 2 +- .../claims/support/claims/view_claims_spec.rb | 21 +++++---- 12 files changed, 172 insertions(+), 31 deletions(-) create mode 100644 app/assets/stylesheets/components/claim/card.scss create mode 100644 app/components/claim/card_component.html.erb create mode 100644 app/components/claim/card_component.rb create mode 100644 spec/components/claim/card_component_spec.rb create mode 100644 spec/components/previews/claim/card_component_preview.rb diff --git a/app/assets/stylesheets/components/_all.scss b/app/assets/stylesheets/components/_all.scss index eaddcd914..706179696 100644 --- a/app/assets/stylesheets/components/_all.scss +++ b/app/assets/stylesheets/components/_all.scss @@ -1,3 +1,4 @@ +@import "claim/card"; @import "autocomplete"; @import "content_header"; @import "header"; diff --git a/app/assets/stylesheets/components/claim/card.scss b/app/assets/stylesheets/components/claim/card.scss new file mode 100644 index 000000000..81315ab50 --- /dev/null +++ b/app/assets/stylesheets/components/claim/card.scss @@ -0,0 +1,46 @@ +.claim-card { + display: flex; + flex-direction: column; + gap: govuk-spacing(1); + border-bottom: 1px solid $govuk-border-colour; + padding: govuk-spacing(2) 0; + margin-bottom: govuk-spacing(1); + + .claim-card__header { + display: flex; + justify-content: space-between; + align-items: flex-start; + flex-wrap: wrap; + + .claim-card__header__name { + display: flex; + gap: govuk-spacing(1); + + &.govuk-heading-s { + margin-bottom: govuk-spacing(1); + } + } + } + + .claim-card__body { + display: flex; + justify-content: space-between; + gap: govuk-spacing(2); + + .claim-card__body__provider { + flex-grow: 1; + max-width: 66%; + } + + .claim-card__body__right { + display: flex; + flex-direction: column; + justify-content: space-between; + align-items: flex-end; + } + + .govuk-body-s { + margin-bottom: govuk-spacing(1); + } + } +} diff --git a/app/components/claim/card_component.html.erb b/app/components/claim/card_component.html.erb new file mode 100644 index 000000000..df9123c50 --- /dev/null +++ b/app/components/claim/card_component.html.erb @@ -0,0 +1,20 @@ +
+
+
+ <%= govuk_link_to claim.school.name, claims_support_claim_path(claim), no_visited_state: true %> + <%= claim.reference %> +
+
<%= render Claim::StatusTagComponent.new(claim:, classes: %w[govuk-body-s]) %>
+
+ +
+
+
<%= claim.provider.name %>
+
+ +
+
<%= l(claim.created_at.to_date, format: :short) %>
+
<%= humanized_money_with_symbol(claim.amount) %>
+
+
+
diff --git a/app/components/claim/card_component.rb b/app/components/claim/card_component.rb new file mode 100644 index 000000000..c17b3f28e --- /dev/null +++ b/app/components/claim/card_component.rb @@ -0,0 +1,9 @@ +class Claim::CardComponent < ApplicationComponent + attr_reader :claim + + def initialize(claim:, classes: [], html_attributes: {}) + super(classes:, html_attributes:) + + @claim = claim + end +end diff --git a/app/queries/claims/claims_query.rb b/app/queries/claims/claims_query.rb index b71ad56cf..185ab42e9 100644 --- a/app/queries/claims/claims_query.rb +++ b/app/queries/claims/claims_query.rb @@ -1,6 +1,7 @@ class Claims::ClaimsQuery < ApplicationQuery def call scope = Claims::Claim.submitted + scope = search_condition(scope) scope = school_condition(scope) scope = provider_condition(scope) scope.order_created_at_desc @@ -8,6 +9,12 @@ def call private + def search_condition(scope) + return scope if params[:search].blank? + + scope.where(Claims::Claim.arel_table[:reference].matches("%#{params[:search]}%")) + end + def school_condition(scope) return scope if params[:school_ids].blank? diff --git a/app/views/claims/support/claims/index.html.erb b/app/views/claims/support/claims/index.html.erb index aa729dceb..7800b0ab0 100644 --- a/app/views/claims/support/claims/index.html.erb +++ b/app/views/claims/support/claims/index.html.erb @@ -1,33 +1,39 @@ <%= render "claims/support/primary_navigation", current: :claims %> -<%= content_for :page_title, t(".heading") %> +<%= content_for :page_title, t(".heading", count: @pagy.count) %>
+

<%= t(".heading", count: @pagy.count) %>

+ + <%= govuk_button_to(t(".download_csv"), download_csv_claims_support_claims_path, method: :get) %> +
+
+
+
+
Filter
+
+
+
+
-

<%= t(".heading") %>

+ <%= render partial: "shared/search_form", locals: { + url: claims_support_claims_path, + label: { text: t(".search_label"), size: "s" }, + name: :search, + clear_search_url: claims_support_claims_path, + } %> - <% if @claims.any? %> - <%= govuk_button_to(t(".download_csv"), download_csv_claims_support_claims_path, method: :get) %> - - <%= govuk_table do |table| %> - <% table.with_head do |head| %> - <% head.with_row do |row| %> - <% row.with_cell(header: true, text: Claims::Claim.human_attribute_name(:id)) %> - <% row.with_cell(header: true, text: Claims::Claim.human_attribute_name(:status)) %> - <% end %> - <% end %> +
- <% table.with_body do |body| %> - <% @claims.each do |claim| %> - <% body.with_row do |row| %> - <% row.with_cell(text: govuk_link_to(claim.id, claims_support_claim_path(claim))) %> - <% row.with_cell(text: render(Claim::StatusTagComponent.new(claim:))) %> - <% end %> - <% end %> + <% if @claims.any? %> +
+ <% @claims.each do |claim| %> + <%= render Claim::CardComponent.new(claim:) %> <% end %> - <% end %> +
<%= govuk_pagination(pagy: @pagy) %> +

<%= t("pagination_info", from: @pagy.from, to: @pagy.to, count: @pagy.count) %>

diff --git a/config/locales/en/claims/support/claims.yml b/config/locales/en/claims/support/claims.yml index 1159153b4..e5cd73eb5 100644 --- a/config/locales/en/claims/support/claims.yml +++ b/config/locales/en/claims/support/claims.yml @@ -3,7 +3,7 @@ en: support: claims: index: - heading: Claims + heading: Claims (%{count}) download_csv: Download claims search_label: Search by claim reference show: diff --git a/spec/components/claim/card_component_spec.rb b/spec/components/claim/card_component_spec.rb new file mode 100644 index 000000000..816b7ed87 --- /dev/null +++ b/spec/components/claim/card_component_spec.rb @@ -0,0 +1,27 @@ +require "rails_helper" + +RSpec.describe Claim::CardComponent, type: :component do + include Rails.application.routes.url_helpers + + subject(:component) { described_class.new(claim:) } + + let(:claim) do + create(:claim, :submitted, created_at: "2024/04/08", school:) do |claim| + claim.mentor_trainings << create(:mentor_training, hours_completed: 20) + end + end + + let(:school) { create(:claims_school, region: regions(:inner_london)) } + + it "renders a card with claim details" do + render_inline(component) + + expect(page).to have_link(claim.school.name, href: claims_support_claim_path(claim)) + expect(page).to have_content(claim.reference) + expect(page).to have_content("Submitted") + + expect(page).to have_content(claim.provider.name) + expect(page).to have_content("08/04/2024") + expect(page).to have_content("£1,072") + end +end diff --git a/spec/components/previews/claim/card_component_preview.rb b/spec/components/previews/claim/card_component_preview.rb new file mode 100644 index 000000000..d9faf4e50 --- /dev/null +++ b/spec/components/previews/claim/card_component_preview.rb @@ -0,0 +1,11 @@ +class Claim::CardComponentPreview < ApplicationComponentPreview + def default + render Claim::CardComponent.new(claim:) + end + + private + + def claim + Claims::Claim.submitted.first + end +end diff --git a/spec/queries/claims/claims_query_spec.rb b/spec/queries/claims/claims_query_spec.rb index cb6ee771c..aabdb10ed 100644 --- a/spec/queries/claims/claims_query_spec.rb +++ b/spec/queries/claims/claims_query_spec.rb @@ -14,6 +14,17 @@ expect(claims_query).to eq([submitted_claim]) end + context "when given a search query" do + let(:params) { { search: "1234" } } + + it "filters the results by provided school ids" do + claim_with_partial_reference = create(:claim, :submitted, reference: "12345678") + _claim_without_partial_reference = create(:claim, :submitted, reference: "87654321") + + expect(claims_query).to match_array([claim_with_partial_reference]) + end + end + context "when given school ids" do let(:school) { create(:claims_school) } let(:params) { { school_ids: [school.id] } } diff --git a/spec/system/claims/support/claims/view_a_claim_spec.rb b/spec/system/claims/support/claims/view_a_claim_spec.rb index 931497147..595e47925 100644 --- a/spec/system/claims/support/claims/view_a_claim_spec.rb +++ b/spec/system/claims/support/claims/view_a_claim_spec.rb @@ -36,7 +36,7 @@ def when_i_visit_claim_index_page end def when_i_click_on_claim(claim) - click_on(claim.id) + click_on(claim.school.name) end def then_i_can_see_the_details_of_a_submitted_claim diff --git a/spec/system/claims/support/claims/view_claims_spec.rb b/spec/system/claims/support/claims/view_claims_spec.rb index c2c0bfa88..3a754685f 100644 --- a/spec/system/claims/support/claims/view_claims_spec.rb +++ b/spec/system/claims/support/claims/view_claims_spec.rb @@ -13,6 +13,7 @@ scenario "Support user visits the claims index page" do when_i_visit_claim_index_page then_i_see_a_list_of_submitted_claims + and_i_see_no_draft_claims end private @@ -27,15 +28,17 @@ def when_i_visit_claim_index_page end def then_i_see_a_list_of_submitted_claims - expect(page).to have_content("ID") - expect(page).to have_content("Status") - expect(page).to have_selector("tbody tr", count: 1) - - expect(page).not_to have_selector("td", text: claim_1.id) - - within("tbody tr:nth-child(1)") do - expect(page).to have_selector("td", text: claim_2.id) - expect(page).to have_selector("td", text: "Submitted") + within(".claim-card:nth-child(1)") do + expect(page).to have_content(claim_2.school.name) + expect(page).to have_content(claim_2.reference) + expect(page).to have_content("Submitted") + expect(page).to have_content(claim_2.provider.name) + expect(page).to have_content(I18n.l(claim_2.created_at.to_date, format: :short)) + expect(page).to have_content(claim_2.amount.format(no_cents_if_whole: true)) end end + + def and_i_see_no_draft_claims + expect(page).not_to have_content(claim_1.reference) + end end