-
-
Notifications
You must be signed in to change notification settings - Fork 236
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
Initial notts police setup. #4987
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## notts-police-reviewed #4987 +/- ##
==========================================================
- Coverage 82.60% 69.02% -13.59%
==========================================================
Files 393 42 -351
Lines 30752 5085 -25667
Branches 4877 0 -4877
==========================================================
- Hits 25404 3510 -21894
+ Misses 3896 1575 -2321
+ Partials 1452 0 -1452 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good start, just one little thing about areas covered
|
||
=cut | ||
|
||
sub council_area_id { 2236 } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a hole for Nottingham City in the County area, so to include that I think you'd need:
sub council_area_id { 2236 } | |
sub council_area_id { [ 2236, 2565 ] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah wow, I'd always assumed county areas would be a superset but this makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, is this why the example 'NG2 3DZ' postcode was not returning results for me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No that's something that needs to be be set-up in the server configuration under example_places
- you can see where I've done that on the notts-police-staging-setup
branch.
Do we want the parent of the cobrand to be UKCouncils, is that needed as it's a totally standalone cobrand? I imagine there are plus/minus points either way - e.g. see HighwaysEngland/Thamesmead for the functions that had to be copied from UKCouncils (that code suggests a role) so that's awkward, but the parent has unnecessary stuff - e.g. the problem/user restrictions presumably aren't needed, nor all the national highways/munging etc bits that only really apply to cobrands. Just wanted to check this had been considered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving this and merging into notts-police-reviewed
.
Some fixes need to be made as per review on #4996, but will be made on top of notts-police-reviewed
.
-Added Notts Police Branding -Removed FMS footer -Added .site-message styling -Added favicon -Added Emails
af237d2
to
7b95b92
Compare
Initial cobrand setup.
Also closes https://github.com/mysociety/societyworks/issues/4340.