Fix bulk custom field parameters#89
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 91e5cd8 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 |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR refactors the custom field bulk edit parameter handling from an array-based assignment model ( Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/tools/documents.test.ts (3)
35-35: 💤 Low valueExtra blank line.
There's an additional blank line here that creates inconsistent spacing between test cases.
♻️ Proposed fix
- - test("buildBulkEditParameters includes Paperless-required empty custom field keys", () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/documents.test.ts` at line 35, Remove the extra blank line in src/tools/documents.test.ts that sits between the test cases (the stray empty line around line 35), so spacing between the adjacent describe/it or test blocks is consistent; simply delete that empty line to restore the file's expected test spacing.
5-43: ⚡ Quick winConsider adding edge case tests for comprehensive coverage.
The current tests cover the core requirements well. For improved robustness, consider adding tests for:
- Empty custom fields array (vs
undefined)- Combination of base parameters, custom fields, and
includeEmptyCustomFieldsflag- Different value types (numbers, booleans) to verify type preservation
💡 Example additional test cases
test("buildBulkEditParameters handles empty custom fields array", () => { const parameters = buildBulkEditParameters({}, []); assert.deepEqual(parameters, { add_custom_fields: {}, }); }); test("buildBulkEditParameters combines base params with custom fields", () => { const parameters = buildBulkEditParameters( { remove_tags: [1, 2], add_tags: [3] }, [{ field: 9, value: "test" }] ); assert.deepEqual(parameters, { remove_tags: [1, 2], add_tags: [3], add_custom_fields: { "9": "test", }, }); }); test("buildBulkEditParameters preserves different value types", () => { const parameters = buildBulkEditParameters({}, [ { field: 1, value: 42 }, { field: 2, value: true }, { field: 3, value: "" }, ]); assert.strictEqual(parameters.add_custom_fields["1"], 42); assert.strictEqual(parameters.add_custom_fields["2"], true); assert.strictEqual(parameters.add_custom_fields["3"], ""); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/documents.test.ts` around lines 5 - 43, Add unit tests around buildBulkEditParameters to cover the suggested edge cases: (1) call buildBulkEditParameters({}, []) and assert it returns add_custom_fields: {} (and no unexpected keys), (2) call buildBulkEditParameters with a non-empty base params object (e.g., remove_tags/add_tags) plus a custom fields array and assert the returned object merges base params and adds add_custom_fields with the correct id->value mapping, and (3) call buildBulkEditParameters with custom field values of different types (number, boolean, empty string, null) and assert the values are preserved as-is in parameters.add_custom_fields; reference the buildBulkEditParameters function in tests and assert presence/absence of keys as in existing tests.
7-7: 💤 Low valueConsider removing the unnecessary type assertion.
The type assertion
as number[]appears redundant since TypeScript should infer the type from the object literal and the function signature.♻️ Proposed simplification
- { remove_custom_fields: [] as number[] }, + { remove_custom_fields: [] },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/documents.test.ts` at line 7, The inline type assertion "as number[]" is unnecessary on the object literal property remove_custom_fields; remove the "as number[]" so the line becomes { remove_custom_fields: [] }, letting TypeScript infer the type from the literal and the surrounding function signature or test helper (look for the test helper/fixture that consumes remove_custom_fields in this file).src/tools/documents.ts (1)
22-46: ⚡ Quick winAdd JSDoc documentation for the exported utility function.
The
buildBulkEditParametersfunction is exported and has non-trivial logic (conditional defaults, array-to-record transformation). As per coding guidelines, complex functions should have clear JSDoc comments, especially exported functions.📝 Proposed JSDoc documentation
+/** + * Builds bulk edit parameters by transforming custom field updates into the format expected by Paperless-NGX. + * Converts an array of custom field updates into a Record mapping field IDs to values. + * + * `@template` T - The base parameters type + * `@param` parameters - Base parameters to include in the result + * `@param` addCustomFields - Optional array of custom field updates to transform + * `@param` includeCustomFieldDefaults - Whether to include empty add_custom_fields and remove_custom_fields defaults (required for modify_custom_fields method) + * `@returns` Combined parameters with transformed custom fields + */ export function buildBulkEditParameters<T extends Record<string, unknown>>(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tools/documents.ts` around lines 22 - 46, Add JSDoc for the exported utility buildBulkEditParameters describing its purpose, parameters, return type, and behavior: document parameters (parameters: T, addCustomFields?: BulkCustomFieldUpdate[], includeCustomFieldDefaults?: boolean), explain that addCustomFields transforms an array into the add_custom_fields record (keys from customField.field and values from customField.value), note that includeCustomFieldDefaults will initialize add_custom_fields and remove_custom_fields defaults, and specify the returned type T & BulkCustomFieldParameters; include examples or edge-case notes about empty addCustomFields and the use of nullish coalescing (apiParameters.add_custom_fields ??= {}).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/types.ts`:
- Line 153: CustomFieldInstanceRequest["value"] currently allows a broad object
type which mismatches the runtime tools that use number[] (see
BulkCustomFieldValue and the Zod schema); update the union for
CustomFieldInstanceRequest["value"] to explicitly include number[] (or replace
the generic object with number[]) so add_custom_fields?: Record<string,
CustomFieldInstanceRequest["value"]> accepts the actual array shape, and ensure
the Zod schema and BulkCustomFieldValue definition remain consistent with this
change.
---
Nitpick comments:
In `@src/tools/documents.test.ts`:
- Line 35: Remove the extra blank line in src/tools/documents.test.ts that sits
between the test cases (the stray empty line around line 35), so spacing between
the adjacent describe/it or test blocks is consistent; simply delete that empty
line to restore the file's expected test spacing.
- Around line 5-43: Add unit tests around buildBulkEditParameters to cover the
suggested edge cases: (1) call buildBulkEditParameters({}, []) and assert it
returns add_custom_fields: {} (and no unexpected keys), (2) call
buildBulkEditParameters with a non-empty base params object (e.g.,
remove_tags/add_tags) plus a custom fields array and assert the returned object
merges base params and adds add_custom_fields with the correct id->value
mapping, and (3) call buildBulkEditParameters with custom field values of
different types (number, boolean, empty string, null) and assert the values are
preserved as-is in parameters.add_custom_fields; reference the
buildBulkEditParameters function in tests and assert presence/absence of keys as
in existing tests.
- Line 7: The inline type assertion "as number[]" is unnecessary on the object
literal property remove_custom_fields; remove the "as number[]" so the line
becomes { remove_custom_fields: [] }, letting TypeScript infer the type from the
literal and the surrounding function signature or test helper (look for the test
helper/fixture that consumes remove_custom_fields in this file).
In `@src/tools/documents.ts`:
- Around line 22-46: Add JSDoc for the exported utility buildBulkEditParameters
describing its purpose, parameters, return type, and behavior: document
parameters (parameters: T, addCustomFields?: BulkCustomFieldUpdate[],
includeCustomFieldDefaults?: boolean), explain that addCustomFields transforms
an array into the add_custom_fields record (keys from customField.field and
values from customField.value), note that includeCustomFieldDefaults will
initialize add_custom_fields and remove_custom_fields defaults, and specify the
returned type T & BulkCustomFieldParameters; include examples or edge-case notes
about empty addCustomFields and the use of nullish coalescing
(apiParameters.add_custom_fields ??= {}).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3bd52ba4-f115-4f17-b3d4-5669be35adfb
📒 Files selected for processing (4)
README.mdsrc/api/types.tssrc/tools/documents.test.tssrc/tools/documents.ts
Tightens the custom field value type, documents buildBulkEditParameters, and adds edge-case coverage for empty custom field arrays and type preservation.\n\nValidation:\n- npm test\n- npm run build
baruchiro
approved these changes
May 18, 2026
This was referenced May 20, 2026
baruchiro
added a commit
that referenced
this pull request
May 24, 2026
Closes #101 ## Summary - **`docker-compose.e2e.yml`** — Paperless-ngx 2.14 + Redis fixture stack with auto-created admin user - **`e2e/e2e.test.ts`** — Deterministic `tools/call` tests (no LLM) using Node's native `node:test` covering all tools from the issue: `list_tags`, `create_tag`, `list_correspondents`, `create_correspondent`, `list_document_types`, `create_document_type`, `list_documents`, `get_document`, `search_documents`, `download_document` (regression for #87), `get_document_thumbnail`, `bulk_edit_documents` (regression for #100, #89), `post_document` - **`e2e/client.ts`** — SDK `StreamableHTTPClientTransport` factory - **`e2e/paperless.ts`** — Direct Paperless REST client for seeding test data in `before()` hooks - **`.github/workflows/e2e.yml`** — Matrix job (CLI + Docker) triggered on every PR and push to `main` - **`README.md`** — Testing section with unit and E2E run instructions - **`package.json`** — `npm run test:e2e` script ## Test plan - [ ] Verify CI passes on this PR (E2E matrix job: cli + docker) - [ ] Check that `list_tags` finds the seeded tag - [ ] Check that `download_document` returns a `paperless://` URI (regression #87) - [ ] Check that `bulk_edit_documents` modify_tags round-trip works (regression #100) - [ ] Confirm Docker image job also passes (builds image then runs same test file) - [ ] Review README Testing section renders correctly https://claude.ai/code/session_01LActHt6HMhygRppaphFRzQ --- _Generated by [Claude Code](https://claude.ai/code/session_01LActHt6HMhygRppaphFRzQ)_ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added a comprehensive E2E test suite that runs locally and in CI against a real Paperless-ngx instance, exercising tag/correspondent/document-type management, document operations (list/get/search/download/thumbnail), bulk tag edits, and uploads. * **Documentation** * Added a “Testing” section with prerequisites and instructions for running unit and E2E tests and cleanup steps. * **Chores** * Added CI workflow, Docker Compose for E2E, and an npm script to run E2E tests; excluded E2E from regular TypeScript compilation. * **Bug Fixes** * Bulk-edit behavior now initializes tag fields by default for tag-modify operations to ensure robust payloads. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/baruchiro/paperless-mcp/pull/104?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude <noreply@anthropic.com>
](https://app.coderabbit.ai/change-stack/baruchiro/paperless-mcp/pull/104?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
modify_custom_fieldsbulk edits using Paperless-NGX's expectedadd_custom_fieldsid/value map instead of unsupportedassign_custom_fieldsfields.add_custom_fields/remove_custom_fieldsdefaults formodify_custom_fieldsoperations.nullcustom field values so workflows can create intentionally empty fields, e.g. pending markers on date custom fields.Why
Paperless-NGX validates bulk custom field edits with
parameters.add_custom_fieldsandparameters.remove_custom_fields. The current MCP transformation sendsassign_custom_fieldsandassign_custom_fields_values, which Paperless rejects with errors likeadd_custom_fields not specified.This also prevents setting an intentionally empty custom field value used by workflows such as "date field is present but empty = pending".
Tests
npm testnpm run buildNote: local environment is Node 22, while the package declares Node >=24, so npm prints an EBADENGINE warning during install. Tests and TypeScript build pass.
Summary by CodeRabbit
Breaking Changes
Documentation
Tests