Fix notices in tag admin code.#2249
Conversation
EreMaijala
left a comment
EreMaijala
left a comment
There was a problem hiding this comment.
While the fixes seem to work, I'd like a couple of tweaks to the current code. Can I ask them here while we're at it? :)
| @@ -198,18 +198,18 @@ protected function getConfirmDeleteMessages($count) | |||
| $user = $this->getTable('user') | |||
There was a problem hiding this comment.
I'd like an explicit null check for getParam return value for each type. Currently, in the unlikely case that the object selected gets deleted before the form is submitted, the current code results in "All", which is not right. I think the code should first check getParam(), and if it's set, either fetch the data or set the message to '' or something.
Later on, when adding the 'tag_delete_filter' message, I'd rather check for message strings other than 'All' rather than comparing them with false.
| ->current(); | ||
| $userMsg = (false !== $user) | ||
| $userMsg = is_object($user) | ||
| ? $user->username . " (" . $user->id . ")" : "All"; |
There was a problem hiding this comment.
"All" here doesn't have a leading space while the others do. Any reason for that?
|
Thanks, @EreMaijala -- I have merged the fixes as-is and will open a separate PR to address your suggestions (since they are not directly related to the work here). |
2 participants
This PR fixes some notices discovered while working on PR #2233. There were some incorrect assumptions about operator precedence and types which are corrected here.