Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Providers Reports index #9359

Merged
merged 9 commits into from
May 15, 2024
3 changes: 2 additions & 1 deletion app/controllers/provider_interface/reports_controller.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
module ProviderInterface
class ReportsController < ProviderInterfaceController
def index
@providers = current_user.providers
@providers = current_user.providers.preload(:performance_reports)
@performance_reports = current_user.providers.any? { _1.performance_reports.present? }
end
end
end
2 changes: 2 additions & 0 deletions app/models/provider.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ class Provider < ApplicationRecord
has_many :vendor_api_requests
has_many :vendor_api_tokens

has_many :performance_reports, class_name: 'Publications::ProviderRecruitmentPerformanceReport'

enum region_code: {
east_midlands: 'east_midlands',
eastern: 'eastern',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,9 @@ module Publications
class ProviderRecruitmentPerformanceReport < ApplicationRecord
belongs_to :provider
validates :cycle_week, :publication_date, presence: true

def reporting_date
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the name of this one is confusing. I would think 'reporting date' would be the date the report was created, which is the publications date. Maybe reporting_end_date. report_range_end_date

CycleTimetable.cycle_week_date_range(cycle_week).last.to_date
end
end
end
18 changes: 15 additions & 3 deletions app/services/cycle_timetable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -287,20 +287,32 @@ def self.date(name, year = current_year)
schedule.fetch(name)
end

#
# cycle_week methods
#

def self.current_cycle_week(time = Time.zone.now)
weeks = (time.to_date - find_opens(current_year(time)).beginning_of_week.to_date).to_i / 7
(weeks % 52).succ
end

def self.start_of_cycle_week(cycle_week, time = Time.zone.now)
def self.cycle_week_date_range(cycle_week, time = Time.zone.now)
year = current_year(time)
cycle_week %= 52
cycle_week -= 1

start_of_week = find_opens(year).beginning_of_week + cycle_week.weeks
[start_of_week, find_opens(year)].max
start_of_week = find_opens(year) + cycle_week.weeks
start_of_week.all_week
end

def self.start_of_cycle_week(...)
cycle_week_date_range(...).first
end

#
# cycle_schedule methods
#

def self.current_cycle_schedule
# Make sure this setting only has effect on non-production environments
return :real if HostingEnvironment.production?
Expand Down
89 changes: 56 additions & 33 deletions app/views/provider_interface/reports/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,46 +5,69 @@
<h1 class="govuk-heading-l">
<%= t('page_titles.provider.reports') %>
</h1>
<ul class="govuk-list govuk-list--spaced">
<li>
<%= govuk_link_to t('page_titles.provider.export_application_data'), provider_interface_new_application_data_export_path %>
</li>
<li>
<%= govuk_link_to t('page_titles.provider.export_hesa_data'), provider_interface_reports_hesa_exports_path %>
</li>
<% if @providers.one? %>
<li>
<%= govuk_link_to t('page_titles.provider.status_of_active_applications'), provider_interface_reports_provider_status_of_active_applications_path(provider_id: @providers.first) %>
</li>
<li>
<%= govuk_link_to t('page_titles.provider.diversity_report'), provider_interface_reports_provider_diversity_report_path(provider_id: @providers.first) %>
</li>
<li>
<%= govuk_link_to t('page_titles.provider.withdrawal_report'), provider_interface_reports_provider_withdrawal_report_path(provider_id: @providers.first) %>
</li>
<% if mid_cycle_report_present_for?(@providers.first) %>
<%= govuk_link_to mid_cycle_report_label_for(@providers.first), provider_interface_reports_provider_mid_cycle_report_path(provider_id: @providers.first) %>

<% if FeatureFlag.active?(:recruitment_performance_report) %>
<% if @performance_reports %>
<h2 class="govuk-heading-m">
Weekly recruitment performance report
Copy link
Contributor

@elceebee elceebee May 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be consistent with the locales and put this in a locale file. (same for the other text on this page)

</h2>
<% end %>

<% @providers.select { _1.performance_reports.any? }.each do |provider| %>
<% if @providers.many? %>
<h3 class="govuk-heading-s"><%= provider.name %></h3>
<% end %>
<% report = provider.performance_reports.last %>
<ul class="govuk-list govuk-list--spaced">
<li><%= govuk_link_to("Weekly report for week ending #{report.reporting_date.to_fs(:govuk_date)}", provider_interface_reports_provider_recruitment_performance_report_path(provider.id)) %></li>
</ul>
<% end %>
</ul>
<% if @providers.many? %>
<% @providers.each do |provider| %>
<h2 class="govuk-heading-m"><%= provider.name %></h2>
<% else %>
<% if @providers.any? { mid_cycle_report_present_for?(_1) } %>
<h2 class="govuk-heading-m">
Mid-cycle recruitment performance report
</h2>
<% end %>

<% @providers.select { mid_cycle_report_present_for?(_1) }.each do |provider| %>
<h3 class="govuk-heading-s"><%= provider.name %></h3>

<ul class="govuk-list govuk-list--spaced">
<li>
<%= govuk_link_to t('page_titles.provider.status_of_active_applications'), provider_interface_reports_provider_status_of_active_applications_path(provider_id: provider) %>
<% if mid_cycle_report_present_for?(provider) %>
<%= govuk_link_to mid_cycle_report_label_for(provider), provider_interface_reports_provider_mid_cycle_report_path(provider_id: provider) %>
<% end %>
</li>
<li>
<%= govuk_link_to t('page_titles.provider.diversity_report'), provider_interface_reports_provider_diversity_report_path(provider_id: provider) %>
</li>
<li>
<%= govuk_link_to t('page_titles.provider.withdrawal_report'), provider_interface_reports_provider_withdrawal_report_path(provider_id: provider) %>
</li>
<% if mid_cycle_report_present_for?(provider) %>
<%= govuk_link_to mid_cycle_report_label_for(provider), provider_interface_reports_provider_mid_cycle_report_path(provider_id: provider) %>
<% end %>
</ul>
<% end %>
<% end %>

<h2 class="govuk-heading-m">Application data for this recruitment cycle</h2>
<% @providers.each do |provider| %>
<% if @providers.many? %>
<h3 class="govuk-heading-s"><%= provider.name %></h3>
<% end %>
<ul class="govuk-list govuk-list--spaced">
<li>
<%= govuk_link_to t('page_titles.provider.status_of_active_applications'), provider_interface_reports_provider_status_of_active_applications_path(provider_id: provider) %>
</li>
<li>
<%= govuk_link_to t('page_titles.provider.diversity_report'), provider_interface_reports_provider_diversity_report_path(provider_id: provider) %>
</li>
<li>
<%= govuk_link_to t('page_titles.provider.withdrawal_report'), provider_interface_reports_provider_withdrawal_report_path(provider_id: provider) %>
</li>
</ul>
<% end %>

<h2 class="govuk-heading-m">Download and export</h2>
<ul class="govuk-list govuk-list--spaced">
<li>
<%= govuk_link_to t('page_titles.provider.export_application_data'), provider_interface_new_application_data_export_path %>
</li>
<li>
<%= govuk_link_to t('page_titles.provider.export_hesa_data'), provider_interface_reports_hesa_exports_path %>
</li>
</ul>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,11 @@
it { is_expected.to validate_presence_of :publication_date }
it { is_expected.to validate_presence_of :cycle_week }
end

describe '#reporting_date' do
it 'returns the date of the last day of the cycle week', time: Time.zone.local(2024, 6, 6) do
report = create(:provider_recruitment_performance_report, cycle_week: 35)
expect(report.reporting_date).to eq(Date.new(2024, 6, 2))
end
end
end
16 changes: 14 additions & 2 deletions spec/services/cycle_timetable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,18 @@ def create_application_for(recruitment_cycle_year)
end
end

describe '#cycle_week_date_range' do
let(:date) { Time.zone.local(2023, 10, 30) }

before { TestSuiteTimeMachine.travel_permanently_to(date) }

it 'returns the correct date range for cycle_week 5' do
cycle_week_date_range = described_class.cycle_week_date_range(5)

expect(cycle_week_date_range).to eql(date.all_week)
end
end

describe '#start_of_cycle_week' do
context 'without time argument' do
let(:date) { Time.zone.local(2023, 10, 30) }
Expand All @@ -742,9 +754,9 @@ def create_application_for(recruitment_cycle_year)
expect(start_of_week_10.wday).to eq 1
end

it 'returns when find opens if the monday is before the cycle begins' do
it 'returns the monday before find_opens for the first week of the cycle' do
start_of_week_one = described_class.start_of_cycle_week(1)
expect(start_of_week_one.to_date).to eq described_class.find_opens.to_date
expect(start_of_week_one.to_date).to eq described_class.find_opens.beginning_of_week.to_date
end

it 'only returns dates in current cycle year' do
Expand Down
105 changes: 0 additions & 105 deletions spec/system/provider_interface/provider_reports_page_spec.rb

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
require 'rails_helper'

RSpec.feature 'Provider reports index' do
include DfESignInHelpers
scenario 'when provider user has one provider' do
given_a_provider_and_provider_user_exists
and_i_am_signed_in_as_provider_user
when_i_visit_the_reports_index
then_the_page_has_the_right_content
end

def when_i_visit_the_reports_index
visit provider_interface_reports_path
expect(page).to have_current_path('/provider/reports')
end

def given_a_provider_and_provider_user_exists
@provider_user = create(:provider_user, :with_dfe_sign_in, email_address: '[email protected]')
@provider = @provider_user.providers.first
end

def then_the_page_has_the_right_content
expect(page).to have_css('h1', text: 'Reports')
expect(page).to have_css('h2', text: 'Application data for this recruitment cycle')
expect(page).to have_link('Status of active applications', href: provider_interface_reports_provider_status_of_active_applications_path(provider_id: @provider))
expect(page).to have_link('Sex, disability, ethnicity and age of candidates', href: provider_interface_reports_provider_diversity_report_path(provider_id: @provider))
expect(page).to have_link('Withdrawals', href: provider_interface_reports_provider_withdrawal_report_path(provider_id: @provider))
expect(page).to have_css('h2', text: 'Download and export')
expect(page).to have_link('Export application data', href: provider_interface_new_application_data_export_path)
expect(page).to have_link('Export data for Higher Education Statistics Agency (HESA)', href: provider_interface_reports_hesa_exports_path)
end

def and_i_am_signed_in_as_provider_user
provider_exists_in_dfe_sign_in
provider_signs_in_using_dfe_sign_in
expect(page).to have_current_path('/provider/applications')
end
end
Loading
Loading