rahulhaque / laravel-filepond

Use FilePond the Laravel way.
MIT License
185 stars 31 forks source link

Added support for configurable Filepond table name #12

Closed dmalta closed 2 years ago

dmalta commented 2 years ago

I wanted a table called 'uploads' instead of 'fileponds'. With this you can configure the table name used in Filepond model in config/filepond.php.

rahulhaque commented 2 years ago

@dmalta can you update the FilepondServiceProvider to migrate with the table name from the config. Also please open another pull request as this got messed up on my end when I tried to revert.

dmalta commented 2 years ago

Hi @rahulhaque , I see you merged and then decided to revert it. Found any issues?

I did think about using the table name from config in the migration, but then I considered it would make the migration flow too vulnerable to accidental changes in the configuration. Even different developers with different.env would get incompatible results. I realized the user would need to run a migration anyway, so would be just natural to change it there and their decision would be kept in the app repository.

rahulhaque commented 2 years ago

Hello @dmalta, welcome to my little package and thanks for the pull request. Yes, I merged it and thought of releasing another version for it. You wanted to change the table name for upload tracking. But, I was concerned about the use cases and consistency. For new installs, if someone changes the table name before migration, it won't be reflected in the database making it inconsistent. It will be more confusing for a model name Filepond to have table name something else rather than fileponds breaking the convention. For the use cases, please explain any where you would like to (or you're forced to) change the table name after migration.

dmalta commented 2 years ago

Hi @rahulhaque , thanks for considering. This feature is actually very important to me. In my current project a large team cooperates and we have conventions for table naming. In a broader sense, I personally think "fileponds" is very implementation specific. There is nothing wrong with that, but I don't like to mention client side component names in the database. I don't think the Laravel model having a different table name is a big deal. Specially because the it's a component model, not under App/Models.

dmalta commented 2 years ago

By the way, if you allow me a feedback, I love your overall solution design, but I think having two different classes named Filepond is confusing. In my code I had to name variables differently because it was never clear what is a Filepond model and a Filepond service.

rahulhaque commented 2 years ago

Hello @dmalta, feedback are always welcome. There's always opportunity to see things from another perspective. However, if I allow you to change table name after migration, it will raise some new test cases for me and make future development complex. I agree with you about the service and model name being same. But that's why php namespacing is there and laravel has a tendency to use same name for different classes if require. Even I had to name variable my own model at an early stage of developing this package. You can use DB::table('fileponds') to avoid that. If it feels too odd to have fileponds table with client side naming, please bear with it. I am really sorry that I cannot merge this at the moment. I hope you'll understand. :) If you allow me I will close the conversation right after I hear from you. Happy coding.

dmalta commented 2 years ago

That's alright, no problem. I will keep using my own forked version for now. That's one of the beauties of FOSS. ;-) I really appreciate your work here. Thank you.

rahulhaque commented 2 years ago

@dmalta I assure you I will keep your use case in mind. Maybe one day I will face the same scenario as you. It was nice having the discussion btw. Thank you.

rahulhaque commented 1 year ago

Hello @dmalta, hope you're doing well. I've just added support for Filepond model extending through config file. This will allow you to set a table name of your choice from custom model with protected $table = 'table_name'. The discussion is available here #45. Hope this come in handy for you. Thanks.