ObjectIds and Strings should both work with useUpdate and filters

[GraemeFulton]

Describe the bug
Currently, for a user to update a document, the userID of the doc needs to be an ObjectId.
Whereas graphQL filters only work for strings, so you can't filter by userID if the ID is an ObjectID.

More detail:
If you have documents with relational userID as a String ID (not ObjectId), the useUpdate mutation will give a permission error when you try to edit it, even if you are the owner.
To fix it, change the userId in the database to an ObjectId, and useUpdate will then work.

Conversely if you set the userIds to objectIds instead of Strings for all those documents, the useMulti filters will not work when filtering by the userId.
useMulti filter will only work with strings.

To Reproduce
Steps to reproduce the behavior:

1. Use Compass to change a userId for a document to a String
2. Try to update the document with useUpdate - you get permission error
3. Change the userId to an ObjectId
4. Try to update the document with useUpdate - it works!
5. Now try filtering with useMulti, by userId
6. It doesn't work

Expected behavior

• useUpdate should work with both String Ids and ObjectIds, so it will work for everyone
• graphQL filters should work for String Ids and ObjectIds - maybe it converts the ObjectId to a string or something, so that filtering works.

Screenshots (if applicable)

[GraemeFulton]

Also, when you create a new document, it looks like the document is inserted with the userId as an objectID always.
So I think only fixing the filter part of this issue would do the trick?

[eric-burel]

Probably, the problem is serialization of the variables, your ObjectId doesn't survive the network connection basically
But server-side you can't use a string either because Mongo will never find a matching userId, as it stores it as ObjectID
So I have to think about it more carefully, we will have a similar issue with any relation id

Maybe adding a graphql scalar that does the translation, in which case we would need a new type "MongoId" for fields, it's annoying though

[GraemeFulton]

That explains it, if it is common practice to use ObjectID everywhere, even for relational IDs, I don't know how people are filtering by document owner with apollo - I tried apollo's useQuery, but it can't filter by objectId either 😤

[GraemeFulton]

I tried doing this too, adding a virtual field that I could query by string, but found it's not possible to query it

uidString:{
    type: String,
    optional:true,
    canRead:['anyone'],
    resolveAs:{
      resolver:async(doc, args, context)=>{
        const usr = await context.VulcanUser.connector.findOne(
          { _id:  ObjectId(doc.userId) }
          );
          return usr._id;
      }
    }
  },

[eric-burel]

Yeah in graphql you have this concept of Scalar if you want to pass stuffs like Date, GraphQL should translate them automatically.

However it's complicated as you have to add this info in the Vulcan schema as well, so that the type of "userId" is not "string" but "objectId" in your graphql schema basically

I think only Mongo+GraphQL user will have the response to this

It's great to have the week-end coming because it will need some careful thinking ^^ but very relevant issue indeed

[eric-burel]

Update:

• See VulcanJS/vulcan-npm#129 to follow progress
• See https://github.com/VulcanJS/vulcan-npm/blob/feature/mongo-objectid/starters/next/src/pages/api/graphql.ts#L29
  It shows how to enable a new GraphQL type "GraphqlObjectId": when used instead of "String", your value will be automatically converted to an "ObjectId" server-side

Your are then expected to do import { GraphqlObjectId } from '@vulcanjs/mongo-apollo'; const mySchema = { _id : { ..., typeName: GraphqlObjectId }}
You just need to tell Vulcan that it's not a String, but a GraphqlObjectId. I haven't actually tested it yet though.

Filters probably don't work yet, it may take some time to figure everything right but I think it should work ok.

This is published in version 0.6.9-alpha.3 of @vulcanjs/mongo-apollo (latest version basically)

[GraemeFulton]

thanks Eric, for short term so I can have filtering, I did a workaround:

• Duplicated userId ObjectIDs to a 'userStringId' to all my documents for filtering to work.
• OnCreate document, I always populate the userStringID for filtering on
• All other relational Ids (think that's called a foreign key?) I just use strings

So only Ownership checks use the userId ObjectID

[eric-burel]

I've improved the support for custom scalar for ids, and exposed a GraphqlObjectId typeName, in the latest alpha release

This seems to work in my basic test, will dig and merge later. I need to double check the "owners" property but it should work ok.

What happens currently:

• GraphQL scalar means that the graphql type "GraphqlObjectId" is automatically translated to a relevant string server side, and the reverse is also true when the data are sent back to the client
• We already had some conversion logic in connectors => it might become redundant now??
• You have to explicitely add this typeName, which is a good practice since Mongo Id are not actually strings

[eric-burel]

Still experimental but it should work better now, we can switch betwen ObjectId (default) and strings
I've started some work to get rid of Mongoose and prefer mongo as well, see VulcanJS/vulcan-npm#130