payload icon indicating copy to clipboard operation
payload copied to clipboard

fix: Versions - make sure parent key is the same type as parent _id.

Open apetrisor opened this issue 1 year ago • 2 comments

Description

Fix for issue #6349

When creating versions in mongodb, the type of the parent key needs to match the type of the _id of the parent. Otherwise you might end up with strings instead of numbers which will break the UI (seeing multiple versions when you shouldn't, not seeing the latest version, etc).

There are a few ways to solve this, however the most elegant I've found was to add support for an optional property to relationship fields, where we can manually specify the type of the parent key. I called this idType:

Screenshot 2024-05-14 at 13 42 57

Then, when building version collection fields, we can pass in the id type of the parent collection:

Screenshot 2024-05-14 at 13 43 33

Finally, when we build our schema we try for one of the allowed id types, falling back to the default Schema.Types.Mixed.

Screenshot 2024-05-14 at 13 44 29 Screenshot 2024-05-14 at 13 45 19

Other ways to solve this would be quite hacky and prone to bugs down the line:

  • In the relationship field schema generator, check whether this is the parent key of a version collection, then look at the parent collection and figure out the id type. - Quite bad
  • In db-mongodb's init function, when we generate the versionSchema, check the parent collection and override the version's schema as needed. - Better but not really

The solution proposed uses payload's own API, it is clear and easy to understand, and opens the possibility of adding your own custom IDs in relationship fields - completely optional with no side effects.

Have not tested this bug on Postgres, however if this PR is accepted, it should be fairly simple for someone to use the idType property with similar logic in db-postgres.

I can make the required changes to the documentation if this is accepted.

  • [x] I have read and understand the CONTRIBUTING.md document in this repository.

Type of change

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [x] This change requires a documentation update

Checklist:

  • [x] Existing test suite passes locally with my changes

apetrisor avatar May 14 '24 12:05 apetrisor

I will investigate.

apetrisor avatar May 16 '24 09:05 apetrisor

I will investigate.

@apetrisor did you get a chance to look into this? I'm actually encountering some issues around this due to adopting typeid-js for ids. I may consider a monkeypatching approach in the meantime if that's the best path for the time being

brandonpapworth avatar Jun 24 '24 16:06 brandonpapworth

This pull request was automatically closed due to lack of activity.

If you believe this PR is critical, please resolve all merge conflicts and re-open it.

If this is a feature, please create a new PR against the latest codebase.

Thank you for being part of the Payload community! 🙌

github-actions[bot] avatar Dec 06 '24 15:12 github-actions[bot]