Conversation
🦋 Changeset detectedLatest commit: 72eec16 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-authored-by: baruchiro <17686879+baruchiro@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves the user experience for Paperless-NGX monetary custom fields by adding early validation and documentation. When users attempt to set a monetary field with a trailing currency symbol (e.g., 10.00$), the system now catches the mistake before making the API call and provides an actionable error message explaining the correct format (USD10.00).
Changes:
- Adds client-side validation that detects and rejects common monetary format mistakes (trailing currency symbols)
- Updates tool descriptions to document the required currency code prefix format for monetary fields
- Provides clear, actionable error messages with format examples when validation fails
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/tools/utils/monetary.ts |
New validation utility that detects trailing currency symbol patterns and generates helpful error messages with the correct format |
src/tools/documents.ts |
Integrates monetary validation in bulk_edit_documents and update_document tools; updates parameter descriptions to document the monetary format requirement |
src/tools/customFields.ts |
Updates create_custom_field tool description to document the monetary format requirement upfront |
.changeset/improve-monetary-field-ux.md |
Changeset file documenting this UX improvement as a patch release |
src/tools/documents.ts
Outdated
| if (add_custom_fields) { | ||
| for (const cf of add_custom_fields) { | ||
| if (typeof cf.value === "string") { | ||
| const monetaryError = getMonetaryValidationError(cf.value); | ||
| if (monetaryError) throw new Error(monetaryError); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
You can do something like add_custom_fields?.filter(value === string).forEach
There was a problem hiding this comment.
Refactored to filter+forEach in 5224c05 (inside the extracted validateCustomFields function).
src/tools/documents.ts
Outdated
| ]) | ||
| .describe( | ||
| "The value for the custom field. For documentlink fields, use a single document ID (e.g., 123) or an array of document IDs (e.g., [123, 456])." | ||
| "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])." |
There was a problem hiding this comment.
You're updating this description in many places, let's save the description in one const place and reuse it.
There was a problem hiding this comment.
Extracted to CUSTOM_FIELD_VALUE_DESCRIPTION const at the top of documents.ts and reused in both tools in 5224c05.
src/tools/documents.ts
Outdated
| if (updateData.custom_fields) { | ||
| for (const cf of updateData.custom_fields) { | ||
| if (typeof cf.value === "string") { | ||
| const monetaryError = getMonetaryValidationError(cf.value); | ||
| if (monetaryError) throw new Error(monetaryError); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This pattern already added in this PR, you should consider a function for "validateCustomFields"
There was a problem hiding this comment.
Extracted to a validateCustomFields function in 5224c05, called in both bulk_edit_documents and update_document.
There was a problem hiding this comment.
Too many comments. And too long. You should consider omitting them at all.
src/tools/utils/monetary.ts
Outdated
| /** Regex detecting values with a trailing currency symbol (common mistake, e.g., "10.00$") */ | ||
| const TRAILING_SYMBOL_REGEX = /^(\d+(?:\.\d+)?)[$€£¥₹]$/; | ||
|
|
||
| /** Map of common currency symbols to ISO 4217 currency codes */ | ||
| const SYMBOL_TO_CODE: Record<string, string> = { | ||
| $: "USD", | ||
| "€": "EUR", | ||
| "£": "GBP", | ||
| "¥": "JPY", | ||
| "₹": "INR", | ||
| }; |
There was a problem hiding this comment.
If you will declare the mapping first, you will be able to use its Object.keys in the regex
There was a problem hiding this comment.
Done in 5224c05 — SYMBOL_TO_CODE is now declared first and Object.keys(SYMBOL_TO_CODE).join("") is used to build the character class in the regex dynamically.
Co-authored-by: baruchiro <17686879+baruchiro@users.noreply.github.com>
Head branch was pushed to by a user without write access
src/tools/documents.ts
Outdated
| 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.
I think it should be in a shared place too.
There was a problem hiding this comment.
Moved validateCustomFields to src/tools/utils/monetary.ts and exported it from there in 7bb0c4f.
src/tools/documents.ts
Outdated
| 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])."; |
There was a problem hiding this comment.
Consider a shared file for string constants/prompts/descriptions.
Do not apply it for all strings, it will be ongoing process.
There was a problem hiding this comment.
Created src/tools/utils/descriptions.ts as the shared file for string constants. CUSTOM_FIELD_VALUE_DESCRIPTION is now exported from there and imported in documents.ts in 7bb0c4f.
| const SYMBOL_TO_CODE: Record<string, string> = { | ||
| $: "USD", | ||
| "€": "EUR", | ||
| "£": "GBP", | ||
| "¥": "JPY", | ||
| "₹": "INR", | ||
| }; |
| 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; | ||
| } |
There was a problem hiding this comment.
This must be covered with unit test.
There was a problem hiding this comment.
Added src/tools/utils/monetary.test.ts with 8 unit tests (covering valid formats, non-monetary strings, and all 6 currency symbols including ILS) using Node's built-in node:test runner in 7bb0c4f. Updated package.json test script — all tests pass.
…t tests Co-authored-by: baruchiro <17686879+baruchiro@users.noreply.github.com>
| }, | ||
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1", | ||
| "test": "node --require ts-node/register --test src/**/*.test.ts", |
There was a problem hiding this comment.
@copilot open a new PR to run GitHub Actions Workflow with this tests o
for every PR.
There was a problem hiding this comment.
I can't open a new PR, but I've added .github/workflows/ci.yml to this PR in eff830c. It runs npm test on every PR and on pushes to main.
Co-authored-by: baruchiro <17686879+baruchiro@users.noreply.github.com>
Head branch was pushed to by a user without write access
src/tools/utils/monetary.test.ts
Outdated
| 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/); | ||
| }); |
There was a problem hiding this comment.
No need to test every symbol. The idea is clear after the first symbol.
Co-authored-by: baruchiro <17686879+baruchiro@users.noreply.github.com>
src/tools/utils/monetary.ts– validation utility for monetary custom field valuesupdate_documentandbulk_edit_documentstools with improved descriptions and validationcreate_custom_fieldtool description.github/workflows/ci.ymlto runnpm teston every PR and push to mainOriginal prompt
🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.