Skip to content

fix: BaseModel/Patcher record desync after save() causes silent sette…#1567

Merged
sandsinh merged 3 commits intomainfrom
SITES-43758
Apr 29, 2026
Merged

fix: BaseModel/Patcher record desync after save() causes silent sette…#1567
sandsinh merged 3 commits intomainfrom
SITES-43758

Conversation

@sandsinh
Copy link
Copy Markdown
Contributor

@sandsinh sandsinh commented Apr 29, 2026

Summary

Fixes a bug in @adobe/spacecat-shared-data-access where setter writes are not visible via getters on the same model instance after a prior save(). Setters appear to silently no-op until the model is reloaded.

Root cause

BaseModel and Patcher are constructed sharing a single record object reference (base.model.js:64,72). On patcher.save(), collection.applyUpdateWatchers(this.record, updates) returns a shallow clone (base.collection.js:255const nextRecord = { ...record }). The patcher then reassigns this.record = watched.record (patcher.js:183), but the model's this.record is never updated.

After that point:

  • setters write to patcher.record (the new clone)
  • getters read model.record (the stale original)

The _saveMany path explicitly syncs the model (base.collection.js:999preparedItem.model.record = preparedItem.record). The single save() path was missing the equivalent sync.

Reproduction

const site = await Site.findById(id);
site.setName('first');
await site.save();

site.setOrganizationId(otherOrgId);
site.getOrganizationId(); // returns the OLD org id

Fix

packages/spacecat-shared-data-access/src/util/patcher.js:183 — replace this.record = watched.record with Object.assign(this.record, watched.record). This preserves the shared object reference between Patcher.record and BaseModel.record, so post-save setters and getters stay in sync. No public API change.

Why it wasn't caught

  • test/unit/util/patcher.test.js stubbed applyUpdateWatchers and asserted only updateByKeys.calledOnce — never constructed a real BaseModel to verify post-save getter reads.
  • test/unit/models/base/base.model.test.js stubs patcher.save entirely, so the real reassignment never runs.
  • No integration test exercised the set → save → set → get sequence.

Test plan

  • Unit regression in test/unit/util/patcher.test.js:
    • preserves the record reference after a collection-strategy save
    • reflects subsequent patches on the same record after a save
  • Integration regression in test/it/site/site.test.js:
    • reflects setter writes via getters after a prior save on the same instance
  • npm run lint -w packages/spacecat-shared-data-access
  • npm test -w packages/spacecat-shared-data-access (full unit suite, CI)
  • npm run test:it -w packages/spacecat-shared-data-access (integration suite, CI/local Docker)

Related Issues

Comment thread package-lock.json Outdated
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown

This PR will trigger a patch release when merged.

@sandsinh sandsinh merged commit c9c782d into main Apr 29, 2026
5 checks passed
@sandsinh sandsinh deleted the SITES-43758 branch April 29, 2026 11:45
solaris007 pushed a commit that referenced this pull request Apr 29, 2026
## [@adobe/spacecat-shared-data-access-v3.55.1](https://github.com/adobe/spacecat-shared/compare/@adobe/spacecat-shared-data-access-v3.55.0...@adobe/spacecat-shared-data-access-v3.55.1) (2026-04-29)

### Bug Fixes

* BaseModel/Patcher record desync after save() causes silent sette… ([#1567](#1567)) ([c9c782d](c9c782d))
@solaris007
Copy link
Copy Markdown
Member

🎉 This PR is included in version @adobe/spacecat-shared-data-access-v3.55.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants