Skip to content

Commit

Permalink
[2282] Improve text 3/6 update location row on search result (#4539)
Browse files Browse the repository at this point in the history
* Add course factory with 2 sites

* Update the Location row in Find Course search results

  - Refactor the SearchResults components
  - Stop passing 'search_params' through the component stack
  - The `ResultsView#query_parameters` contains the `search_params` anyway

* Add SearchResultComponent with School placement variations

* Update fee based course copy

* Replace Location with School on Course show page
  • Loading branch information
inulty-dfe authored Sep 27, 2024
1 parent 94f3fae commit 650254e
Show file tree
Hide file tree
Showing 20 changed files with 365 additions and 131 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

<% if course.fee_based? %>
<p class="govuk-body">
You’ll be placed in schools for most of your course to get classroom experience. You will also spend time at a location where you will study.
You should get 120 days of classroom experience in schools. You will also spend time at a location where you will study.
</p>
<% else %>
<p class="govuk-body">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<%= row.with_value do %>
<p class="govuk-body"><%= potential_placements_text %></p>
<%= govuk_link_to(t("view_list_of_school_placements"), placements_url) if show_school_placements_link? %>
<p class="govuk-hint govuk-!-font-size-16">Locations can change and are not guaranteed</p>
<p class="govuk-hint govuk-!-font-size-16">Schools can change and are not guaranteed</p>
<% end %>
<% end %>
<%= summary_list.with_row do |row| %>
Expand Down
2 changes: 1 addition & 1 deletion app/components/find/courses/training_locations/view.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def placements_url

def potential_placements_text
if course.fee_based?
pluralize(course.sites.size, 'potential placement location')
pluralize(course.sites.size, 'potential placement school')
else
pluralize(course.sites.size, 'potential employing school')
end
Expand Down
3 changes: 1 addition & 2 deletions app/components/find/results/results_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@
<%= render Find::Results::SearchResultComponent.new(
course:,
filtered_by_location: results.location_filter?,
sites_count: results.sites_count(course),
search_params:
results_view: results
) %>
<% end %>
</ul>
Expand Down
5 changes: 2 additions & 3 deletions app/components/find/results/results_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ module Results
class ResultsComponent < ViewComponent::Base
include ::ViewHelper

attr_reader :results, :courses, :search_params
attr_reader :results, :courses

def initialize(results:, courses:, search_params:)
def initialize(results:, courses:)
super
@results = results
@courses = courses
@search_params = search_params
end
end
end
Expand Down
17 changes: 11 additions & 6 deletions app/components/find/results/search_result_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
<li class="app-search-results__item" data-qa="course">
<%= govuk_summary_card(classes: ["course-summary-card"], title: course_title_link) do |card| %>
<%= card.with_summary_list(actions: false, classes: ["govuk-summary-list--no-border"]) do |summary_list| %>
<% if filtered_by_location? && has_sites? %>
<% summary_list.with_row do |row| %>
<% if course.provider.decorate.website.present? %>
<% if course.provider.decorate.website.present? %>
<% summary_list.with_row do |row| %>
<% row.with_key(text: location_label) %>
<% row.with_value do %>
<% if course.university_based? %>
<%= render partial: "find/results/university", locals: { course: } %>
<% if has_sites? %>
<% if filtered_by_location? %>
<%= render partial: "find/results/by_location",
locals: { site_distance:, location_name:, school_term:, sites_count: } %>
<% else %>
<%= render partial: "find/results/non_university", locals: { course: } %>
<%= render partial: "find/results/by_country_or_provider",
locals: { sites_count:, school_term: } %>
<% end %>

<% else %>
Not listed yet
<% end %>
<% end %>
<% end %>
Expand Down
35 changes: 28 additions & 7 deletions app/components/find/results/search_result_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,58 @@ module Results
class SearchResultComponent < ViewComponent::Base
include ::ViewHelper

attr_reader :course
attr_reader :course, :sites_count

delegate :age_range_in_years_and_level, :course_length_with_study_mode, to: :course

def initialize(course:, search_params:, filtered_by_location: false, sites_count: 0)
def initialize(course:, results_view:, filtered_by_location: false)
super
@course = course.decorate
@filtered_by_location = filtered_by_location
@sites_count = sites_count
@search_params = search_params
@sites_count = results_view.sites_count(course)
@results_view = results_view
@search_params = results_view.query_parameters.to_query
end

def filtered_by_location?
@filtered_by_location
end

def has_sites?
@sites_count.positive?
sites_count.positive?
end

def course_title_link
t(
'.course_title_html',
course_path: find_course_path(provider_code: course.provider_code, course_code: course.course_code, search_params: @search_params.to_query),
course_path: find_course_path(provider_code: course.provider_code, course_code: course.course_code, search_params: @search_params),
provider_name: helpers.smart_quotes(course.provider.provider_name),
course_name: course.name_and_code
)
end

def location_label
t('.location', count: @sites_count)
if course.fee_based?
t('.fee_based.location', count: sites_count)
else
t('.salary_based.location', count: sites_count)
end
end

def location_name
@results_view.query_parameters['lq']
end

def school_term
if course.fee?
'placement'
else
'employing'
end
end

def site_distance
@results_view.site_distance(course)
end

private
Expand Down
3 changes: 3 additions & 0 deletions app/views/find/results/_by_country_or_provider.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<span>
<%= sites_count %> potential <%= school_term %> <%= "school".pluralize(sites_count) %>
</span>
10 changes: 10 additions & 0 deletions app/views/find/results/_by_location.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<div>
<span><strong><%= pluralize(site_distance, "mile") %></strong> from <strong><%= location_name %></strong></span>
</div>
<span class="govuk-hint govuk-!-font-size-16">
<% if sites_count == 1 %>
Nearest potential <%= school_term %> school
<% elsif %>
Nearest of <%= sites_count %> potential <%= school_term %> schools
<% end %>
</span>
18 changes: 0 additions & 18 deletions app/views/find/results/_non_university.html.erb

This file was deleted.

24 changes: 0 additions & 24 deletions app/views/find/results/_university.html.erb

This file was deleted.

3 changes: 1 addition & 2 deletions app/views/find/results/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,5 @@

<%= render Find::Results::ResultsComponent.new(
results: @results_view,
courses: @courses,
search_params: @search_params
courses: @courses
) %>
11 changes: 8 additions & 3 deletions config/locales/find.yml
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,14 @@ en:
results:
search_result_component:
no_degree_required: No degree required
location:
one: Location
other: Nearest location
fee_based:
location:
one: Placement school
other: Placement schools
salary_based:
location:
one: Employing school
other: Employing schools
nearest_location: Nearest location
fee_or_salary: Fee or salary
course_fee: Course fee
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@

result = render_inline(described_class.new(course))

expect(result.text).to include('You’ll be placed in schools for most of your course to get classroom experience. You will also spend time at a location where you will study.')
expect(result.text).to include('You should get 120 days of classroom experience in schools. You will also spend time at a location where you will study.')
end
end

Expand Down
8 changes: 4 additions & 4 deletions spec/components/find/courses/training_locations/view_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
end

it 'renders the hint about placements not being guaranteed' do
expect(subject).to have_css('.govuk-hint', text: 'Locations can change and are not guaranteed')
expect(subject).to have_css('.govuk-hint', text: 'Schools can change and are not guaranteed')
end
end

Expand All @@ -39,7 +39,7 @@
end

it 'renders the hint about placements not being guaranteed' do
expect(subject).to have_css('.govuk-hint', text: 'Locations can change and are not guaranteed')
expect(subject).to have_css('.govuk-hint', text: 'Schools can change and are not guaranteed')
end

it "renders 'Where you will study' for study sites" do
Expand Down Expand Up @@ -120,7 +120,7 @@
let(:course) { create(:course, :with_full_time_sites, funding: 'fee', study_sites: [build(:site, :study_site)]) }

it 'returns the correct text for one potential placement location' do
expect(component.potential_placements_text).to eq('1 potential placement location')
expect(component.potential_placements_text).to eq('1 potential placement school')
end
end

Expand All @@ -136,7 +136,7 @@
let(:course) { create(:course, funding: 'fee', sites: [create(:site), create(:site), create(:site)]) }

it 'returns the correct text for multiple placements' do
expect(component.potential_placements_text).to eq('3 potential placement locations')
expect(component.potential_placements_text).to eq('3 potential placement schools')
end
end
end
Expand Down
46 changes: 11 additions & 35 deletions spec/components/find/results/results_component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,6 @@ module Find
end

context 'when there are no search results' do
let(:search_params) do
{ 'age_group' => 'primary',
'applications_open' => 'true',
'can_sponsor_visa' => 'false',
'has_vacancies' => 'true',
'l' => '2',
'subjects' => ['00'],
'visa_status' => 'false' }
end

let(:results_view) do
instance_double(
Find::ResultsView,
Expand All @@ -30,40 +20,29 @@ module Find
subjects: [],
number_of_courses_string: 'No courses',
no_results_found?: true,
has_results?: false,
sites_count: 0
has_results?: false
)
end

let(:courses) { ::Course.all.page(1) }

it 'renders a "No courses found" message when there are no results' do
component = render_inline(
described_class.new(results: results_view, courses:, search_params:)
described_class.new(results: results_view, courses:)
)

expect(component.text).to include('No courses found')
end

it 'renders the inset text' do
component = render_inline(
described_class.new(results: results_view, courses:, search_params:)
described_class.new(results: results_view, courses:)
)
expect(component.text).to include('event near you')
end
end

context 'when there are 10 matching courses' do
let(:search_params) do
{ 'age_group' => 'primary',
'applications_open' => 'true',
'can_sponsor_visa' => 'false',
'has_vacancies' => 'true',
'l' => '2',
'subjects' => ['00'],
'visa_status' => 'false' }
end

let(:results_view) do
instance_double(
Find::ResultsView,
Expand All @@ -73,30 +52,28 @@ module Find
number_of_courses_string: '10 courses',
no_results_found?: false,
has_results?: true,
location_filter?: false,
sites_count: 2
location_filter?: false
)
end

let(:courses) { ::Course.all.page(1) }

before do
create_list(:course, 10)
create_list(:course, 10, :with_2_full_time_sites)
end

it 'renders "10 courses found" and a `SearchResultComponent` for each course' do
allow(Results::SearchResultComponent).to receive(:new).and_return(plain: '')

component = render_inline(
described_class.new(results: results_view, courses:, search_params:)
described_class.new(results: results_view, courses:)
)

courses.each do |course|
expect(Results::SearchResultComponent).to have_received(:new).with(
course:,
search_params:,
filtered_by_location: false,
sites_count: 2
results_view:,
filtered_by_location: false
)
end

Expand All @@ -105,15 +82,14 @@ module Find

it 'renders the inset text' do
component = render_inline(
described_class.new(results: results_view, courses:, search_params:)
described_class.new(results: results_view, courses:)
)

courses.each do |course|
expect(Results::SearchResultComponent).to have_received(:new).with(
course:,
search_params:,
filtered_by_location: false,
sites_count: 2
results_view:,
filtered_by_location: false
)
end
expect(component.text).to include('event near you')
Expand Down
Loading

0 comments on commit 650254e

Please sign in to comment.