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
Merged

Conversation

inulty-dfe
Copy link
Collaborator

@inulty-dfe inulty-dfe commented May 14, 2024

Context

Update the reports section in the Provider Interface to show link to the new Recruitment Performance Report and remove the link to the old mid-cycle report

Changes proposed in this pull request

The page has multiple potential states. The context is always for a particular provider user.

  1. Provider User has one provider they can access reports for
  2. Provider User has multiple providers they can access reports for
  3. Each provider may or may not have a mid-cycle report generated
  4. Each provider may or may not have a recruitment performance report generated
  5. When the FeatureFlag is enabled, we show any recruitment performance reports but not the mid-cycle reports
  6. When the FeatureFlag is disabled, we show any mid-cycle reports but not the recruitment performance reports

One provider for the user

Performance Report active Performance Report inactive
image image

Multiple providers for the user

Performance Report active Performance Report inactive
image image

In order to calculate the date up to which the report counts candidates, I've added CycleTimetable.cycle_week_date_range. We can get the first and last day of the cycle week from this method as a bonus.

Guidance to review

Providers 8 and 1 have recruitment performance reports generated for them
https://apply-review-9359.test.teacherservices.cloud/support/providers/8/users

Link to Trello card

Trello Ticket

Things to check

  • If the code removes any existing feature flags, a data migration has also been added to delete the entry from the database
  • This code does not rely on migrations in the same Pull Request
  • If this code includes a migration adding or changing columns, it also backfills existing records for consistency
  • If this code adds a column to the DB, decide whether it needs to be in analytics yml file or analytics blocklist
  • API release notes have been updated if necessary
  • If it adds a significant user-facing change, is it documented in the CHANGELOG?
  • Required environment variables have been updated added to the Azure KeyVault

  Add Recruitment Performance report
  Show mid-cycle / recruitment performance based on FeatureFlag
  Add specs for single / multiple providers
@inulty-dfe inulty-dfe force-pushed the 1634-update-provider-reports-view branch from 0d6795b to 90dc2a5 Compare May 14, 2024 10:15
@inulty-dfe inulty-dfe added the deploy_v2 Deploy the review app to AKS label May 14, 2024
@github-actions github-actions bot temporarily deployed to review_aks-9359 May 14, 2024 10:31 Destroyed
  This method gives us the ability to get the first and last day of a
  cycle_week.
  Also make change to .start_of_cycle_week
   - The first day of the first cycle_week should be Monday, even if
     that day is not in the cycle
  - use :govuk_date to display the last date the report comprises
@github-actions github-actions bot temporarily deployed to review_aks-9359 May 14, 2024 11:19 Destroyed
@github-actions github-actions bot temporarily deployed to review_aks-9359 May 14, 2024 11:49 Destroyed
@github-actions github-actions bot temporarily deployed to review_aks-9359 May 14, 2024 12:01 Destroyed
@inulty-dfe inulty-dfe requested a review from elceebee May 14, 2024 12:04
@github-actions github-actions bot temporarily deployed to review_aks-9359 May 14, 2024 12:12 Destroyed
@inulty-dfe inulty-dfe marked this pull request as ready for review May 14, 2024 12:18
@github-actions github-actions bot temporarily deployed to review_aks-9359 May 14, 2024 12:27 Destroyed
@inulty-dfe inulty-dfe force-pushed the 1634-update-provider-reports-view branch from 8f80578 to cc57bf2 Compare May 14, 2024 12:33
@github-actions github-actions bot temporarily deployed to review_aks-9359 May 14, 2024 12:39 Destroyed
@@ -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

<% 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)

Copy link
Contributor

@elceebee elceebee left a comment

Choose a reason for hiding this comment

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

I've tested this locally and it all looks good to me. I have a couple of minor suggestions, but they are not blockers.

@github-actions github-actions bot temporarily deployed to review_aks-9359 May 14, 2024 17:02 Destroyed
Copy link
Contributor

@elceebee elceebee left a comment

Choose a reason for hiding this comment

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

🚀

@inulty-dfe inulty-dfe merged commit 9c13f76 into main May 15, 2024
24 checks passed
@inulty-dfe inulty-dfe deleted the 1634-update-provider-reports-view branch May 15, 2024 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy_v2 Deploy the review app to AKS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants