search

webfox / laravel-xero-oauth2

A Laravel integration for Xero using the Oauth 2.0 spec
MIT License
50 stars 32 forks source link

[6.x] Document Credential Storages and Refactored Auth User Store to support different guards #122

Closed JamesFreeman closed 6 days ago

JamesFreeman commented 1 month ago

I've made some changes to Auth User store, do take into account that users may be authenticated by a different guard.

I've also wrote some docs for the new Credential Storages: You can see a demo here:

https://jamesfreeman.github.io/laravel-xero-oauth2/credential-storage.html

hailwood commented 4 weeks ago

This looks good to me. One of the developers at work has been using the beta version and ran into an interesting scenario - they wanted a per user storage, but not directly on the user.

In this case it was - user belongs to organisation, so we want the organisations xero credentials.

I don't believe this is a case we're current covering right?

JamesFreeman commented 4 weeks ago

Yeah, thinking this through at the moment, users would not be able to provide a dynamic model selection.

I was thinking as a potential solution, users could provide a callback on the useModelStore for selection.

Xero::useModelStore(Team::class, fn($builder) => $builder->where('id', Auth::user()->currentTeamId));

If you're happy with this pr, let's get it merged in, and I'll create a new task for this. 👍

JamesFreeman commented 4 weeks ago

I've moved all code changes from this PR into #121 to make the AuthenicatedStore changes.

JamesFreeman commented 2 weeks ago

Yeah, thinking this through at the moment, users would not be able to provide a dynamic model selection.

I was thinking as a potential solution, users could provide a callback on the useModelStore for selection.

Xero::useModelStore(Team::class, fn($builder) => $builder->where('id', Auth::user()->currentTeamId));

If you're happy with this pr, let's get it merged in, and I'll create a new task for this. 👍

Do you have any thoughts on this @hailwood?

JamesFreeman commented 1 week ago

Hey @hailwood,

Have you had any thoughts on this PR and the comment above?