Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[candidate] Allow dob format Y-m #8648

Merged

Conversation

CamilleBeau
Copy link
Contributor

Brief summary of changes

When a dob Format of Ym (year and month without day) is set in the config module, candidates could not be created for 2 reasons.

  1. The wrong format (!Y-m-d) was being passed through the function DateTime::createFromFormat in the createNew function of the candidate class, which gave a wrong format error to the user even if they were passing through the expected format of Year-month
  2. Even if this was fixed, the date was inserted into the candidate table as YYYY-MM, but this is not compatible with the SQL Type date

This PR fixes this issue by getting the date format from config, processing it to the right format and passing it through to DateTime::createFromFormat, as well as adding a '-01' to the end of the date if Ym format is selected in the configuration. This inserts the first of the month as the arbitrary date for a candidate.

  • Have you updated related documentation?

Would someone be able to help me out as to whether this should be added to the changelog? I'm not sure if this is a bug introduced in 25 but I would be surprised if it existed in previous versions since it is so critical.

Testing instructions (if applicable)

  1. Attempt to create a candidate with both 'Ymd' and 'Ym' selected as dob format in the configuration module.

Link(s) to related issue(s)

@CamilleBeau CamilleBeau changed the base branch from main to 25.0-release April 19, 2023 15:52
@cmadjar cmadjar added the Critical to release PR or issue is key for the release to which it has been assigned label Apr 25, 2023
@racostas racostas self-requested a review April 26, 2023 14:45
Copy link
Contributor

@racostas racostas left a comment

Choose a reason for hiding this comment

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

It is working.
Have some components of hack.
Not sure what will be doing in the case we have to show at some point this DoB and we override the day to the first of each month.
I don't see any other "easy solution"
I'm approving it.
I let the final judge to @driusan

@racostas racostas added the Passed Manual Tests PR has undergone proper testing by at least one peer label Apr 26, 2023
@driusan
Copy link
Collaborator

driusan commented Apr 26, 2023

For display, we can update any places that display it to honour the config if they're missing.

But I'm not sure if it should be the first or middle (15th) of the month, I think different projects use different days. Added something to the LORIS agenda to discuss/decide.

@driusan driusan merged commit 80bfc9c into aces:25.0-release May 11, 2023
@ridz1208 ridz1208 added this to the 25.0.0 milestone Jun 20, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
25.0.0 - Bugs Critical to release PR or issue is key for the release to which it has been assigned Passed Manual Tests PR has undergone proper testing by at least one peer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to create candidate because the date field doesn't allow inclusion of day of the month
5 participants