Skip to content

fix(documentResource): send only changed fields on save#803

Open
ps173 wants to merge 2 commits into
frappe:mainfrom
ps173:fix/802-document-resource-save-standard-fields
Open

fix(documentResource): send only changed fields on save#803
ps173 wants to merge 2 commits into
frappe:mainfrom
ps173:fix/802-document-resource-save-standard-fields

Conversation

@ps173

@ps173 ps173 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Problem

save.makeParams serialized the entire doc and stripped only doctype and name, so read only standard fields returned by frappe.client.get (owner, creation, modified, modified_by, docstatus, idx) leaked into the set_value payload. The server rejected it with Cannot edit standard fields and the edit was never persisted.

Fix

Diff out.doc against out.originalDoc and send only the changed fields. Since originalDoc is a snapshot taken right after load, unchanged standard fields never enter the payload. doctype and name are still stripped to cover the case where originalDoc is null.

Tests

Added src/resources/documentResource.test.ts covering changed field only payloads, the no change case and the multi field subset case.

Fixes #802

🤖 Generated with Claude Code

Coverage: 58.03% (+0.05% vs main)

save serialized the whole doc and stripped only doctype and name, so
standard fields like owner, creation, modified, docstatus and idx leaked
into the set_value payload and the server rejected it with "Cannot edit
standard fields". Diff against originalDoc and send only changed fields.

Fixes frappe#802

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ps173 ps173 requested a review from netchampfaris June 26, 2026 12:25
@ps173

ps173 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

/barista review

@barista-for-frappe

Copy link
Copy Markdown

Looks good — diffs out.doc against out.originalDoc so only changed fields hit the set_value payload, fixing the "Cannot edit standard fields" rejection in #802. originalDoc is a snapshot set right after load/save (documentResource.js:87, :54), and the originalDoc || {} fallback degrades safely to the old behavior. Scope is right — only save changed; setValue/setValueDebounced still pass explicit field values. Tests cover changed-field, no-change, and subset cases.

Minor nit:

  • documentResource.js:110-119 — when nothing changed, save.submit() still fires a request with fieldname: {} (the test at documentResource.test.ts:74 asserts this). Not a regression (the old code also sent a full payload), but you could short-circuit the empty case to skip the round-trip.

Short-circuit save when the diff is empty so it no longer fires a
set_value request with an empty fieldname. Diff logic extracted into a
shared getChangedFields helper.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mehmehsloth

Copy link
Copy Markdown

Addressed the review nit in 367b3e9

  • save now short-circuits and resolves without a request when there are no pending changes, so it no longer fires set_value with an empty fieldname
  • diff logic extracted into a shared getChangedFields helper used by both makeParams and the short-circuit check
  • updated the no-change test to assert set_value is never hit

All 174 tests pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

documentResource save sends standard fields and fails with Cannot edit standard fields

2 participants