Add database source

[raphcollective]

Hello people at whitecube!

Thanks again for your beautiful package. I spent some time drafting a Database source and wanted to share it with you. We plan on using your package for our projects and would have needed that source, might as well share it with everyone as I think it could be a common request.

It's not 100% complete as it's missing unit tests and locale handling, both of which I will need guidance/help on to finish.

I also added tags on the publishes functions calls in the provider as I added a migration and wanted to make the publishing optional so that people using the Filesystem source don't have to deal with it. Let me know if you were thinking of a different option or if that's good for you.

Also we might want to do something for the getParsedAttributes function as it's now duplicated across two files.

Please let me know what you think! Thanks again

[toonvandenbos]

Hi @raphcollective,

We'll take a look at your PR as soon as possible, it seems promising.

I personally think it's a good idea to add tags on the publishes method in order to make the migrations optional.

Could you explain why you'd prefer the use of this package with a Database Source instead of traditional Models & Resources? I guess the Page facade can come in handy, but I'm curious to see if there would be other reasons :)

[voidgraphics]

Salut Raph !

Thanks for the PR and for the kind words, it put a smile on our faces.

Can you clarify what you need help with in regards to locale handling? We can definitely help with the tests though.

Also, do you think you could write some documentation for your changes in the readme files? That would be greatly helpful.

Edit: do not worry if the travis checks fail, it is a configuration error on our end in regards to PRs, I need to fix that soon. It does not mean your changes have broken our tests.

[raphcollective]

Hi to you both!

First of all thank you for the very quick reply.

Could you explain why you'd prefer the use of this package with a Database Source instead of traditional Models & Resources? I guess the Page facade can come in handy, but I'm curious to see if there would be other reasons :)

The page facade is very handy but it's mainly the template layer that we are very interested in! We want different static pages to use different fields in nova without having to create a model for each. And your package allow us to do exactly that, we however prefer database over flat files hence this PR.

Can you clarify what you need help with in regards to locale handling?

It looks like the package is able to handle multiple locale, I wasn't sure where all of this was handled so I currently only store the locale. If you can explain a bit more how you are dealing with multiple locale editing/indexing I can start working on a better integration.

We can definitely help with the tests though.

To be honest I never dealt with database unit testing before, let me know if I can be of any help on that point.

Also, do you think you could write some documentation for your changes in the readme files? That would be greatly helpful.

Absolutely, I'll take care of that today

Thanks again

[voidgraphics]

It looks like the package is able to handle multiple locale, I wasn't sure where all of this was handled so I currently only store the locale. If you can explain a bit more how you are dealing with multiple locale editing/indexing I can start working on a better integration.

The package used to handle locales, however we removed that. I explained why and how we handle translated data here.

[raphcollective]

The package used to handle locales, however we removed that. I explained why and how we handle translated data here.

Sorry I missed that one. Should I remove the locale column from the table then?

[voidgraphics]

I think it would be best, yes.

You are free to maintain a fork if you really need to have the locale column in there - however I can think of a slight issue with that approach. Imagine you have a page where some of the fields are translatable, and some not; how do you deal with that non-translatable data, then? If I understand correctly, as your PR stands as of now, that data would need to be duplicated and stored multiple times, once for every locale your page has. Correct me if I'm wrong.

If this is the case, we think the other approach I linked above is cleaner as it removes that duplication entirely.

[raphcollective]

The locale column was there just temporarily as it was unclear to me how you were treating that. We do not need to handle locales and I agree with you on the points you mention.

I will update the PR soon to remove it

[toonvandenbos]

Also, in addition to what @voidgraphics said, if you're storing translatable fields in JSON columns, your current PR would already perfectly work for translated content since all attributes are stored in a JSON column.

The attributes column would contain something like :

{
    "my_regular_field_attribute": "An untranslated value",
    "my_translated_field_attribute": {
        "en": "An English value",
        "fr": "Une valeur en Français",
        "nl": "Een waarde in het Nederlands"
    } 
}

[raphcollective]

Every attribute is stored in a single JSON column currently and I'm using your getParsedAttribute function so it should be working as intended on that side.

[raphcollective]

Hi there,

I've updated the PR with the requested changes. I wasn't sure if you wanted me to update the root README as well as the documentation one so I only updated the latter as it looked like you wanted to keep the main one short. Tell me if you want me to add something to it as well.

Let me know if there is something that needs to be modified or if you need a clarification on anything.

Thanks!

[voidgraphics]

Awesome, thank you very much for your contribution.

[raphcollective]

@voidgraphics Thank you so much for the very quick merge (and sorry for the update mistake)!

I'll try to keep a look on the repo and give a hand on some issues if I can in the future :)