Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable table names #218

Closed
LukeTowers opened this issue Oct 22, 2016 · 12 comments
Closed

Configurable table names #218

LukeTowers opened this issue Oct 22, 2016 · 12 comments
Assignees
Milestone

Comments

@LukeTowers
Copy link

Are there any plans to introduce configurable table names into this library? I'm working with an existing project that has a naming convention for the tables in the database, which means that I need to have these tables follow that strict naming convention.

I know that I could always just extend the model classes present with my own and modify the table names where applicable, but I would love it if the table names were settable through a config file. cmgmyr/laravel-messenger is an excellent example of how to implement this functionality and I would love for it to be present in this library.

If this functionality were to be implemented via a pull request, would it be accepted (assuming that it followed the guidelines of this repository)?

@Gummibeer
Copy link
Collaborator

We can implement it but atm I have not much time. If you create a PR pls do it for the version-4 branch. Version 3 isn't supported any longer with new Features.
If it works and doesn't conflict with something I will be happy to merge it.

@Gummibeer
Copy link
Collaborator

And the super best would be if you can add this new config to the documentation at https://github.com/Astrotomic/notifynder-documentation

@sakalauskas
Copy link
Contributor

sakalauskas commented Oct 28, 2016

If you have an issue only with "notifications" table renaming, and others are just fine, you can just use custom notification model where you would extend the Notifications class and override table name:


class Notification extends \Fenos\Notifynder\Models\Notification
{
    protected $table = 'XYZ_notifications';
}

However, it might not help you in this case.

@LukeTowers
Copy link
Author

Yeah, the issue I'm having is that all of the tables have to conform to the standard, which means that I would have to extend each of the models to replace the table names, and then also replace the relationship definitions to correspond to the new models, which is a bit more modifying of the library than I'd like to do ideally.

@Gummibeer Gummibeer added this to the v4.1 milestone Jan 26, 2017
@Gummibeer Gummibeer removed the Idea label Jan 26, 2017
@Gummibeer
Copy link
Collaborator

@LukeTowers can you pls take a look in the last commit dev-develop should be the corresponding composer version. Is this what you want?

$resolver = app('notifynder.resolver.model');
$resolver->setTable(Notification::class, 'prefixed_notifications');

@LukeTowers
Copy link
Author

@Gummibeer That could work, although I have to use 5.1 on my project. Does the current version of this library support 5.1?

@Gummibeer
Copy link
Collaborator

Yes it does. Take a look in the travis tests or the official doc page for more details.
We Support php 5.6+, L5.0+ and sqlite, mysql and postgresql.

@Gummibeer Gummibeer reopened this Feb 11, 2017
@Gummibeer
Copy link
Collaborator

@LukeTowers have you tested it? In my tests it's working. If it is also for you I will merge it into master and create a new release for it.

@LukeTowers
Copy link
Author

@Gummibeer Haven't tested it yet as my project is using the previous version of the library and I haven't upgraded because of breaking changes yet and lack of time to reconcile the implementation with the latest version. I'll try to do that sometime soon though

@Gummibeer
Copy link
Collaborator

Ok, cause of the successful tests and the, in my eyes, implemented requested feature - I will close this issue after release this new version.

@Gummibeer
Copy link
Collaborator

@LukeTowers
Copy link
Author

@Gummibeer Thanks. I'll let you know if anything goes wrong when implementing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants