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

Added isFreightContainerID validator #2144

Merged
merged 7 commits into from
Feb 8, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ Validator | Description
**isBoolean(str [, options])** | check if a string is a boolean.<br/>`options` is an object which defaults to `{ loose: false }`. If loose is is set to false, the validator will strictly match ['true', 'false', '0', '1']. If loose is set to true, the validator will also match 'yes', 'no', and will match a valid boolean string of any case. (eg: ['true', 'True', 'TRUE']).
**isBtcAddress(str)** | check if the string is a valid BTC address.
**isByteLength(str [, options])** | check if the string's length (in UTF-8 bytes) falls in a range.<br/><br/>`options` is an object which defaults to `{min:0, max: undefined}`.
**isContainerID(str)** | check if the string is an ISO6346 shipping container identification.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the name might be a bit too unspecific, as "container ID" feels a bit too generic, e.g. just google it, brought up several hits, like e.g. "docker container ID", "Windows Drivers Container ID", etc.

So I would probably say it makes sense to rename this?

Options could be maybe:

  • isShippingContainerID
  • isISO6346ContainerID (that one does look ugly to be honest :-D)
  • ?

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

We already have some ISO standards so a simple isISO6346 could also suffice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have changed it to isISO6346, for consistency with other validators.

**isCreditCard(card, [, options])** | check if the string is a credit card.<br/><br/> options is an optional object that can be supplied with the following key(s): `provider` is an optional key whose value should be a string, and defines the company issuing the credit card. Valid values include `amex` , `dinersclub` , `discover` , `jcb` , `mastercard` , `unionpay` , `visa` or blank will check for any provider.
**isCurrency(str [, options])** | check if the string is a valid currency amount.<br/><br/>`options` is an object which defaults to `{symbol: '$', require_symbol: false, allow_space_after_symbol: false, symbol_after_digits: false, allow_negatives: true, parens_for_negatives: false, negative_sign_before_digits: false, negative_sign_after_digits: false, allow_negative_sign_placeholder: false, thousands_separator: ',', decimal_separator: '.', allow_decimal: true, require_decimal: false, digits_after_decimal: [2], allow_space_after_digits: false}`.<br/>**Note:** The array `digits_after_decimal` is filled with the exact number of digits allowed not a range, for example a range 1 to 3 will be given as [1, 2, 3].
**isDataURI(str)** | check if the string is a [data uri format](https://developer.mozilla.org/en-US/docs/Web/HTTP/data_URIs).
Expand Down
2 changes: 2 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ import isISIN from './lib/isISIN';
import isISBN from './lib/isISBN';
import isISSN from './lib/isISSN';
import isTaxID from './lib/isTaxID';
import isContainerID from './lib/isContainerID';

import isMobilePhone, { locales as isMobilePhoneLocales } from './lib/isMobilePhone';

Expand Down Expand Up @@ -195,6 +196,7 @@ const validator = {
isMobilePhoneLocales,
isPostalCode,
isPostalCodeLocales,
isContainerID,
isEthereumAddress,
isCurrency,
isBtcAddress,
Expand Down
28 changes: 28 additions & 0 deletions src/lib/isContainerID.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import assertString from './util/assertString';

// https://en.wikipedia.org/wiki/ISO_6346
const isContainerIdReg = new RegExp('^[A-Z]{3}U[0-9]{7}$');
const isDigit = new RegExp('^[0-9]{1}');
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this could be rewritten to ^[0-9]$, since you are checking against each string character using its index, and that will always be just one character

Also maybe just a matter of taste in general, but this project seems to be generally prefering the use of regular expression literals. as opposed to a constructor function, like you used here.
So for the sake of style consistency, I would probably also suggest the following change:

new RegExp('^[0-9]{1}')-> const isDigit = /^[0-9]$/
(same applies to the isContainerIdReg regexp in line 4)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi Panagiotis,

Thank you very much for pointing it out! I have updated the corresponding lines.


export default function isContainerID(str) {
assertString(str);
str = str.trim();
Copy link
Contributor

Choose a reason for hiding this comment

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

I have the same concerns about this, as I have mentioned in your other PR:
#2143

what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

You are right @pano9000 -- I don't think we should be doing cleaning behind the validators, let the users do their own cleaning. @songyuew pls update.


if (!isContainerIdReg.test(str)) return false;

let sum = 0;
for (let i = 0; i < str.length - 1; i++) {
if (!isDigit.test(str[i])) {
let convertedCode;
const letterCode = str.charCodeAt(i) - 55;
if (letterCode < 11) convertedCode = letterCode;
else if (letterCode >= 11 && letterCode <= 20) convertedCode = 12 + (letterCode % 11);
else if (letterCode >= 21 && letterCode <= 30) convertedCode = 23 + (letterCode % 21);
else convertedCode = 34 + (letterCode % 31);
sum += convertedCode * (2 ** i);
} else sum += str[i] * (2 ** i);
}

const checkSumDigit = sum % 11;
return Number(str[str.length - 1]) === checkSumDigit;
}
25 changes: 25 additions & 0 deletions test/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -11837,6 +11837,31 @@ describe('Validators', () => {
});
});

it('should validate containerID', () => {
test({
validator: 'isContainerID',
valid: [
'HLXU2008419',
'TGHU7599330',
'ECMU4657496',
'MEDU6246078',
'YMLU2809976',
'MRKU0046221',
'EMCU3811879',
'OOLU8643084',
'HJCU1922713',
],
invalid: [
'OOLU1922713',
'HJCU1922413',
'FCUI985619',
'ECMJ4657496',
'TBJA7176445',
'AFFU5962593',
],
});
});

// EU-UK valid numbers sourced from https://ec.europa.eu/taxation_customs/tin/specs/FS-TIN%20Algorithms-Public.docx or constructed by @tplessas.
it('should validate taxID', () => {
test({
Expand Down