fix(api): Serialize TemporalDateTime in identifier for ModelQueries.get - #7255
fix(api): Serialize TemporalDateTime in identifier for ModelQueries.get#7255VarshithaPamisetty wants to merge 7 commits into
Conversation
Run get-by-identifier values through _getSerializedValue (like list/filter) so a TemporalDateTime custom primary key no longer throws JsonUnsupportedObjectError. Issues: #6298
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7255 +/- ##
==========================================
+ Coverage 40.97% 40.98% +0.01%
==========================================
Files 121 121
Lines 8273 8273
Branches 3598 3598
==========================================
+ Hits 3390 3391 +1
+ Misses 4883 4882 -1 🚀 New features to boost your workflow:
|
| final variables = GraphQLRequestFactory.instance | ||
| .buildVariablesForGetRequest(modelIdentifier: modelIdentifier); |
There was a problem hiding this comment.
I think this need to be added to delete operations too.
There was a problem hiding this comment.
And do we need to adapt test models (e.g. test/test_models/schema.graphql)? Do we need to add something like
type CpkTemporalPrimaryKey @model {
hwid: String! @primaryKey(sortKeyFields: ["sessionStart"])
sessionStart: AWSDateTime!
}
There was a problem hiding this comment.
Added the delete operation and the CpkTemporalPrimaryKey type to the test schema (as suggested).
|
|
||
| // NOTE: This file is generated and may not follow lint rules defined in your app | ||
| // Generated files can be excluded from analysis in analysis_options.yaml | ||
| // For more info, see: https://dart.dev/guides/language/analysis-options#excluding-code-from-analysis |
There was a problem hiding this comment.
Who/what generated it? The Amazon header is usually not used for autogenerated files.
There was a problem hiding this comment.
I added the header manually to match our license convention, it's not from modelgen. Yes, it doesn't belong on a generated file. I'll remove it and restore the generator's default.
| CpkTemporalPrimaryKey: a | ||
| .model({ | ||
| hwid: a.string().required(), | ||
| sessionStart: a.datetime().required(), | ||
| }) | ||
| .identifier(["hwid", "sessionStart"]) | ||
| .authorization((allow) => [allow.authenticated()]), | ||
|
|
There was a problem hiding this comment.
Where is this used in the modified tests? If we want to extend e2e, we should also modify the test itself (example/integration_test/graphql/user_pools_test.dart).
There was a problem hiding this comment.
Added an e2e test that exercises it, along with the generated example model it needs.
Applies the get-path serialization to delete via a shared serializeModelIdentifier helper so ModelMutations.delete() with a TemporalDateTime custom key no longer throws JsonUnsupportedObjectError. Restores the generator's default header on the CpkTemporalPrimaryKey fixture, adds the type to test_models/schema.graphql, and adds an e2e test (create/get/delete by the composite key) plus the matching example model. Addresses review feedback on #7255. Issues: #6298
3 participants
Description
ModelQueries.get()on a model whose custom identifier includes aTemporalDateTimethrewJsonUnsupportedObjectError. The get-by-identifier path passed identifier values straight to JSON encoding without running them through_getSerializedValue(which the list/filter path already uses), so theTemporalDateTimewas never stringified.This serializes identifier values for both the get variables and the composite-key document. Adds a Gen2-generated
CpkTemporalPrimaryKeytest model + regression test.Testing
graphql_test.dart(asserts the get document and JSON-encodable, correctly-serialized variables).ModelQueries.get()by the composite key with aTemporalDateTime, across multiple date formats (incl. timezone offset), all returning cleanly.Supersedes #7237.
Fixes #6298