-
Notifications
You must be signed in to change notification settings - Fork 12
Fix monetary custom field UX: clarify currency format and catch trailing-symbol mistakes early #57
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
Changes from 6 commits
bd2ca4d
e417f19
5224c05
24b5182
7bb0c4f
eff830c
72eec16
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@baruchiro/paperless-mcp": patch | ||
| --- | ||
|
|
||
| Improve UX for monetary custom fields: clarify currency format in tool descriptions, and add client-side validation that catches common mistakes (e.g., trailing `$` like `10.00$`) with actionable error messages suggesting the correct format (e.g., `USD10.00`). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| name: CI | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| pull_request: | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - name: Setup Node.js | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version-file: ".node-version" | ||
| - name: Install dependencies | ||
| run: npm ci | ||
| - name: Run tests | ||
| run: npm test |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| export const CUSTOM_FIELD_VALUE_DESCRIPTION = | ||
| "The value for the custom field. For monetary fields, use currency code prefix format (e.g., USD10.00, GBP123.45, EUR9.99) — NOT trailing symbol format (e.g., 10.00$). For documentlink fields, use a single document ID (e.g., 123) or an array of document IDs (e.g., [123, 456])."; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import { test } from "node:test"; | ||
| import assert from "node:assert/strict"; | ||
| import { getMonetaryValidationError } from "./monetary"; | ||
|
|
||
| test("returns null for non-monetary strings", () => { | ||
| assert.equal(getMonetaryValidationError("hello"), null); | ||
| assert.equal(getMonetaryValidationError("some text"), null); | ||
| assert.equal(getMonetaryValidationError(""), null); | ||
| }); | ||
|
|
||
| test("returns null for valid monetary prefix format", () => { | ||
| assert.equal(getMonetaryValidationError("USD10.00"), null); | ||
| assert.equal(getMonetaryValidationError("GBP123.45"), null); | ||
| assert.equal(getMonetaryValidationError("EUR9.99"), null); | ||
| assert.equal(getMonetaryValidationError("ILS50.00"), null); | ||
| }); | ||
|
|
||
| test("returns error for trailing dollar sign", () => { | ||
| const err = getMonetaryValidationError("10.00$"); | ||
| assert.ok(err); | ||
| assert.match(err, /USD10\.00/); | ||
| assert.match(err, /currency code as a prefix/); | ||
| }); | ||
|
|
||
| test("returns error for trailing euro sign", () => { | ||
| const err = getMonetaryValidationError("123€"); | ||
| assert.ok(err); | ||
| assert.match(err, /EUR123\.00/); | ||
| }); | ||
|
|
||
| test("returns error for trailing pound sign", () => { | ||
| const err = getMonetaryValidationError("50£"); | ||
| assert.ok(err); | ||
| assert.match(err, /GBP50\.00/); | ||
| }); | ||
|
|
||
| test("returns error for trailing shekel sign", () => { | ||
| const err = getMonetaryValidationError("100₪"); | ||
| assert.ok(err); | ||
| assert.match(err, /ILS100\.00/); | ||
| }); | ||
|
|
||
| test("returns error for trailing rupee sign", () => { | ||
| const err = getMonetaryValidationError("200₹"); | ||
| assert.ok(err); | ||
| assert.match(err, /INR200\.00/); | ||
| }); | ||
|
|
||
| test("returns error for trailing yen sign", () => { | ||
| const err = getMonetaryValidationError("500¥"); | ||
| assert.ok(err); | ||
| assert.match(err, /JPY500\.00/); | ||
| }); | ||
|
||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Too many comments. And too long. You should consider omitting them at all.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed all comments in 5224c05. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| const SYMBOL_TO_CODE: Record<string, string> = { | ||
| $: "USD", | ||
| "€": "EUR", | ||
| "£": "GBP", | ||
| "¥": "JPY", | ||
| "₹": "INR", | ||
| "₪": "ILS", | ||
| }; | ||
|
|
||
| const TRAILING_SYMBOL_REGEX = new RegExp( | ||
| `^(\\d+(?:\\.\\d+)?)[${Object.keys(SYMBOL_TO_CODE).join("")}]$` | ||
| ); | ||
|
|
||
| export function getMonetaryValidationError(value: string): string | null { | ||
| const trailingMatch = TRAILING_SYMBOL_REGEX.exec(value); | ||
| if (trailingMatch) { | ||
| const amount = trailingMatch[1]; | ||
| const symbol = value.slice(-1); | ||
| const code = SYMBOL_TO_CODE[symbol] || "USD"; | ||
| const numericAmount = parseFloat(amount); | ||
| const formattedAmount = isNaN(numericAmount) ? amount : numericAmount.toFixed(2); | ||
| return ( | ||
| `Invalid monetary format "${value}". ` + | ||
| `Paperless-NGX requires the currency code as a prefix, e.g. "${code}${formattedAmount}". ` + | ||
| `Use the format: {CURRENCY_CODE}{amount} (e.g., USD10.00, GBP123.45, EUR9.99).` | ||
| ); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
Comment on lines
+14
to
+30
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This must be covered with unit test.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added |
||
|
|
||
| export function validateCustomFields( | ||
| custom_fields: { field: number; value: unknown }[] | undefined | ||
| ) { | ||
| custom_fields | ||
| ?.filter((cf) => typeof cf.value === "string") | ||
| .forEach((cf) => { | ||
| const monetaryError = getMonetaryValidationError(cf.value as string); | ||
| if (monetaryError) throw new Error(monetaryError); | ||
| }); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new PR to run GitHub Actions Workflow with this tests o
for every PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't open a new PR, but I've added
.github/workflows/ci.ymlto this PR in eff830c. It runsnpm teston every PR and on pushes tomain.