fix: Prevent mutations from secondary side of relation #3124
Conversation
AndrewSisley
added
bug
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3124 +/- ##
===========================================
- Coverage 80.09% 80.06% -0.03%
===========================================
Files 353 353
Lines 28210 28175 -35
===========================================
- Hits 22593 22557 -36
- Misses 4025 4028 +3
+ Partials 1592 1590 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 13 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
AndrewSisley
force-pushed
the
3102-rm-sec-rel-updates
branch
from
9b5cc3b to
73d016f
Compare
| func TestMutationCreateOneToMany_AliasedRelationNameInvalidIDManySide_CreatedDoc(t *testing.T) { | ||
| test := testUtils.TestCase{ | ||
| Description: "One to many create mutation, invalid id, from the many side, with alias", | ||
| Actions: []any{ | ||
| testUtils.CreateDoc{ | ||
| CollectionID: 0, | ||
| Doc: `{ | ||
| "name": "Painted House", | ||
| "author": "ValueDoesntMatter" | ||
| }`, | ||
| }, | ||
| testUtils.Request{ | ||
| Request: `query { | ||
| Book { | ||
| name | ||
| } | ||
| }`, | ||
| Results: map[string]any{ | ||
| "Book": []map[string]any{ | ||
| { | ||
| "name": "Painted House", | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| executeTestCase(t, test) | ||
| } |
There was a problem hiding this comment.
question: Why not assert the current behavior rather than removing?
There was a problem hiding this comment.
There are already tests that test what this test would test if it was modified to match the current behaviour.
| func TestMutationCreateOneToOne_UseAliasWithNonExistingRelationSecondarySide_Error(t *testing.T) { | ||
| test := testUtils.TestCase{ | ||
| Description: "One to one create mutation, alias relation, from the secondary side", | ||
| Actions: []any{ | ||
| testUtils.CreateDoc{ | ||
| CollectionID: 0, | ||
| Doc: `{ | ||
| "name": "Painted House", | ||
| "author": "bae-be6d8024-4953-5a92-84b4-f042d25230c6" | ||
| }`, | ||
| ExpectedError: "document not found or not authorized to access", | ||
| }, | ||
| }, | ||
| } | ||
| executeTestCase(t, test) | ||
| } |
There was a problem hiding this comment.
question: Similar question as above
There was a problem hiding this comment.
Same answer :)
| @@ -566,6 +566,20 @@ func (g *Generator) buildMutationInputTypes(collections []client.CollectionDefin | |||
| continue | |||
| } | |||
|
|
|||
| if field.Kind == client.FieldKind_DocID && strings.HasSuffix(field.Name, request.RelatedObjectID) { | |||
There was a problem hiding this comment.
thought: the logic here and in client.Document is the same. Would it make sense to have a field.IsSecondaryRelation helper function instead?
There was a problem hiding this comment.
I thought about it, but decided it wasn't worth coupling them together and boost the (public) client package surface area. Especially given that the gql/schema package is begging for a re-write.
Spotted by Keenan, no longer needed.
Relevant issue(s)
Resolves #3102
Description
Prevents mutations from secondary side of relation.
Also validates that values given to relation fields are actually valid docIDs - previously it only validated this on the secondary side, and it was as easy to introduce it to the primary side as it was to correct the tests expecting the failure, so I've added this here.
As IDs set via secondary affected the secondary docID the order of results in some tests have changed. Also changed are tests that accidentally created docs via the secondary side that didn't test the secondary mutation (e.g. a lot of tests testing queries from the secondary side also created the test docs from this direction).