-
Notifications
You must be signed in to change notification settings - Fork 884
nat/npt: Refactor network prefix translation view #9504
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
base: master
Are you sure you want to change the base?
Conversation
…llowed and there is no inversion allowed
| $filter_funct = function ($record) use ($category) { | ||
| return empty($category) || array_intersect(explode(',', $record->categories), $category); | ||
| /* categories are indexed by name in the record, but offered as uuid in the selector */ | ||
| $catids = !empty((string)$record->categories) ? explode(',', (string)$record->categories) : []; |
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.
string? :D explode? :D
getValues()
| return empty($category) || array_intersect(explode(',', $record->categories), $category); | ||
| /* categories are indexed by name in the record, but offered as uuid in the selector */ | ||
| $catids = !empty((string)$record->categories) ? explode(',', (string)$record->categories) : []; | ||
| return empty($category) || array_intersect($catids, $category); |
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.
if you cast to array please use count()
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.
Can I fix this in a separate PR, because the same pattern is used in 3 or 4 other controllers.
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.
okay
| foreach ($results['rows'] as &$record) { | ||
| /* offer list of colors to be used by the frontend */ | ||
| $record['category_colors'] = $this->getCategoryColors( | ||
| !empty($record['categories']) ? explode(',', $record['categories']) : [] |
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 are we getting array keys here instead of objects? these constructs handling multi value conversions are not good as they bloat the controller code
Uses two methods (toggle log and move rule) that I generalized into base helper methods in this PR: #9492