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
Enabled dynamic loading of Stripe model #337
Conversation
Also, updated the README to better mach test setup conditions.
$model = getenv('STRIPE_MODEL') ?: config('services.stripe.model', 'User'); | ||
$foreignKey = Str::snake(class_basename($model)).'_id'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not create the model instance here and use getForeignKey()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would of course be safer. I made a commit to fix that.
@@ -234,7 +234,7 @@ protected function formatAmount($amount) | |||
public function view(array $data) | |||
{ | |||
return View::make('cashier::receipt', array_merge( | |||
$data, ['invoice' => $this, 'user' => $this->user] | |||
$data, ['invoice' => $this, 'owner' => $this->owner] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll need to still send user
to the view as well to not break backwards compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should send both owner
and user
even though they are the same variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. In case of manual changes to the invoice, developers still referring to the user
variable will have issues. Fixed that.
Any thoughts on feedback? |
This is a pull request related to proposal #334
I know that guidelines dictate that a proposal needs to be approved, but I think this PR will better explain the goal of the proposal.
One of the main changes other than the dynamic loading of the correct model and foreign key based on the configuration, I renamed a variable related to the model from
user
toowner
(in lack of a better word).For backward compatibility, the
Subscription::user
method remains, but utilizesSubscription::owner
.Also, updated the README to better match test setup conditions.