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

Numeric characters in relation codes for 'typed relation' field result in Workbench validation error #701

Closed
eldonquijote opened this issue Oct 23, 2023 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@eldonquijote
Copy link

if not re.match("^[a-zA-Z]+:[a-zA-Z]+:.+$", field_value.strip()):

assumes that the relationship code for typed relation fields contains all letters (as is the case with MARC relator codes). We are using AAT terms to describe roles/relationships in addition to MARC relator codes; the AAT term identifiers are numbers though. Workbench throws a validation error for an entry in an import CSV like aat:300024987:person:Wees, J.L., even if the relation is listed in the field configuration (aat:300024987|architect (aat)). Ideally, Workbench should accept/validate alphanumeric values as configured.

@mjordan mjordan self-assigned this Oct 23, 2023
@mjordan mjordan added the bug Something isn't working label Oct 23, 2023
@mjordan
Copy link
Owner

mjordan commented Oct 23, 2023

Can you check out the issue_701 branch and test?

@eldonquijote
Copy link
Author

if not re.match("^[09-0a-zA-Z]+:[0-9a-zA-Z]+:.+$", field_value.strip()):
gives me a regex parsing error:
if not re.match("^[09-0a-zA-Z]+:[0-9a-zA-Z]+:.+$", field_value.strip()):
should probably be
if not re.match("^[0-9a-zA-Z]+:[0-9a-zA-Z]+:.+$", field_value.strip()):

@eldonquijote
Copy link
Author

(Seems to work fine if I correct the typo locally.)

@mjordan
Copy link
Owner

mjordan commented Oct 23, 2023

Duh, thanks, I'll fix that.

mjordan added a commit that referenced this issue Oct 23, 2023
@mjordan
Copy link
Owner

mjordan commented Oct 23, 2023

OK, typo fixed, can you retest?

@eldonquijote
Copy link
Author

Works great, thank you very much!

@mjordan
Copy link
Owner

mjordan commented Oct 23, 2023

OK, great. I'm going to do a little testing this evening to make sure the change has no side effects, and if it doesn't (I don't think it will), I'll merge it into the main branch. Thanks for finding this bug.

@mjordan
Copy link
Owner

mjordan commented Oct 23, 2023

Issue_701 branch merged into main. Thanks again @eldonquijote for finding this and for testing the fix.

@mjordan mjordan closed this as completed Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants