search

TIPOFF / authorization

Laravel Package for opinionated authorization style
MIT License
0 stars 1 forks source link

Edit User Table Migration #42

Closed drewroberts closed 3 years ago

drewroberts commented 3 years ago

PR to find a better solution for previous sqlite workarounds.

drewroberts commented 3 years ago

@jasonmccreary Here was the article we referenced in the previous solution:

As we discussed in the other PR (#38), I would like to find a better approach to making this migration without the workarounds.

drewroberts commented 3 years ago

@pdbreen also discussed it in PR #34

drewroberts commented 3 years ago

@jasonmccreary Whenever you have time, feel free to push a commit to this PR with another solution. Thank you!

jasonmccreary commented 3 years ago

@drewroberts, will do. Should have time this evening.

jasonmccreary commented 3 years ago

@drewroberts, simply allowing them to be nullable is the easiest, and most pragmatic, solution considering this package could be added to an existing application with user records. As such, either defaulting the new columns or allowing them to be null are the only choices.

drewroberts commented 3 years ago

...either defaulting the new columns or allowing them to be null are the only choices.

That makes sense. I was hoping I wouldn't have to allow Null values, but I guess I can add application logic to handle that and throw an exception. Thank you, @jasonmccreary

jasonmccreary commented 3 years ago

@drewroberts, I hear you. You could make a secondary migration/seeder which audited the existing records and removed the nullable. However, that becomes a slippery slope. Many packages just make all new columns nullable. Cashier itself is an example.