Skip to content

fix: allow nullable fields to be set to null via PATCH request#1783

Open
mrcavisg wants to merge 2 commits intoLerianStudio:mainfrom
mrcavisg:fix/issue-1778-nullable-fields
Open

fix: allow nullable fields to be set to null via PATCH request#1783
mrcavisg wants to merge 2 commits intoLerianStudio:mainfrom
mrcavisg:fix/issue-1778-nullable-fields

Conversation

@mrcavisg
Copy link

@mrcavisg mrcavisg commented Feb 4, 2026

Summary

Fixes #1778

When sending a PATCH request with "segmentId": null (or portfolioId, entityId), the null value was being
ignored and the field remained unchanged. This prevented users from unlinking an account from a segment/portfolio.

Root Cause

Go cannot distinguish between "field omitted from JSON" and "field explicitly set to null" when using pointer types -
both result in nil.

Solution

  • Used the existing nullable.Nullable[T] generic type (already in the codebase at pkg/nullable/) which tracks
    whether a field was present in the JSON payload
  • Modified UpdateAccountInput to use nullable.Nullable[string] for segmentId, portfolioId, and entityId
  • Updated the repository Update method to conditionally build the SQL query based on which fields were actually
    sent

Changes

  • pkg/mmodel/account.go - Changed nullable field types in UpdateAccountInput
  • components/onboarding/.../account.postgresql.go - Updated Update method to handle nullable fields
  • Updated corresponding mock and tests

Test Plan

  • Manual testing via Swagger UI:
    • Created account with segmentId
    • Sent PATCH with "segmentId": null → segmentId correctly set to null
    • Sent PATCH with {} (empty) → segmentId unchanged
    • Sent PATCH with new segmentId → segmentId updated
  • Unit tests passing

Notes

This fix only addresses the Account entity. If approved, the same pattern can be applied to other entities
(Organization, Alias, Holder) that have similar nullable fields.

Previously, sending null for segmentId, portfolioId, or entityId in
Account PATCH requests was ignored, preventing users from unlinking
accounts from segments.

This change introduces a nullable package that distinguishes between
"field not provided" and "field explicitly set to null" in JSON payloads.

The nullable package can be reused for other entities with similar fields.

Closes LerianStudio#1778

Signed-off-by: mrcavisg <caio.vinicius.contato@gmail.com>
@mrcavisg mrcavisg requested a review from a team as a code owner February 4, 2026 18:24
@coderabbitai
Copy link

coderabbitai bot commented Feb 4, 2026

Walkthrough

This pull request adds a generic Nullable[T] type to represent three JSON states (unset, null, value) and updates account input types to use nullable.Nullable[string] for SegmentID, PortfolioID, and EntityID. The account repository Update signature now accepts mmodel.UpdateAccountInput instead of a full Account, and the implementation applies conditional SQL updates based on Nullable semantics (ShouldUpdate / ShouldSetNull), sets updated_at, and returns the fresh record via a follow-up find query. Mock and service call sites were updated to pass the new UpdateAccountInput.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Service as Service Layer
    participant Repo as Repository
    participant DB as Database

    Client->>Service: UpdateAccount(ctx, UpdateAccountInput)
    Service->>Repo: Update(ctx, orgID, ledgerID, portfolioID, id, UpdateAccountInput)
    Repo->>Repo: Evaluate Nullable fields (ShouldUpdate / ShouldSetNull)
    Repo->>DB: EXECUTE UPDATE ... SET updated_at = now(), field = value/NULL ... WHERE id=?
    DB-->>Repo: OK
    Repo->>DB: SELECT * FROM account WHERE id=?
    DB-->>Repo: account row
    Repo-->>Service: *Account (fresh)
    Service-->>Client: Updated Account response
Loading
🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description covers the summary, root cause, solution, changes, test plan, and notes. However, it does not follow the required template structure with explicit checkbox items and sections. Restructure the description to follow the provided template with Pull Request Type checkboxes and numbered Checklist items to ensure consistency with repository standards.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: enabling nullable fields to be set to null via PATCH requests, which directly addresses the bug fix described in the PR.
Linked Issues check ✅ Passed The PR successfully addresses issue #1778 by implementing nullable field handling for Account entity via the Nullable[T] type, enabling explicit null values in PATCH requests to unlink accounts from segments/portfolios.
Out of Scope Changes check ✅ Passed All changes are scoped to Account entity handling of nullable fields. The addition of pkg/nullable package components and validator support are necessary dependencies. The PR notes that similar patterns can be applied to other entities in future work.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@mrcavisg
Copy link
Author

mrcavisg commented Feb 4, 2026

This is my first contribution to this project. I noticed the nullable package was already designed for this issue, so I used the existing infrastructure. Happy to adjust anything based on feedback.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@pkg/mmodel/account.go`:
- Around line 115-129: The nullable.Nullable[string] fields SegmentID,
PortfolioID, and EntityID currently carry validator tags (e.g.,
`validate:"omitempty,uuid"`) which won't work because the validator sees the
Nullable struct, not the inner string; register a custom type extractor with the
validator (e.g., using RegisterCustomTypeFunc) that returns the inner string
only when the Nullable is set and not null, or implement a custom validation
rule via RegisterValidation that inspects Nullable.IsSet/IsNull and then
validates Nullable.Value against uuid/max constraints so the existing tags on
SegmentID, PortfolioID and EntityID behave correctly.

In `@pkg/nullable/nullable.go`:
- Around line 69-76: MarshalJSON currently serializes unset fields (Nullable[T]
with IsSet=false) as JSON null which causes absent fields to appear as null on
re-marshal; implement an IsZero() bool method on Nullable[T] that returns
!n.IsSet so the type can be omitted via json:",omitzero" or omitempty semantics
(Go 1.24+ or struct tags) instead of changing MarshalJSON behavior — add IsZero
to the Nullable[T] type so callers can rely on omitzero/omitempty when they want
unset fields omitted.

  Register a custom type function in the validator to extract the inner
  string value from nullable.Nullable[string] fields. This ensures that
  validation tags (uuid, max) work correctly on SegmentID, PortfolioID,
  and EntityID fields.

  Signed-off-by: mrcavisg <caio.vinicius.contato@gmail.com>
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.

🐛 [BUG] - Nullable fields, once updated, do not allow the null value to be entered again.

1 participant