Merged
Conversation
…ation cookie sess remove, organisation name from dataset removal
4 tasks
Contributor
WalkthroughThis PR surfaces alternate data sources: it extends authority middleware to populate alternateSources when authority is "some", filters system fields from specifications, threads alternateSources/uniqueDatasetFields through middleware to views and schemas, updates templates and download URL handling, adds a HEAD→GET fallback for URL checks, and expands the README Jira local-setup steps. Changes
Sequence Diagram(s)sequenceDiagram
participant Req as Request Handler
participant Auth as prepareAuthority
participant Platform as Platform API
participant FetchAlt as fetchAlternateSources
participant DB as Database
participant Filter as filterOutSystemFields
participant Render as Template Renderer
Req->>Auth: determine authority for dataset
Auth->>Platform: fetch entities (quality: 'some')
Platform-->>Auth: return entities
Auth->>Req: set req.authority and req.alternateEntityList
alt req.authority == 'some'
Req->>FetchAlt: fetch alternateSources (entity list)
FetchAlt->>DB: query distinct organisation names (exclude current)
DB-->>FetchAlt: organisation names
FetchAlt->>Req: set req.alternateSources
end
Req->>Filter: remove system fields from req.specification
Filter-->>Req: spec without system fields
Req->>Render: render dataset/dataview with alternateSources and fields
Render-->>Req: response HTML
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
pooleycodes
temporarily deployed
to
development
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
readme.md (1)
182-189: Clarify redundant project creation steps.Steps 2 and 5 both mention creating a project:
- Step 2: "Create a 'Customer Desk' Project"
- Step 5: "create a new project"
If these refer to the same project, remove the duplication. If they're different projects, please clarify the distinction and purpose of each.
🤖 Fix all issues with AI agents
In `@readme.md`:
- Line 181: Update the single-line caution "Do not do this under a communities
account" to briefly define what a "communities account" is and state the risk:
e.g., explain it refers to free/shared/community-tier Atlassian accounts with
restricted permissions and potentially no support or billing separation, and
instruct readers to use a paid or org-managed account instead; modify the
sentence near the step text "Go to [JIRA Service Desk]..." so it reads something
like "Do not use a Communities (free/shared) account — use an org-managed or
paid Atlassian account to ensure proper permissions, support, and billing
isolation."
In `@src/middleware/common.middleware.js`:
- Around line 237-257: The generated SQL in fetchAlternateSources can produce an
invalid IN () when entityIds is empty; update the query builder in
fetchAlternateSources (used by fetchMany) to short-circuit: detect when
entityIds.length === 0 and return a safe no-row SQL (e.g., a SELECT that always
yields no rows such as SELECT DISTINCT o.name FROM lookup l LEFT JOIN
organisation o ON l.organisation = o.organisation WHERE 1=0) instead of
constructing WHERE l.entity IN (); keep the existing filtering on
req.orgInfo.organisation when returning the no-row query so the result key
alternateSources remains consistent.
- Around line 208-218: In prepareAuthority middleware update the
platformApi.fetchEntities call (where quality: 'some') to include an explicit
limit parameter (e.g., limit: 1000 or another appropriate cap) so
req.alternateEntityList (populated from someResult.formattedData) contains a
complete set; then in fetchAlternateSources guard against empty entityIds before
building the SQL IN clause (return early or skip the query when entityIds.length
=== 0) to avoid generating invalid SQL IN () syntax.
In `@src/views/check/error-redirect.html`:
- Around line 55-57: Two consecutive <p> elements in the error branch for "The
URL must be accessible" both use the same id "bad-upload", violating unique ID
rules and breaking the error-summary focus; update the template so only the
first paragraph retains id="bad-upload" (and its tabindex="-1") and remove the
id attribute from the second paragraph text about contacting IT, leaving the
copy unchanged otherwise to match the other error branches.
In `@src/views/check/results/results.html`:
- Around line 46-49: The datasetBanner call in results.html currently passes
options.requestParams.dataset without a fallback; update the call to use the
same fallback pattern used elsewhere (options.datasetName or
options.requestParams.dataset) so it becomes
datasetBanner(options.requestParams.organisationName, options.datasetName or
options.requestParams.dataset) — locate the datasetBanner invocation in
results.html and modify its second argument accordingly.
In `@src/views/components/alternativeSourceNotice.html`:
- Around line 3-21: The template macro alternativeSourceNotice currently
compares source.name to a hard-coded string with "&" which doesn't match the
canonical "and" used elsewhere; update the equality check on source.name inside
the loop to use the canonical spelling "Ministry of Housing, Communities and
Local Government" or (preferably) normalise the comparison (e.g., lower-case and
replace '&' with 'and' or trim punctuation) so that the condition in the {% if
source.name == ... %} reliably matches both variants and the notice renders
correctly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/controllers/submitUrlController.js`:
- Around line 139-156: When falling back to GET with Range: bytes=0-0 in
SubmitUrlController, the GET response's content-length may reflect only the
requested byte range; before returning getResponse in the GET-fallback branch in
submitUrlController.js, parse the Content-Range header (e.g. "bytes 0-0/12345")
to extract the total file size and overwrite
getResponse.headers['content-length'] with that total size so
urlResponseIsNotTooLarge() sees the real file size; if Content-Range is missing
or unparsable, fall back to the original headers but prefer the parsed total
when available.
- Around line 148-169: The GET fallback success condition in
SubmitUrlController.headRequest() currently treats only statuses <400 and 416 as
acceptable, so a GET 404 falls through and returns the original HEAD 403; update
the condition that checks getResponse.status to also consider 404 as a valid
returned response (i.e., include getResponse.status === 404 alongside the
existing checks) so the function returns getResponse (the GET response) when GET
yields 404 and the isUrlAccessible validator receives the correct 404 status.
🧹 Nitpick comments (1)
src/controllers/submitUrlController.js (1)
66-67: Track this TODO as a work item.Leaving a TODO here risks it lingering indefinitely; please convert it into a tracked issue or remove the dead code path when possible.
pooleycodes
temporarily deployed
to
development
GitHub Actions
Inactive
pooleycodes
temporarily deployed
to
development
GitHub Actions
Inactive
eveleighoj
approved these changes
Jan 28, 2026
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
2 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
What type of PR is this? (check all applicable)
Related Tickets & Documents
QA Instructions, Screenshots, Recordings
New Alternate Content Display:
Added/updated tests?
We encourage you to keep the code coverage percentage at 80% and above.
Summary by CodeRabbit
Documentation
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.