refactor #determine_order_user helper method

[gvaughn]

There's no behavior change to this. It allows subclasses to easily override how to determine the user.

Bonobos has a particular use case where one of our employees is guiding a customer through the creation of an order (and potentially user account too). We don't want the employee to be automatically found as current_api_user and this is a clean way to provide that hook.

[jhawthorn]

Seems fine to me 👍

Does it make sense to pass in parameters to determine_order_user? order_params is already available on the controller class. Seems like it would be simpler to use that directly.

[gvaughn]

@jhawthorn Yeah, I wasn't sure about the parameters parameter. I noticed that order_params and normalize_params performs some logic, so maybe it's just premature optimization to avoid running those multiple times. I'm open to changing this if you believe it would make the code clearer to read.

[magnusvk]

If we intend to allow for overriding this we should mark the method as // @api public so we know not to change it without deprecation warnings. Otherwise 👍 on this.

[gvaughn]

Thanks @magnusvk I wasn't aware of that convention. The comment is added now.

[magnusvk]

👍 from me. @jhawthorn are you ok with leaving the parameters as an argument or would you prefer that being pulled out?

[jhawthorn]

Yes, I would prefer it changed. If we're concerned about the time it takes to calculate order_params (I am not) we can memoize the method itself at a later date.

[gvaughn]

@jhawthorn Updated per your request

[jhawthorn]

Great 👍

[BenMorganIO]

I tend to add #present? incase the param is an empty string like if order_params[:user_id] = ''.

[gvaughn]

@BenMorganIO I added the #present? call

@magnusvk Would you have time this morning to merge this?

[magnusvk]

👍