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.

Sign up for GitHub

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

Jump to bottom

Bug: edtf date validation problems #1533

Closed
patdunlavey opened this issue Jun 2, 2020 · 1 comment
Closed

Bug: edtf date validation problems #1533

patdunlavey opened this issue Jun 2, 2020 · 1 comment

Comments

@patdunlavey
Copy link

A few things in EDTFUtils::validate():

  1. Line 140: The regex pattern for a date set should not have the char lists internally separated by commas. Valid multiple date set strings are not matched.
  2. Line 140: I think the string [] should not be considered a properly encoded date string, so I think the date part match group quantity should be changed from * to +.
  3. Line 141: When there is a match, $match[1] and $match[2] will never equal each other. If $has_match is false, that's sufficient. And, we should return at this point, there's no reason to continue testing each date in the set.
  4. Line 159: !$date === '..' will never be true. Put the negation outside the expression by using parentheses.

PR coming...
patdunlavey added a commit to Born-Digital-US/controlled_access_terms that referenced this issue Jun 2, 2020
@patdunlavey
issues/1533-EDTFUtils-bugs --- see Islandora/documentation#1533
89323ba 
- removed extraneous commas from date set regex pattern
- require at least one date string
- failure to match exits, rather than continuing to parse dates
- fixed bug in  date part matching
@patdunlavey patdunlavey mentioned this issue Jun 2, 2020
Merged
seth-shaw-unlv pushed a commit to Islandora/controlled_access_terms that referenced this issue Jun 3, 2020
@patdunlavey
issues/1533-EDTFUtils-bugs --- see Islandora/documentation#1533 (#52)
c19f743 
- removed extraneous commas from date set regex pattern
- require at least one date string
- failure to match exits, rather than continuing to parse dates
- fixed bug in  date part matching

Co-authored-by: Pat Dunlavey <[email protected]>
@seth-shaw-unlv
Copy link
Contributor

Resolved with Islandora/controlled_access_terms@c19f743.

However, this reminds me that we really need to add some unit tests for validation, if nothing else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@patdunlavey @seth-shaw-unlv