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

Change in the structure to use Dynamic Model #108

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lucassmacedo
Copy link
Contributor

I made some changes to the project structure so that it is possible to "extend" the use of the package model, thus making it possible to create new fields in the table.
I hope it is useful, I use this package in a SaaS and the need for the change was to add a column as account ID, hence the suggestion.

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 16, 2024

So this wouldn't be merged in as you've edited
src/Listeners/LoginListener.php and added in your Tenant model, which most people won't have.
src/Models/AuthenticationLog.php and added in tenant_id, which again, most people don't want.

Noting that if extending AuthenticationLog, then the type hinting should remain, as PHP will accept an extension of a Model.

If you remove the tenant references from the package, then it should be able to be merged in.

@lrljoe lrljoe added the invalid This doesn't seem right label Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants