Skip to content

Hotfix/ved 754 processor task cp1252 encoding#796

Merged
nhsdevws merged 20 commits intorelease-2025-09-04from
hotfix/VED-754-processor-task-cp1252-encoding
Sep 10, 2025
Merged

Hotfix/ved 754 processor task cp1252 encoding#796
nhsdevws merged 20 commits intorelease-2025-09-04from
hotfix/VED-754-processor-task-cp1252-encoding

Conversation

@nhsdevws
Copy link
Contributor

@nhsdevws nhsdevws commented Sep 6, 2025

Summary

  • Hotfix Change

Addresses filename processor task - row processing encountering chars not utf-8.

Resolves:

  • exception thrown if issue detected at header validation (small files)
  • exception thrown if issue detected during iteration of DictReader (large files)

Note:

  • small file tested with 3 rows. Large file tested with 1500 rows
  • sonarcloud doesnt run unit tests. Unit tests pass run locally make test
  • Changes only affect cp1252 files. utf-8 files run unhindered.

Reviews Required

  • Dev
  • Test
  • Tech Author
  • Product Owner

Review Checklist

ℹ️ This section is to be filled in by the reviewer.

  • I have reviewed the changes in this PR and they fill all or part of the acceptance criteria of the ticket, and the code is in a mergeable state.
  • If there were infrastructure, operational, or build changes, I have made sure there is sufficient evidence that the changes will work.
  • I have ensured the changelog has been updated by the submitter, if necessary.

@nhsdevws nhsdevws marked this pull request as ready for review September 6, 2025 11:30
@nhsdevws nhsdevws changed the title Hotfix/ved 754 processor task cp1252 encoding Hotfix/ved 754 processor task cp1252 encoding - Work In Progress Sep 6, 2025
@nhsdevws nhsdevws force-pushed the hotfix/VED-754-processor-task-cp1252-encoding branch 2 times, most recently from 8811cf1 to 2c67fc9 Compare September 9, 2025 10:02
@nhsdevws nhsdevws changed the title Hotfix/ved 754 processor task cp1252 encoding - Work In Progress Hotfix/ved 754 processor task cp1252 encoding Sep 9, 2025
@nhsdevws nhsdevws force-pushed the hotfix/VED-754-processor-task-cp1252-encoding branch 2 times, most recently from cb31157 to 3f4e27e Compare September 10, 2025 08:10
@nhsdevws nhsdevws force-pushed the hotfix/VED-754-processor-task-cp1252-encoding branch from b9f6067 to 5a780f0 Compare September 10, 2025 08:12
Copy link
Collaborator

@dlzhry2nhs dlzhry2nhs left a comment

Choose a reason for hiding this comment

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

Thanks so much for doing this. It will avoid us reading the file for validation and is a smarter solution.

I've added a handful of comments. The only things I think I'd really like to see would be handling for the specific exception and a comment to explain the rationale for the change more clearly.

@nhsdevws nhsdevws force-pushed the hotfix/VED-754-processor-task-cp1252-encoding branch from 105e043 to 772d52f Compare September 10, 2025 09:41
@dlzhry2nhs dlzhry2nhs self-requested a review September 10, 2025 10:37
Copy link
Collaborator

@dlzhry2nhs dlzhry2nhs left a comment

Choose a reason for hiding this comment

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

Changing my remaining comments to advisory rather than Request changes so it does not block if someone approves over lunch.

Thanks for resolving so quickly.

Would be good to get a quick change in for the remaining 3 points.

Copy link
Collaborator

@dlzhry2nhs dlzhry2nhs left a comment

Choose a reason for hiding this comment

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

Awesome work. Thanks for your patience and sorting them all out so quickly. As discussed offline there is a failing test locally (shame release branch does not have the pipeline fixes).

It's gonna be fixed, so approving now.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@nhsdevws nhsdevws merged commit 7c5a363 into release-2025-09-04 Sep 10, 2025
7 of 8 checks passed
@nhsdevws nhsdevws deleted the hotfix/VED-754-processor-task-cp1252-encoding branch September 10, 2025 14:27
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.

2 participants