Skip to content

fix(utils): correct bank number validation #917

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix(utils): correct bank number validation #917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
15e81b9
my first commit
devmooo3 Jul 21, 2025
73c5ee4
impl(utils): add isValidBankNumber function
devmooo3 Jul 24, 2025
079f1e1
fix(utils): correct bank number validation
devmooo3 Jul 24, 2025
059b72c
Remove additional error
devmooo3 Jul 24, 2025
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
25 changes: 24 additions & 1 deletion packages/utils/src/validation/invoice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,29 @@ export function isValidBankname(
export function isValidBankNumber(
bankNumber: string
): FieldError.InvalidBankAccountNumber | true {
if (isNaN(Number(bankNumber))) return FieldError.InvalidBankAccountNumber;
if (isNaN(Number(bankNumber))) {
return FieldError.InvalidBankAccountNumber;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

The length of bank numbers varies between 9 digits to 12. We may use that as well before commencing on the luhn algorithm.

const isLuhnValid =
bankNumber
.split("")
.reverse()
.map((char, index) => {
const digit = Number(char);
return index % 2 === 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefer writing declarative code; use isEven or isOdd functions.

? digit * 2 > 9
? digit * 2 - 9
: digit * 2
: digit;
Comment on lines +102 to +105
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is unreadable at all. It's logic is correct 100%, but it's hard to read. Try using if statements instead.
Moreover, try to write the algorithm yourself first:

Algorithm isValidBankAccount:
   Input: bankNumber - string of the bank number
   Output: true or false
   
   sum <- 0
   for each digit in the bank number do the following
       d <- the digit
       if the digit is an even number
           sum <- sum + d
           continue
       ...

Remember, you will never learn by allotting everything to the AI.

You way manna write the function in an empty JS file in order to be able to experiment and execute it easily with node.

})
.reduce((sum, digit) => sum + digit, 0) %
10 ===
0;
Comment on lines +107 to +109
Copy link
Collaborator

Choose a reason for hiding this comment

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

This looks so ugly.


if (!isLuhnValid) {
return FieldError.InvalidBankAccountNumber;
}

return true;
}
24 changes: 24 additions & 0 deletions packages/utils/tests/isValidBankNumber.test.ts
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

This's not how we write unit tests. Use AI as a tool only. You are the coder; read other unit tests files and follow its pattern or so to speak our implicit convention of writing tests.

the unit tests should be added in the invoice.test.ts file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for the note🫡

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { isValidBankNumber } from "@/validation/invoice";
import {

Check failure on line 2 in packages/utils/tests/isValidBankNumber.test.ts

View workflow job for this annotation

GitHub Actions / checks 

'./luhnGenerator' import is restricted from being used by a pattern. Relative imports are not allowed
generateValidLuhnNumber,
generateInvalidLuhnNumber,
} from "./luhnGenerator";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Relative imports are not recommended.

import { FieldError } from "@litespace/types";

describe("isValidBankNumber - Generated numbers", () => {
const validNumbers = Array.from({ length: 100 }, () =>
generateValidLuhnNumber()
);

const invalidNumbers = Array.from({ length: 100 }, () =>
generateInvalidLuhnNumber()
);

test.each(validNumbers)("✅ Valid: %s", (number) => {
expect(isValidBankNumber(number)).toBe(true);
});

test.each(invalidNumbers)("❌ Invalid: %s", (number) => {
expect(isValidBankNumber(number)).toBe(FieldError.InvalidBankAccountNumber);
});
});
28 changes: 28 additions & 0 deletions packages/utils/tests/luhnGenerator.ts
View file
Open in desktop
Copy link
Collaborator

Choose a reason for hiding this comment

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

First of all, only test files, like {entity}.test.ts, are stored in the tests directory. This shouldn't be store here. You may write these functions within the file that uses them. Second, I don't believe this is the best approach. I prefer using just multiple valid and invalid bank account numbers.

We may use the ones being used by check50:
https://github.com/cs50/problems/blob/2025/x/credit/__init__.py

Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Generate Valid Luhn Number Function
export function generateValidLuhnNumber(length = 16): string {
const digits = Array.from({ length: length - 1 }, () =>
Math.floor(Math.random() * 10)
);

const sum = digits
.slice()
.reverse()
.map((digit, idx) =>
idx % 2 === 0 ? (digit * 2 > 9 ? digit * 2 - 9 : digit * 2) : digit
)
.reduce((acc, val) => acc + val, 0);

const checkDigit = (10 - (sum % 10)) % 10;
digits.push(checkDigit);

return digits.join("");
}

// Generate Invalid Luhn Number Function
export function generateInvalidLuhnNumber(length = 16): string {
const valid = generateValidLuhnNumber(length);
const index = Math.floor(Math.random() * (length - 1));
const wrongDigit = (Number(valid[index]) + 1) % 10;

return valid.slice(0, index) + wrongDigit + valid.slice(index + 1);
}
Loading