Get quantity from request as an integer. #239
Conversation
|
Hey @ce-bo. Thanks for contribution. I would merge it but the tests are failing :( Can you adjust them as well? |
|
Hi @lchrusciel, the tests have passed now. Could you merge it please? |
| @@ -31,7 +31,7 @@ public function __construct(Request $request) | |||
| { | |||
| $this->token = $request->attributes->get('token'); | |||
| $this->id = $request->attributes->get('id'); | |||
| $this->quantity = $request->request->get('quantity'); | |||
| $this->quantity = $request->request->has('quantity') ? $request->request->getInt('quantity') : null; | |||
There was a problem hiding this comment.
Quantity cannot be null, it's declared as int. $request->request->getInt('quantity', 1) instead?
There was a problem hiding this comment.
$request->request->getInt('quantity', 0)? This should trigger validation error. Question is, what should be a default behaviour. 1 as a default would be more flexible for frontends...
There was a problem hiding this comment.
If we allow not to pass quantity then it makes sense to use 1 as the default value. If we require quantity then we should throw an exception here if it does not exist.
There was a problem hiding this comment.
Let's add 1 as default. But we need to mention it in docs.
There was a problem hiding this comment.
sure, but it will take some time
| @@ -44,7 +44,7 @@ public static function fromArray(array $item) | |||
|
|
|||
| public static function fromRequest(Request $request) | |||
| { | |||
| return new self($request->attributes->get('token'), $request->request->get('productCode'), $request->request->get('options'), $request->request->get('quantity')); | |||
| return new self($request->attributes->get('token'), $request->request->get('productCode'), $request->request->get('options'), $request->request->has('quantity') ? $request->request->getInt('quantity') : null); | |||
|
Hi @ce-bo, do you plan to continue on this PR? :) |
|
Hi @pjedrzejewski, unfortunately not. I already discussed with @lchrusciel that he would rather do it. |
|
Thanks @ce-bo for all your effort! And welcome to our community! 🎉 |
|
@lchrusciel thanks for finalizing! |
|
I will make a release this week |
PR does fix this issue: #235