Fixes #21711: Added support for filtering and viewing modules by their module type profile#21900
Conversation
|
@claude /review Only report on issues of potential concern; we don't need commentary on anything that doesn't require further consideration. |
|
Claude finished @pheus's task in 2m 13s —— View job Code Review
Two items worth flagging: 1.
The fix is logically correct: However, the PR description only mentions this change indirectly ("utility widget regression tests"). Given the broad impact, it would be worth explicitly calling this out as a pre-existing bug fix, and confirming no regressions appear in the UI for other fields that currently have 2. Hardcoded
self.assertIn('value="null"', html)This hardcodes the value of |
|
Re 1 This was a pain point when I was testing locally. I can drop it if its too far out of scope. Re 2 Is the preference to lookup the settings reference or to use the hardcoded string |
arthanson
left a comment
There was a problem hiding this comment.
Looks good, a few changes:
- Please take out the null option widget change - if desired open up a separate FR and PR for that as it is outside the scope of this issue.
- Use settings.FILTERS_NULL_CHOICE_VALUE in tests rather than the literal string. see test_filter_modifiers.py:110,131 - null_value = settings.FILTERS_NULL_CHOICE_VALUE, and test_filters.py:81,89 - same pattern
- I think it makes potentially makes sense to add this to the detail view as well, under Module Type. As we are filtering on it and in the table seems like it should probably be in the detail view for the module?
8f0be22 to
90bc40f
Compare
|
Changes made as requested |
| ### Profile | ||
|
|
||
| The [module type profile](./moduletypeprofile.md) associated with the selected module type. Module list views and the REST API can be filtered by this related field. | ||
|
|
There was a problem hiding this comment.
This should not have been added. There is no profile field on the Module model.
Fixes: #21711
Added support for filtering
/dcim/modules/by module type profile, matching the existing module type profile relationship onModuleType.Changes
profileandprofile_idfilters to the module list filterset.None/ unprofiled module types./dcim/modules/.