1166 local plans dataset list deploy to production#1168
Merged
pooleycodes merged 16 commits intomainfrom Apr 16, 2026
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdated dataset configuration names and guidance links; added five plan-related dataset entries and adjusted plan-fallback schema; added Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
config/default.yaml (1)
136-139: Consider adding local-planning-group to default configuration.The
organisationTypeslist in default.yaml does not includelocal-planning-group, which was added to bothconfig/production.yamlandconfig/staging.yaml. This creates an inconsistency where development environments (using default.yaml) won't recognise local-planning-group organisations, whilst production and staging will.Based on the middleware code in
organisations.middleware.js, the SQL query dynamically filters organisations using these types, so local-planning-group entries will be excluded in development. Consider whether development and testing environments should also support this organisation type to maintain consistency and enable proper testing before deployment.♻️ Proposed addition to align with production and staging
organisationTypes: - local-authority - national-park-authority - development-corporation + - local-planning-group🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/default.yaml` around lines 136 - 139, The default configuration's organisationTypes list is missing the local-planning-group entry which causes dev environments to behave differently than staging/production; update the organisationTypes YAML sequence by adding "local-planning-group" to the list so it matches the other environment configs and ensure the middleware that filters by organisationTypes (organisationTypes key used by organisations.middleware.js) will include those organisations during development and testing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@config/default.yaml`:
- Around line 136-139: The default configuration's organisationTypes list is
missing the local-planning-group entry which causes dev environments to behave
differently than staging/production; update the organisationTypes YAML sequence
by adding "local-planning-group" to the list so it matches the other environment
configs and ensure the middleware that filters by organisationTypes
(organisationTypes key used by organisations.middleware.js) will include those
organisations during development and testing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e2df5b72-7594-4e03-a5f2-3d197252ac5e
📒 Files selected for processing (3)
config/default.yamlconfig/production.yamlconfig/staging.yaml
pooleycodes
had a problem deploying
to
development
GitHub Actions
Failure
Ben-Hodgkiss
had a problem deploying
to
development
GitHub Actions
Failure
Ben-Hodgkiss
had a problem deploying
to
development
GitHub Actions
Failure
Ben-Hodgkiss
had a problem deploying
to
development
GitHub Actions
Failure
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
config/plan-fallback.json (1)
100-100: Verify compatibility with external systems before production deployment.The fallback resolver uses exact-name field matching. Whilst the codebase shows consistent use of the new "minerals-and-waste-planning-authorities" field name, ensure any external data stores, integrations, or third-party systems do not reference legacy field names that could silently fail to map.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/plan-fallback.json` at line 100, The fallback resolver currently does exact-name matching for the field "minerals-and-waste-planning-authorities", which may break integrations using legacy names; update the resolver (or its mapping table) to accept aliases by adding known legacy field names as synonyms for "minerals-and-waste-planning-authorities" and/or implement a normalization step (e.g., lowercase, dash/underscore normalization) before matching, and add a short comment and test covering the alias set so external data stores or third-party systems referencing legacy names will still map correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@config/plan-fallback.json`:
- Line 100: The fallback resolver currently does exact-name matching for the
field "minerals-and-waste-planning-authorities", which may break integrations
using legacy names; update the resolver (or its mapping table) to accept aliases
by adding known legacy field names as synonyms for
"minerals-and-waste-planning-authorities" and/or implement a normalization step
(e.g., lowercase, dash/underscore normalization) before matching, and add a
short comment and test covering the alias set so external data stores or
third-party systems referencing legacy names will still map correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e972bb62-225c-4c90-95ae-9be789c0381c
📒 Files selected for processing (1)
config/plan-fallback.json
…, add alternate pre populated notice for local plan
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/datasetteQueries/fetchDatasetsFromProvisions.js (1)
6-6: Clarify the purpose ofLIMIT -1.In SQLite,
LIMIT -1means "no limit" and returns all rows, which is equivalent to omitting theLIMITclause entirely. Is this change intentional to work around a specific datasette behaviour or pagination default? A brief comment explaining the reasoning would help future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/datasetteQueries/fetchDatasetsFromProvisions.js` at line 6, The SQL uses "LIMIT -1" which in SQLite means "no limit"; clarify intent by adding a brief inline comment next to the const sql definition in fetchDatasetsFromProvisions.js (the const named sql with value 'SELECT DISTINCT dataset, provision_reason FROM provision LIMIT -1') stating that LIMIT -1 is used to force no-limit behavior (or to work around Datasette's default pagination) and note whether this is intentional or could be replaced by omitting LIMIT; update the comment to reference Datasette pagination if that was the reason.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/utils/datasetteQueries/fetchDatasetsFromProvisions.js`:
- Line 6: The SQL uses "LIMIT -1" which in SQLite means "no limit"; clarify
intent by adding a brief inline comment next to the const sql definition in
fetchDatasetsFromProvisions.js (the const named sql with value 'SELECT DISTINCT
dataset, provision_reason FROM provision LIMIT -1') stating that LIMIT -1 is
used to force no-limit behavior (or to work around Datasette's default
pagination) and note whether this is intentional or could be replaced by
omitting LIMIT; update the comment to reference Datasette pagination if that was
the reason.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 30a1aee0-eb42-40fb-b8c5-97c3c7d4f122
📒 Files selected for processing (5)
src/utils/datasetteQueries/fetchDatasetCollections.jssrc/utils/datasetteQueries/fetchDatasetsFromProvisions.jssrc/views/components/alternativePrePopulatedSourceNotice.htmlsrc/views/organisations/dataset-overview.htmlsrc/views/organisations/dataview.html
Ben-Hodgkiss
temporarily deployed
to
development
GitHub Actions
Inactive
pooleycodes
had a problem deploying
to
development
GitHub Actions
Failure
Ben-Hodgkiss
previously approved these changes
Apr 16, 2026
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/views/organisations/dataset-overview.html (1)
221-225: Consider centralising this dataset-type predicate.The same
local-plan/plan-timetablecheck now exists in multiple templates. A shared helper/macro would reduce drift when this list changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/organisations/dataset-overview.html` around lines 221 - 225, Extract the repeated predicate (dataset.dataset === "local-plan" or dataset.dataset === "plan-timetable") into a single shared template helper/macro (e.g., isPrepopulatedDataset or is_local_plan_dataset) and replace the inline checks in dataset-overview.html and all other templates with a call to that macro; update the conditional here to use the new macro to choose between calling alternativePrePopulatedSourceNotice(organisation, dataset, downloadUrl, alternateSources) and alternativeSourceNotice(...), and add unit/template tests or comments where the macro is defined so future dataset-type changes are made in one place.src/views/components/alternativePrePopulatedSourceNotice.html (1)
6-12: Render the MHCLG explanatory paragraph at most once.If
alternateSourcescontains duplicate entries for the same source name, this paragraph will repeat. Consider guarding this so it renders once per page.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/components/alternativePrePopulatedSourceNotice.html` around lines 6 - 12, The MHCLG explanatory paragraph can render multiple times if alternateSources contains duplicates; change the template so it renders at most once by detecting whether any source with name "Ministry of Housing, Communities & Local Government" exists and emitting the paragraph only once—for example, replace the per-item conditional inside the {% for source in alternateSources %} loop with a single check over alternateSources (or use a local rendered flag such as mhclgRendered set before/after the loop) that references alternateSources and source.name to ensure the paragraph is output only once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/views/components/alternativePrePopulatedSourceNotice.html`:
- Around line 6-12: The MHCLG explanatory paragraph can render multiple times if
alternateSources contains duplicates; change the template so it renders at most
once by detecting whether any source with name "Ministry of Housing, Communities
& Local Government" exists and emitting the paragraph only once—for example,
replace the per-item conditional inside the {% for source in alternateSources %}
loop with a single check over alternateSources (or use a local rendered flag
such as mhclgRendered set before/after the loop) that references
alternateSources and source.name to ensure the paragraph is output only once.
In `@src/views/organisations/dataset-overview.html`:
- Around line 221-225: Extract the repeated predicate (dataset.dataset ===
"local-plan" or dataset.dataset === "plan-timetable") into a single shared
template helper/macro (e.g., isPrepopulatedDataset or is_local_plan_dataset) and
replace the inline checks in dataset-overview.html and all other templates with
a call to that macro; update the conditional here to use the new macro to choose
between calling alternativePrePopulatedSourceNotice(organisation, dataset,
downloadUrl, alternateSources) and alternativeSourceNotice(...), and add
unit/template tests or comments where the macro is defined so future
dataset-type changes are made in one place.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1526b138-39b5-4b56-a73a-95ebc7abe719
📒 Files selected for processing (7)
config/default.yamlsrc/utils/datasetteQueries/fetchDatasetCollections.jssrc/utils/datasetteQueries/fetchDatasetsFromProvisions.jssrc/views/components/alternativePrePopulatedSourceNotice.htmlsrc/views/organisations/dataset-overview.htmlsrc/views/organisations/dataview.htmltest/unit/views/organisations/lpaOverviewPage.test.js
✅ Files skipped from review due to trivial changes (2)
- src/utils/datasetteQueries/fetchDatasetCollections.js
- config/default.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/datasetteQueries/fetchDatasetsFromProvisions.js
Ben-Hodgkiss
approved these changes
Apr 16, 2026
pooleycodes
deleted the
1166-local-plans-dataset-list---deploy-to-production
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Update the default yaml file to include the hard coded list of datasets to allow plans datasets and timetable ones to show through to production dataset.
Also removed the requirement for a database to already exist for a dataset, now supplementary plan dataset will show on the platform even though it has not database set up in datasette (as no data provided yet)
What type of PR is this? (check all applicable)
Related Tickets & Documents
QA Instructions, Screenshots, Recordings
Before
Before screenshot here
After
After screenshot here
Added/updated tests?
We encourage you to keep the code coverage percentage at 80% and above.
QA sign off
[optional] Are there any post-deployment tasks we need to perform?
[optional] Are there any dependencies on other PRs or Work?
Summary by CodeRabbit
New Features
Chores
Documentation
Tests