Skip to content

fix: Allow null trait values - #101

Merged
Zaimwa9 merged 2 commits into
mainfrom
fix/null-trait-values
Oct 21, 2026
Merged

Zaimwa9 merged 2 commits into
mainfrom
fix/null-trait-values

Conversation

@Zaimwa9

@Zaimwa9 Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Sending a trait with a null value threw ArgumentError: Invalid value type, so getFeatureFlags(user: …, traits: [Trait(key: 'plan', value: null)]) failed before making the request. In Flagsmith, a null trait value deletes the trait, so null is a valid value.

Trait and TraitWithIdentity now accept null in both directions. A null value is sent as "trait_value": null, and a null value in a response parses as null instead of throwing. *.g.dart is unchanged: it already writes trait_value even when it is null.

Tests cover parsing and serialising null for both classes, and assert the identity request body for a null trait. Without the fix, all four new tests fail.

Traits are not persisted in the SDK's flag storage, so no cache changes are needed.

@Zaimwa9
Zaimwa9 requested a review from a team as a code owner October 21, 2026 15:10
@Zaimwa9
Zaimwa9 requested review from kyle-ssg and removed request for a team October 21, 2026 15:10
@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

Comment thread lib/src/core/model/trait.dart
@themis-blindfold

Copy link
Copy Markdown

⚖️ Themis review: 🟠 Fix before merge

TL;DR: Null traits now serialise through all request paths, but the public copyWith API still cannot produce a null value. The completed Flutter tests and Dart analysis checks passed.

Area Score
🎯 Correctness 3/5
🧪 Test coverage 3/5
📐 Code quality 4/5
🚀 Product impact 3/5

🟠 Majors

  • lib/src/core/model/trait.dart:58 — copyWith(value: null) retains the old value instead of producing a deletion request.
📝 Walkthrough
  • Trait serialisation - accepts and emits an explicit JSON null for deletion.
  • Identity evaluation - includes a null trait value in the identity request body.
  • Public trait models - copyWith still conflates an omitted value with explicit null.
🧪 How to verify
  1. Run flutter test test/core/models/trait_test.dart test/core/models/trait_identity_test.dart.
  2. Assert that Trait(key: 'plan', value: 'pro').copyWith(value: null).toJson()['trait_value'] is null.
  3. Assert the same behaviour for TraitWithIdentity, then call createTrait with the copied value against a test identity.
  4. Run flutter test test/fg/flagsmith_traits_test.dart.
    Automate: add the two copyWith(value: null) regression cases to the model tests.

Product take: This makes trait deletion usable across the SDK’s standard update paths. It is a small but important consistency fix for applications that model trait changes immutably.

🧭 Assumptions & unverified claims

No unverified assumptions or claims.

One last null-shaped pebble remains in the immutable path · reviewed at a37840a

@Zaimwa9

Zaimwa9 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

@themis-blindfold review

@Zaimwa9
Zaimwa9 merged commit 7487076 into main Oct 21, 2026
3 checks passed
@Zaimwa9
Zaimwa9 deleted the fix/null-trait-values branch October 21, 2026 16:19
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.

3 participants