Conversation
There was a problem hiding this comment.
Pull request overview
This PR, "DDCE-7698 Update to exclusion journey", continues the incremental rollout of the MPBIK (Mandatory Payrolling of Benefits In Kind) feature behind a feature toggle (pbikAppConfig.mpbikToggle). It introduces a new payrolling summary page, a new MPBIK-specific exclusion search results view, a new route for the combined registered-benefits-expenses page, and updates multiple views and controllers to use the toggle to route users to the correct page.
Changes:
- Adds a new
/registered-benefits-expensesroute andHomePageController.onPageLoadaction that renders the newPayrollingSummaryPageMpbikview when the toggle is enabled. - Introduces two new Twirl templates (
PayrollingSummaryPageMpbik.scala.htmlandSearchResultsMPBIK.scala.html) with corresponding Welsh and English messages, and updates numerous existing views to accept ampbik: Booleanparameter and conditionally resolve the correct summary page link. - Updates tests (view specs and controller specs) to branch on the toggle, updates dependency versions (
sbt-plugin3.0.10,bootstrap-play10.6.0,play-frontend-hmrc12.32.0), and temporarily lowers code coverage threshold from 95% to 91%.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
app/controllers/HomePageController.scala |
Adds onPageLoad action rendering new MPBIK summary view when toggle is on |
app/controllers/ExclusionListController.scala |
Conditionally uses SearchResultsMPBIK view and updated error pages with mpbik flag |
app/controllers/WhatNextPageController.scala |
Passes mpbik toggle to confirmation views |
app/controllers/StartPageController.scala |
Renames view import from StartPageMpbikToggle to StartPageMpbik; adds TODO comments |
app/views/PayrollingSummaryPageMpbik.scala.html |
New payrolling summary page for the MPBIK journey |
app/views/exclusion/SearchResultsMPBIK.scala.html |
New MPBIK-specific exclusion search results view |
app/views/StartPageMpbik.scala.html |
Updates "Start now" link to point to new onPageLoad route |
app/views/registration/AddBenefitConfirmationNextTaxYear.scala.html |
Adds mpbik parameter to conditionally resolve summary link |
app/views/registration/RemoveBenefitConfirmationNextTaxYear.scala.html |
Adds mpbik parameter to conditionally resolve summary link |
app/views/registration/CurrentTaxYear.scala.html |
Adds mpbik parameter for back-link routing |
app/views/exclusion/WhatNextExclusion.scala.html |
Adds mpbik parameter for summary link routing |
app/views/exclusion/WhatNextRescind.scala.html |
Adds mpbik parameter for summary link routing |
app/views/ErrorPage.scala.html |
Adds mpbik parameter for summary link routing |
app/utils/ControllersReferenceData.scala |
Passes mpbik to error page views |
app/services/RegistrationService.scala |
Passes mpbik to error page view |
conf/app.routes |
Adds /registered-benefits-expenses route |
conf/messages / conf/messages.cy |
Adds PayrollingSummaryMPBIK.* and ExclusionSearchMPBIK.* message keys |
test/views/PayrollingSummaryPageViewSpec.scala |
New test for PayrollingSummaryPageMpbik view |
test/views/SummaryViewSpec.scala |
Branches on toggle to test old or new summary view |
test/controllers/HomePageControllerSpec.scala |
Branches on toggle to test old or new home page behaviour |
test/controllers/ExclusionListControllerSpec.scala |
Updates expected redirect URLs based on toggle |
project/AppDependencies.scala / project/plugins.sbt |
Bumps dependency versions |
project/CodeCoverageSettings.scala |
Temporarily lowers coverage threshold to 91% |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import scala.language.postfixOps | ||
|
|
There was a problem hiding this comment.
The import scala.language.postfixOps is unused — there are no postfix operator usages in this file. This import should be removed.
| import scala.language.postfixOps |
|
|
||
| <h2 class="govuk-heading-m">@expenseOrBenefit</h2> | ||
|
|
||
| @defining( if(year=="cy"){""+taxYearRange.cyminus1}else{""+taxYearRange.cy} ) { yearvalue => |
There was a problem hiding this comment.
The yearvalue variable is bound inside a @defining block on line 63 but is never referenced anywhere in the template body. This unused binding adds confusion to the code and should either be used or removed along with the @defining wrapper if it's no longer needed.
| @defining( if(year=="cy"){""+taxYearRange.cyminus1}else{""+taxYearRange.cy} ) { yearvalue => | |
| @defining( if(year=="cy"){""+taxYearRange.cyminus1}else{""+taxYearRange.cy} ) { _ => |
|
|
||
| @govukWarningText(WarningText( | ||
| iconFallbackText = Some(messages("site.warning")), | ||
| content = Text(messages("ExclusionSearch.warning")) |
There was a problem hiding this comment.
In the multiple-matches branch (listOfMatches.length > 1), the warning text uses ExclusionSearch.warning ("You will not be able to payroll this benefit or expense for this employee again in this tax year.") instead of ExclusionSearchMPBIK.warning ("You will not be able to payroll this benefit for this employee again for this tax year."). This is an inconsistency between the two branches of the same MPBIK-specific view: the single-match path correctly uses ExclusionSearchMPBIK.warning, but the multiple-match path uses the old generic warning that mentions "benefit or expense", whereas the MPBIK journey only deals with benefits.
| content = Text(messages("ExclusionSearch.warning")) | |
| content = Text(messages("ExclusionSearchMPBIK.warning")) |
| class PayrollingSummaryPageViewSpec extends PBIKViewSpec { | ||
|
|
||
| private val payrollingSummaryPageView: PayrollingSummaryPageMpbik = injected[PayrollingSummaryPageMpbik] | ||
| private val carIabdType: String = IabdType.CarBenefit.id.toString |
There was a problem hiding this comment.
The carIabdType variable is declared and assigned but never used anywhere in this test file. It should be removed to avoid dead code.
| private val carIabdType: String = IabdType.CarBenefit.id.toString |
bf824ba to
244f92c
Compare
|
074c7a6 to
d30f4f2
Compare
|
No description provided.