Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Conflict Resolver] remove date to array conversion #2885

Merged

Conversation

davidblader
Copy link
Contributor

@davidblader davidblader commented Jun 15, 2017

converting date string to array causes Utility::calculateAge to throw the following error
screenshot from 2017-06-15 12-55-14
when attempting to resolve conflicts for the Date_taken field

https://redmine.cbrain.mcgill.ca/issues/12659

@davidblader davidblader added the Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label) label Jun 15, 2017
@davidblader davidblader added this to the 17.0 milestone Jun 15, 2017
@davidblader davidblader added this to Needs Review in Code review 18.1 Jun 15, 2017
Copy link
Contributor

@johnsaigle johnsaigle left a comment

Choose a reason for hiding this comment

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

Looks fine. Is this meant to go to 17.0 though? The related task on Redmine has a milestone of 17.1

@johnsaigle johnsaigle moved this from Needs Review to Needs Testing in Code review 18.1 Jun 15, 2017
@davidblader
Copy link
Contributor Author

davidblader commented Jun 15, 2017

Looks fine. Is this meant to go to 17.0 though?

I think CCNA would definitely prefer to have this on the sooner side; I'll put the needs discussion label on it
EDIT: never mind, the 17.1 task is for preventing conflict resolution in the event of an error. this is more of a quick fix for an error our users have been reporting

@davidblader davidblader added the Discussion Required PR or issue awaiting the resolution of a discussion between all involved parties label Jun 15, 2017
@ridz1208
Copy link
Collaborator

@johnsaigle @davidblader this is definitely 17.0 !! IBIS is overriding CCNA will be doing so as well so we need the fix asap

@ridz1208 ridz1208 removed the Discussion Required PR or issue awaiting the resolution of a discussion between all involved parties label Jun 15, 2017
@sruthymathew123
Copy link
Contributor

@davidblader @mohadesz Tested a couple of date conflicts in conflict resolver. The date conflicts can be saved/resolved.

@sruthymathew123 sruthymathew123 added the Passed Manual Tests PR has undergone proper testing by at least one peer label Jun 16, 2017
@johnsaigle johnsaigle moved this from Needs Testing to Needs Senior Developer Review And Senior Developer Should Merge If Everything Is Fine So There's No Further States in Code review 18.1 Jun 16, 2017
@driusan driusan merged commit f465817 into aces:17.0-dev Jun 16, 2017
@driusan driusan removed this from Needs Senior Developer Review And Senior Developer Should Merge If Everything Is Fine So There's No Further States in Code review 18.1 Jun 16, 2017
jstirling91 pushed a commit to jstirling91/Loris that referenced this pull request Jun 21, 2017
Fix bug where conflict resolver was using an old quickform style date-to-array conversion, instead of directly using the HTML5 date which is passed to LORIS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label) 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.

None yet

5 participants