Skip to content

Conversation

@Akol125
Copy link
Contributor

@Akol125 Akol125 commented Sep 24, 2025

Summary

  • Routine Change
  • ❗ Breaking Change
  • 🤖 Operational or Infrastructure Change
  • ✨ New Feature
  • ⚠️ Potential issues that might be caused by this change

Add any other relevant notes or explanations here. Remove this line if you have nothing to add.

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-307

"2100-01-01", # Year in future
"2050-12-31", # Year in future
"2029-06-15", # Year in future
(datetime.now() + timedelta(days=random.randint(1, 30))).strftime("%Y-%m-%d")
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for doing this. I would question whether we need to do random and 3 different points.

Maybe just some boundary tests e.g. 1 day ahead and 1 or 2 years ahead would be sufficient.

Optional - or maybe you already did it, but could be nice to test that the same day is successful i.e. it is not flagged as a future date. maybe that test was implemented somewhere else though. And happy for it to be left out too.

now = datetime.now()
sample_inputs = [
now + timedelta(days=1),
now + timedelta(days=365),
Copy link
Contributor

Choose a reason for hiding this comment

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

Good set of test cases. Always preferred to mock datetime/now but in place of this, the approach used is fine.

)
return json_data

def format_future_dates(dates: List[Union[date, datetime]], mode: str = "auto") -> List[str]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't think this helper function has anything to do with the future. Is just a helped to format a some datetime or date strings.

dlzhry2nhs
dlzhry2nhs previously approved these changes Sep 25, 2025
@Akol125 Akol125 enabled auto-merge (squash) September 25, 2025 15:46
@sonarqubecloud
Copy link

@Akol125 Akol125 merged commit c4986ba into master Sep 25, 2025
7 checks passed
@Akol125 Akol125 deleted the VED-307-Autogenerate-Futuredates branch September 25, 2025 15:55
Akol125 added a commit that referenced this pull request Nov 7, 2025
* autogenerated date for test and remove PII logs
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