[IDEA] Have WinkAuthor extend base User class about wink HOT 20 CLOSED

I feel like it might be a good idea to allow developers to change which model is considered the user model via the config file. By default, it could use the default Laravel User model.

Any extra meta datta about the users could then be stored in a seperate wink_user_meta table, with a 1-to-1 relationship to the configured user model.

from wink.

Any reason we couldn't just ship with a default "WinkUser" model which is defined in a config file,
An additional "WinkGuard" for auth, which is again defined in a config file,

Make the WinkUser model pretty much empty and move it's relationships etc out to a trait.

We could also have two sets of migrations, one to "update the users table" and one to "create wink_users" table.
Get the update migrations to get the users table name via (new config('wink.user_model'))->getTable()

Which set of migrations we publish would depend on whether wink.user_model points to the default WinkUser model or not.

Users can then, if they wish to use the default Laravel setup simply edit a config file

return [
    'user_model' => \App\User::class,
    'auth_guard' => 'web'
];

And apply a trait to their user model

class User extends Model
{
    use IsWinkUser;
}

from wink.

@hailwood I'm considering this yes for 1.0

from wink.

@idragon81 I just need to free sometime to take a deep look into it :) It has to be done in a way that doesn't break the current method of authentication.

from wink.

@themsaid My original intention was to just pose the question of how best Wink can integrate with Apps that already have defined user and authentication systems.

The idea being that Wink shouldn't make any assumptions about how users should be stored. If a site/app already has Users that own content pieces, it may be best to reuse those users instead of having them duplicated in wink_authors.

Or maybe an App/Site already has a User system where different user roles extend the base User class. In their system it may be best to define their own WinkAuthor that extends their base User. In systems like this the authentication might only ever use the User model and then trades the instance for role specific ones as needed.

IMHO, Wink would be most versatile and re-usable if it allowed this type of customization out of the box. For the user, it could be as simple as swapping the user in Laravel's main auth config file.

from wink.

Used wink for the first time to check it out today, great first pass @themsaid!

+1 for the trait idea under User IMO. We have an existing platform of a few thousand users with a simple enough User model, it uses Laravel Spark under the hood, so we're employing the Spark::developer() system, so possibly as a simple version, something like that to access /wink?

This probably undoes a lot of your great work in implementing the migration-based system Mohamed, sorry!

from wink.

@DivineOmega That's an interesting idea! I like the sounds of that.

from wink.

I wonder if now that Jetstream is out we could use that as a "base" - more like a control - system to design and implement a contract/trait for the WinkAuthor. This would provide a concrete/working example of how it's implemented.
And, in theory, could make it easier for other systems (backpack/voyager/etc) to integrate to Wink within a single project more cohesively. I think maybe even it would help with the Spark integrations some users seek too since AFAIK new versions of Spark will use Jetstream too?

from wink.

I think it's a good idea. Other approach would be to get a relation from WinkAuthor to a User automatically, but I think extending it would be better (not sure about using the same table though)

from wink.

@mallardduck Thanks. I think it would be a good way forward.

The only complication I can see is that the current wink_authors table uses a UUID primary key rather than a numeric auto-increment.

from wink.

Yeah i wanted to ask that to @themsaid also.

Why not using auto increment ids? They are supported in almost every database engine, it’s easy to read or check against and uses less size

from wink.

I used UUID as it's easier to reference to something while it's not still persisted in the database. I'm not using it in anyway now but just wanted to leave it flexible for the future.

Maybe if we switch to auto-increment keys would make linking between Wink Author and an existing user easier.

I'd love to make this link easier, however for example if you want to create a Wink user from Wink, but your user model requires much more data than the ones you can provide via Wink, things that can't be left null, what would you do?

from wink.

What would be a good example of that kind of data non-nullable but you can't fill on the first create step?

I've always worked with auto-increment ID both in work and in personal projects without any trouble.

I would like to use auto-increment in all the Wink tables, but that's my personal opinion 😅.

I used UUID as it's easier to reference to something while it's not still persisted in the database.

In which case do you would need to reference something not persisted yet? I can only think about the auto-save feature, but that's compatible with auto-increment ID as well. For example You can check if the post has an id, if not, create it and return the id of the object created, and next auto-save iterations just update that post with the returned id.

I don't know if I'm explaining it well enough since my english is not as good as I'd wanted to be, let me know if I screwed up 😅

from wink.

Is the problem we're trying to solve that Wink has a separate Authentication Middleware? So for example if you're logged in as App\User::1 and go to /wink you should be able to access without a second authentication?

If that's the only problem here I think we can make a bridge between the two auth guards. For example link between WinkAuthor and App\User via email, and check if current App\User has a WinkAuthor we let him in without authentication.

from wink.

I agree with using / extending Laravel's default auth system, or relating it to WinkAuthor which I still think it's necessary to store wink fields. Also using id instead of uuid for primary keys

from wink.

I agree, too.

Wink should not use a specific auth and leverage the use of the Laravel's default auth. Wink should only use the ID (or whatever property is used to identify the authenticated user) and use that to relate to posts and other wink specific tables.

Also, and after a little more reflection on my discussion with @Lloople on my PR (#22), about using a specific table to store the wink columns, I can also agree that it's a better way to not mess, directly, with the users' table, on any Laravel application. Although it adds a little bit of complexity it may payout by enabling more complex sites to use this package.

from wink.

@hailwood That sounds like a very solid idea to me. It's literally exactly on par with what I was imagining too. Such an elegant (sounding) solution.

from wink.

Any updates on this? I was wondering if this is being worked on in any branch as I'd like to contribute

from wink.

Brilliant - want to use Wink, but would be a bit much to ask someone to log in twice to the same application.
Brilliant starting point you have though @themsaid

from wink.

Has any headway been made on this front?

from wink.