createTicket respects alert/autorespond negatives #1435
Conversation
It was previously not possible to use the `alert` and `autorespond` flags in a negative way to prevent those two events, since the ternary operator would have used the `false` ternary value, which was `true`. This corrects that by allowing the default of `true` to be used only when the `$data` array does not have the appropriate keys set. Assigning `$data['source']` was altered to use the same formatting and caution against an undefined index error.
| $data['source'] = $data['source'] ? $data['source'] : 'API'; | ||
| $alert = array_key_exists('alert', $data) ? $data['alert'] : true; | ||
| $autorespond = array_key_exists('autorespond', $data) ? $data['autorespond'] : true; | ||
| $data['source'] = array_key_exists('source', $data) ? $data['source'] : 'API'; |
There was a problem hiding this comment.
This is unnecessary, right? You would never want to send false for the source, would you?
There was a problem hiding this comment.
$data['source'] was unrelated to the "false" fix. Since it was in the same code block, I thought it would have less code smell by formatting it in the same way while also preventing a possible undefined index error if the $data ever didn't include that value.
|
I'll advocate using |
|
In that case, it's probably better to type-cast the value; the only way I see to provide a NULL value is via the JSON payload. Different means to the same end. In the process of patching the patch, I think there's a case where an XML payload (according to the example in https://github.com/osTicket/osTicket-1.8/blob/develop/setup/doc/api.md ) will send a scalar "false" that should be interpreted as a boolean false too. I'll patch accordingly. |
- Allows the string "false" to be used in the XML payload as the documentation demonstrates, and have that be interpreted as a boolean false. - Switching $alert/$autorespond to use isset(), and forcing those variables to be type-casted to booleans.
|
@stevepacker thanks for the patch |
createTicket respects alert/autorespond negatives Reviewed-By: Peter Rotich <peter@osticket.com>
It was previously not possible to use the
alertandautorespondflags in a negative way to prevent those two events, since the ternary operator would have used thefalseternary value, which wastrue. This corrects that by allowing the default oftrueto be used only when the$dataarray does not have the appropriate keys set. Assigning$data['source']was altered to use the same formatting and caution against an undefined index error.