Skip to content

VED-850 Redundant code#956

Merged
JamesW1-NHS merged 7 commits intomasterfrom
VED-850-redundant-code
Nov 3, 2025
Merged

VED-850 Redundant code#956
JamesW1-NHS merged 7 commits intomasterfrom
VED-850-redundant-code

Conversation

@JamesW1-NHS
Copy link
Contributor

@JamesW1-NHS JamesW1-NHS commented Oct 30, 2025

Summary

  • Routine Change

Small piece of code cleaning:

  • models.failures.CsvImmunisationErrorModel doesn't appear to be used anywhere in the code at all. We can remove it.
  • UpdateOutcome: the only value of this which is ever returned is UPDATE. We can remove it as well.

Note for testers: No functionality has changed here. Smoke testing only.

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.

@github-actions
Copy link
Contributor

This branch is working on a ticket in the NHS England VED JIRA Project. Here's a handy link to the ticket:

VED-850

patient = self._validate_patient(immunization)
if "diagnostics" in patient:
return None, patient, None
return False, patient, None
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should not be looking for 'diagnostics' in patient. This is all legacy code from when we used to interact with PDS. I have removed it from the create journey and will be doing so for the in-flight ticket where we do the full refactoring of the update service.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot. In fact that means I can remove the 'outcome' element from the method return entirely, and all the tests which check it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After discussion, I won't remove the check for 'diagnostics' nor the 'outcome' element yet in this ticket. It should be resolved in 747.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 3, 2025

@JamesW1-NHS JamesW1-NHS temporarily deployed to internal-dev-sandbox November 3, 2025 17:37 — with GitHub Actions Inactive
@JamesW1-NHS JamesW1-NHS temporarily deployed to internal-dev-sandbox November 3, 2025 17:37 — with GitHub Actions Inactive
@JamesW1-NHS JamesW1-NHS merged commit e3807bd into master Nov 3, 2025
19 of 20 checks passed
@JamesW1-NHS JamesW1-NHS deleted the VED-850-redundant-code branch November 3, 2025 17:45
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