Add Config for Table name and Route Path

[shiroamada]

The update goal is allow user change the settings table name and route path. Even though normal case it will use default value. For my case, I use another settings package by spatie, and it conflict with backpack table name and route. Hence, I make this update, so allow other user to do some customisation in their setting.

1. I added config/settings.php, allow to change the default table name and route.
2. Change migration to migration stub so can generated by current time
3. vendor:publish added publish config file, if not publish will use default value.

First PR to backpack.

[welcome[bot]]

BOOM! Your first PR with us, thank you so much! Someone will take a look at it shortly.

Please keep in mind that:

• if this constitutes a breaking change, it might take quite a while for this to get merged; we try to emulate the Laravel release cycle as much as possible, so developers can upgrade both software once; this means a new big release every ~6 months;
• even if it's a non-breaking change, it might take a few days/weeks for the PR to get merged; unless it's a no-brainer, we like to have some community feedback on new features, before we merge them; this leads to higher-quality code, in the end; we learnt this the hard way :-)
• not all PRs get merged; sometimes we just have to hold out new features, to keep the packages lean; sometimes we don't include features that only apply to niche use cases;
• we're not perfect; if you think we're wrong, call us out on it; but in a kind way :-) we all make mistakes, best we learn from them and build better software together;

Thank you!

-- Justin Case The Backpack Robot

[tabacitu]

Excellent first PR to Backpack @shiroamada ! Thank you 🙏 Very clean, very easy to understand.

I understand the use case, settings is a very general table name and it might conflict with other packages, so yes... we can do more to help devs tackle that problem if they get it. But since this is a problem we could also have with all other first-party Backpack addons, I think we should think about this a little bit more, and only implement a solution that we're also ok with implementing in the other packages as well.

From that perspective - if we do this here people will expect they're able to do the same thing inside PermissionManager, PageManager, NewsCRUD, MenuCRUD, etc - I'm wondering... should this problem be tackled as a feature (your PR does a great job at that) or as instructions?

Why am I saying that? Well to me... the devs in this position where settings conflict still need to jump through a few hoops to get it working, still need to follow some additional instructions:

• publish config file
• change variable in config file
• (only then) publish & run migration

So I'm wondering, is it worth adding that extra bulk to the package for everyone, to accommodate the fact that a few people need to change the table name? I don't have a definite answer or a strong opinion either way 😀 I'm brainstorming here - your input would be helpful.

---

What would be the alternative? How could we instruct those few people who need to change the table name to proceed, if we weren't to offer this as a feature? How would that look like?

Step 1. Create a new Setting model that extends the one in this package:

namespace App\Models;

use Backpack\Settings\app\Models\Setting as OriginalSetting;

class Setting extends OriginalSetting
{
    protected $table = 'your-new-table-name';
}

Step 2. Create a migration that changes the settings table name to whatever you want. Then run it (we should also provide code here so it's easy to copy-paste the entire migration including Schema::rename('settings', 'your-new-table-name');).

Step 3. Go to your app\Providers\AppServiceProvider::boot() and add:

$this->app->extend(\Backpack\Setting\app\Models::class, function () {
    return new \App\Models\Setting;
});

Now anytime you use the Setting model, you'll actually be using your model which gets data from the custom table name.

---

At first glance, I think this second option (instructions) has some PROs and CONs:

• CON: it's a little more work for the dev that needs it (3 steps instead of 2);
• CON: it makes it less transparent which model is actually being used by that CRUD (some might say it's "magic");
• PRO: doesn't affect the package code at all (or add complexity for devs who don't need it);
• PRO: changing something inside the modal is done the exact same way you would do for any other Backpack add-on (consistency);
• PRO: if the user needs to change anything else inside the Model (or CrudController) the instructions are exactly the same, and steps have already been made towards that; the next alteration is a lot easier;

I guess the first CON could be eliminated by making it a command that publishes all of this depending on the command line input, but then... it would also eliminate the first PRO 😂 Kind of.

---

I don't know, what do you think when weighing the options from this perspective @shiroamada ?

@pxpm how about you?

[pxpm]

I like the fact that there is an alternative, but I don't like the alternative.

What I mean is that the alternative is so much more work for those who want to change the table name, than having a config in code that does not mess with people using the default settings.

I think a lot of packages allow for table name customization, and I don't see why we shouldn't provide it.

One key point that @tabacitu mentioned is the fact that people would probably expect to be able to change table names in the other oficial Backpack packages, and, to be true, I don't think there is any reason (except for the work of doing), that we would not allow that to happen but ...

... I agree that pushing this change to all packages look like extra effort with no visible benefit, just for the case that in the future someone migth need to change table name, that was not requested until now.

On the other side, and as I mentioned before, lots of packages provide configs to override table names, because in the end, it's a simple config for people that need it other than overwritting models, mess with providers and stuff.

I would take a wild shoot here, since we know that spatie provides a settings package that is probably widely used, and we know that the name is conflicting with ours, and since @shiroamada did the heavy lifting here, I don't see why we wouldn't review and merge this.

Let me know @tabacitu

And thanks @shiroamada :)

Wish you guys the best, Pedro

[shiroamada]

1. Thanks for the the alternative way for another way doing this. I didn't think of this method, as we can extend/overwrite the current class. I think this will be hard way for beginner developer. This is good to include in the README for this package, if another developer would do it manual way and learn from this method.

2. First I was thinking put in config/backpack/base.php, but not everyone will use this settings package, so I create another file for handle this particular config.

3. For publish the config, from my view if anyone thinking for customise or change package variable, they will look for config first, this is the place to change any settings. Perhaps I should add inside the config/settings.php only applicable to Laravel-Backpack/Settings to make it clear.

[tabacitu]

Pff.... yeah ok you're right guys... thanks for talking sense into me 😅

Ok, let's have this 🎉

@pxpm could you please test and give your thumbs up / down?

[pxpm]

I am closing this in favor of #118