Generators icon indicating copy to clipboard operation
Generators copied to clipboard

Add support to mongodb driver

Open promatik opened this issue 2 years ago • 11 comments

Fix for https://github.com/Laravel-Backpack/CRUD/issues/4000.

This adds support to MongoDB databases.

promatik avatar Dec 11 '21 01:12 promatik

Since Backpack itself has unofficial support for MongoDB... yeah... I guess we should do this 😀 I just want it written down in history that once again we have to litter our code with conditionals for MongoDB... but yeah... I don't see any way around this... thank you @promatik .

I don't have a way to test this right now so let's please ask @MeerKatDev and/or @pxpm to give this a go, so we can merge it.

Cheers!

tabacitu avatar Dec 11 '21 05:12 tabacitu

I would avoid setting this \Jenssegers\Mongodb\Eloquent\Model::class as hardcoded model for mongodb, but rather make it the default of a configurable value.

MeerKatDev avatar Dec 11 '21 09:12 MeerKatDev

@MeerKatDev great suggestion 👌 We don't have a config right now on this package, adding one just to set the Model, I'll let @tabacitu decide on that 😬 (I agree with that btw)

Anyway @MeerKatDev, would you prefer to not have the base Model changed and change it manually?

promatik avatar Dec 11 '21 23:12 promatik

Anyway @MeerKatDev, would you prefer to not have the base Model changed and change it manually?

I think it's ok to keep it changed, because as of now and I think in the near future, that package is the standard for using mongodb

MeerKatDev avatar Dec 14 '21 13:12 MeerKatDev

This package will be rafactored/rewritten in a few months, so I don't think it's worth adding a config file for it. That file would be shortlived anyway. Plus, I bet 6 months from now Jenssegers\Mongodb will still be the de-facto package used for MongoDB, so I don't think these 6 months anybody's going to need to configure that. If they do, they can say so and we'll create a config file then.

I think this PR is ok as it is (all things considered 😅). I'm just waiting for at least one more person to test this and give a thumbs up, and I'll merge it.

Cheers!

tabacitu avatar Dec 17 '21 13:12 tabacitu

Ok, so one other thing I noticed is that there is no casting support. When I normally work with Laravel and MongoDB, objects need to be kept objects, because Laravel converts them to string automatically, and same goes for dates - they should be saved as ISODates.

MeerKatDev avatar Dec 21 '21 15:12 MeerKatDev

so-how do i make this work? same issue, and im not seeing any resolution here as to what was done or what needs to be done to make it work with mongodb....

skylarr1227 avatar Nov 26 '22 18:11 skylarr1227

@skylarr1227 you can use this PR as composer require --dev backpack/generators:"dev-mongodb-support as 3.3.99"

I've just merged master here so everything is up-to-date with current version, plus this fixes. I am not a MongoDB user myself, I am concerned I didn't fully understand MongoDB concepts to confidently say this 100% works.

It seems to work, I've tested this PR and it allow to generate without the errors .. but I didn't fully understood some requirements mentioned here.

If you give this branch a go, and this works as you expect, please let us know, maybe it help us to merge this into core with more confidence.

Cheers

pxpm avatar Nov 28 '22 13:11 pxpm

Absolutely, ill try this later tonight when i get home. Sorry i missed you asking <3.

skylarr1227 avatar Dec 05 '22 21:12 skylarr1227

It's already been ages since this was put up (welp PR from 2021, last comment 2022) but hey. Here comes another tester! I just pretty much copied the changes and the first thing that came up was that apparently the \Jenssegers\Mongodb\Eloquent\Model has been moved to \MongoDB\Laravel\Eloquent\Model. After fixing that the commands execute without throwing exceptions.

Other than that the created Model is completely empty ofc and theres nothing to see on the crud.

ShuriZma avatar Jan 01 '24 18:01 ShuriZma

Hey @ShuriZma thanks for the feedback.

We don't "officially" support mongodb, but we aim to have non-blockers or solutions if people want to use it with mongo. AFAIK it works, at least from my simple tests.

Working on this kind of stuff is time consuming. I don't think it makes sense having mentions of mongodb and other engines that we don't "officially" support, and making sure that the crud functions work too.

What I would probably do here was just to either: a) create mongo db stubs in the package b) create a tutorial or some gist with those stubs and mention it in docs/readme

I would probably go with option b). As you can see as of today there are already changes related to MongoDB\Laravel\Eloquent\Model. Us not officially supporting it and really not using it, makes me wary of introducing specific mongo db code on our "supported" code. We did it in the past, I still regret it until today. It's not a proper solution, it's a patch and those usually come back to bit us in the future.

Sorry this haven't been prioritized, and I don't think this will be in a near future, I just don't wanted to let you without an answer.

So here goes, my proposed solution for people that need this stubs at the moment is to publish the backpack stubs and change them accordingly to your needs

If you use them in multiple projects you can put them in a package that requires generators and has the views changed, then instead of requiring backpack/generators you will require your/package-with-the-changes. Your package will install backpack generators and publish the changed stubs from your package.

If someone puts this in a package more people could benefit from it, if it's maintained we can promote it as our "recommended solution for mongo users".

Cheers

pxpm avatar Jan 11 '24 14:01 pxpm