Skip to content

fix(units): patch synonyms as item-level add/delete operations - #559

Merged
prasad-albert merged 4 commits into
mainfrom
fix/units-synonyms-patch
Jun 24, 2026
Merged

fix(units): patch synonyms as item-level add/delete operations#559
prasad-albert merged 4 commits into
mainfrom
fix/units-synonyms-patch

Conversation

@prasad-albert

Copy link
Copy Markdown
Collaborator

Summary

  • Updating a unit's synonyms previously sent a single whole-list update op, which the backend rejects — it has no update operation for Synonyms (only item-level add/delete, each with a scalar value), and it rejects more than one operation on the same attribute in a single request.
  • synonyms changes are now diffed into per-item add/delete operations, and each Synonyms operation is sent as its own PATCH request (other attributes like symbol/category still go in one request).
  • Folded synonyms coverage into the existing test_unit_crud (no new fixture/seeding) — adds and removes a synonym and asserts the result. Verified end-to-end against app.albertinvent.com (full test_units.py suite: 8 passed).

@claude

claude Bot commented Jun 19, 2026

Copy link
Copy Markdown

Code Review

Issues Found

  • [src/albert/collections/units.py:121] update() is missing @validate_call. Every other public collection method in this file has it (get_by_id, get_by_ids, delete), and the method accepts a Unit argument that should be validated at the boundary.

Summary

1 issue found. Low-severity — the fix logic is correct; the missing decorator is the only gap.

@prasad-albert prasad-albert left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unset synonyms will delete all existing synonyms

_generate_unit_patch_payload reads updated.synonyms or [] without checking model_fields_set. If a caller builds a partial update object without setting synonyms:

unit = Unit(id="x", symbol="y")   # synonyms never set → not in model_fields_set
client.units.update(unit=unit)

updated.synonyms or [] resolves to [], so existing_synonyms - [] emits a DELETE for every existing synonym. Same unset-vs-empty bug fixed in base.py / generate_adv_patch_payload in #561.

One-liner fix — add this right after stripping the old Synonyms datum:

payload.data = [d for d in payload.data if d.attribute != "Synonyms"]
if "synonyms" not in updated.model_fields_set:
    return payload   # caller never set synonyms → leave them untouched

Explicit synonyms=[] still lands in model_fields_set so intentional clears are preserved.

Partial update objects without synonyms in model_fields_set resolved to
[] via 'or []', causing all existing synonyms to be deleted. Guard on
model_fields_set before diffing, matching the pattern in base.py.
@prasad-albert
prasad-albert force-pushed the fix/units-synonyms-patch branch from eda8e6e to 73cbeca Compare June 24, 2026 09:44
@prasad-albert
prasad-albert merged commit 30e64c5 into main Jun 24, 2026
1 check passed
@prasad-albert
prasad-albert deleted the fix/units-synonyms-patch branch June 24, 2026 10:07
prasad-albert added a commit that referenced this pull request Jun 25, 2026
The API rejects multiple operations on the same list attribute (e.g.
Symbols) in a single request with '1 entities duplicated'. Batch scalar
ops together and send each list-attribute op (delete/add) individually,
matching the pattern used for Units/Synonyms in PR #559.
prasad-albert added a commit that referenced this pull request Jun 26, 2026
The API rejects multiple operations on the same list attribute (e.g.
Symbols) in a single request with '1 entities duplicated'. Batch scalar
ops together and send each list-attribute op (delete/add) individually,
matching the pattern used for Units/Synonyms in PR #559.
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.

2 participants