Skip to content

Conversation

@struan
Copy link
Member

@struan struan commented Dec 9, 2025

This is mostly based on the Sutton code with a few Kingston specific changes to add checkbox extra details to the bin not returned, plus an extra message for FAS properties for spillages.

Fixes mysociety/societyworks#5245
Fixes mysociety/societyworks#5246

[skip changelog]

@codecov
Copy link

codecov bot commented Dec 9, 2025

Codecov Report

❌ Patch coverage is 98.21429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.14%. Comparing base (13cc03e) to head (02b6e09).
⚠️ Report is 29 commits behind head on master.

Files with missing lines Patch % Lines
perllib/FixMyStreet/Cobrand/Kingston.pm 94.44% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5768      +/-   ##
==========================================
+ Coverage   82.59%   83.14%   +0.54%     
==========================================
  Files         457      457              
  Lines       35689    38061    +2372     
  Branches     5814     6366     +552     
==========================================
+ Hits        29479    31646    +2167     
- Misses       4515     4694     +179     
- Partials     1695     1721      +26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@struan struan force-pushed the 5244-kingston-spillages-not-returned branch 3 times, most recently from 700ebbf to 2c9d9c6 Compare December 16, 2025 15:18
@struan struan marked this pull request as ready for review December 16, 2025 15:20
@struan struan requested a review from dracos December 16, 2025 15:21
@struan struan force-pushed the 5244-kingston-spillages-not-returned branch from 2c9d9c6 to 81d14ec Compare December 16, 2025 16:06
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

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

Looks good, some refactoring mostly I think :)

@struan struan requested a review from dracos December 17, 2025 14:48
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

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

Tiny issue with the email template to resolve, symlink thing I'm sure we can fix.

[% END %]

[% ELSIF cobrand.moniker == 'kingston' AND report.category == 'Request new container' %]
[% ELSIF cobrand.moniker == 'kingston'%]
Copy link
Member

Choose a reason for hiding this comment

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

There's one more kingston check unneeded in the second sub-IF.

Copy link
Member

Choose a reason for hiding this comment

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

And same issue with missed collections as the HTML

@struan struan requested a review from dracos December 18, 2025 10:06
Copy link
Member

@dracos dracos left a comment

Choose a reason for hiding this comment

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

Woo; still got that superfluous ELSIF cobrand.moniker == 'kingston' in the text email but doesn't matter if it's left in :)

This is mostly based on the Sutton code from #0cc3311cf5 with a few
Kingston specific changes to add checkbox extra details to the bin not
returned, plus a message for FAS properties for spillages.

Fixes #5245
Fixes #5246
@struan struan force-pushed the 5244-kingston-spillages-not-returned branch from e195700 to 02b6e09 Compare December 18, 2025 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants