Fix(Identifier-Editor): Improve identifier editor behavior for ISBNs #832
Conversation
There was a problem hiding this comment.
Thanks for woking on this !
I dug deeper this time (armed with more knowledge of ISBN peculiarities), and I don't think the ISBN10 validation is working properly
These valid ISBN 10 don't pass validation:
9783257862
0198526636
0306406152
1-250-03775-1 // 1250037751
At this point, instead of digging our own grave, let's revisit this and use an existing library to solve these issues of validation, conversion and formatting.
For example: https://github.com/inventaire/isbn3
I would suggest we do this:
- Keep the commenting out of auto ISBN conversion
- Keep the "don't change the identifier type if it is set"
- Remove the ISBN check digit validation (not sure it's 100% working at the moment)
We'll be back to more or less the previous functionality (with the small improvement of not auto-selecting type if manually selected) which is what we want for a patch.
Then in a separate PR, we can work on improving the functionality using the third-party library, and spend more time testing everything instead of putting out fires :)
| @@ -43,7 +43,8 @@ function reducer( | |||
| const updatedValue = state.setIn( | |||
| [payload.rowId, 'value'], payload.value | |||
| ); | |||
| if (payload.suggestedType) { | |||
| // don't switch type if user already selected it | |||
| if (payload.suggestedType && !state.getIn([payload.rowId, 'type'])) { | |||
There was a problem hiding this comment.
❤️ CatQuest is going to be very happy about this one !
In this PR, I am making ISBN detection more strict by verifying its check digit, disabling switching type after user manually selected it and disabling new ISBN row addition(temporary).