Repository: netbox-community/netbox
Pull request: Add device & vm to `FHRPGroupAssignmentFilterSet`
<!--
    Thank you for your interest in contributing to NetBox! Please note
    that our contribution policy requires that a feature request or bug
    report be opened for approval prior to filing a pull request. This
    helps avoid wasting time and effort on something that we might not
    be able to accept.

    Please indicate the relevant feature request or bug report below.
    IF YOUR PULL REQUEST DOES NOT REFERENCE AN ACCEPTED BUG REPORT OR
    FEATURE REQUEST, IT WILL BE MARKED AS INVALID AND CLOSED.
-->
### Fixes: #8858
<!--
    Please include a summary of the proposed changes below.
-->
Add `device` and `virtual_machine` to `FHRPGroupAssignmentFilterSet` along with tests.


Problem statement:
Allow filtering FHRP group assignments by device/VM
### NetBox version

v3.1.8

### Feature type

New functionality

### Proposed functionality

Currently there is no direct way to get all FHRP group assignments for a particular device using the API. Currently the only way to do this is to use the `interface_id` filter multiple times on `GET /ipam/fhrp-group-assignments/`.

The following parameters could be added to the FHRP group assignment filterset (taken from `/ipam/ip-address`):
* `virtual_machine`
* `virtual_machine_id`
* `device`
* `device_id`

### Use case

Get all FHRP group assignments for a particular device in order to configure them.

### Database changes

n/a

### External dependencies

n/a

Hints from issue discussion:
Could I be assigned to this?

Commit message: Add device & vm to `FHRPGroupAssignmentFilterSet`

Patch to review:
diff --git a/netbox/ipam/filtersets.py b/netbox/ipam/filtersets.py
index 53c589bb377..f5e54c3f422 100644
--- a/netbox/ipam/filtersets.py
+++ b/netbox/ipam/filtersets.py
@@ -681,11 +681,53 @@ class FHRPGroupAssignmentFilterSet(ChangeLoggedModelFilterSet):
         queryset=FHRPGroup.objects.all(),
         label='Group (ID)',
     )
+    device = MultiValueCharFilter(
+        method='filter_device',
+        field_name='name',
+        label='Device (name)',
+    )
+    device_id = MultiValueNumberFilter(
+        method='filter_device',
+        field_name='pk',
+        label='Device (ID)',
+    )
+    virtual_machine = MultiValueCharFilter(
+        method='filter_virtual_machine',
+        field_name='name',
+        label='Virtual machine (name)',
+    )
+    virtual_machine_id = MultiValueNumberFilter(
+        method='filter_virtual_machine',
+        field_name='pk',
+        label='Virtual machine (ID)',
+    )
 
     class Meta:
         model = FHRPGroupAssignment
         fields = ['id', 'group_id', 'interface_type', 'interface_id', 'priority']
 
+    def filter_device(self, queryset, name, value):
+        devices = Device.objects.filter(**{'{}__in'.format(name): value})
+        if not devices.exists():
+            return queryset.none()
+        interface_ids = []
+        for device in devices:
+            interface_ids.extend(device.vc_interfaces().values_list('id', flat=True))
+        return queryset.filter(
+            Q(interface_type=ContentType.objects.get_for_model(Interface), interface_id__in=interface_ids)
+        )
+
+    def filter_virtual_machine(self, queryset, name, value):
+        virtual_machines = VirtualMachine.objects.filter(**{'{}__in'.format(name): value})
+        if not virtual_machines.exists():
+            return queryset.none()
+        interface_ids = []
+        for vm in virtual_machines:
+            interface_ids.extend(vm.interfaces.values_list('id', flat=True))
+        return queryset.filter(
+            Q(interface_type=ContentType.objects.get_for_model(VMInterface), interface_id__in=interface_ids)
+        )
+
 
 class VLANGroupFilterSet(OrganizationalModelFilterSet):
     scope_type = ContentTypeFilter()
diff --git a/netbox/ipam/tests/test_filtersets.py b/netbox/ipam/tests/test_filtersets.py
index 4bb72dce25d..94c806d0def 100644
--- a/netbox/ipam/tests/test_filtersets.py
+++ b/netbox/ipam/tests/test_filtersets.py
@@ -1024,6 +1024,24 @@ def test_priority(self):
         params = {'priority': [10, 20]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
 
+    def test_device(self):
+        device = Device.objects.all()[0]
+
+        params = {'device': [device.name]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
+        params = {'device_id': [device.pk]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
+    def test_virtual_machine(self):
+        vm = VirtualMachine.objects.all()[0]
+
+        params = {'virtual_machine': [vm.name]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
+        params = {'virtual_machine_id': [vm.pk]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
 
 class VLANGroupTestCase(TestCase, ChangeLoggedFilterSetTests):
     queryset = VLANGroup.objects.all()


Code review comments:
diff_hunk: @@ -1024,6 +1024,24 @@ def test_priority(self):
         params = {'priority': [10, 20]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
 
+    def test_device(self):
+        device = Device.objects.all()[0]
+
path: netbox/ipam/tests/test_filtersets.py
text: @user1:
Using `first()` retrieves just a single object instead of querying everything, and better conveys the intent of the query.

```suggestion
        device = Device.objects.first()

```

diff_hunk: @@ -681,11 +681,53 @@ class FHRPGroupAssignmentFilterSet(ChangeLoggedModelFilterSet):
         queryset=FHRPGroup.objects.all(),
         label='Group (ID)',
     )
+    device = MultiValueCharFilter(
+        method='filter_device',
+        field_name='name',
+        label='Device (name)',
+    )
+    device_id = MultiValueNumberFilter(
+        method='filter_device',
+        field_name='pk',
+        label='Device (ID)',
+    )
+    virtual_machine = MultiValueCharFilter(
+        method='filter_virtual_machine',
+        field_name='name',
+        label='Virtual machine (name)',
+    )
+    virtual_machine_id = MultiValueNumberFilter(
+        method='filter_virtual_machine',
+        field_name='pk',
+        label='Virtual machine (ID)',
+    )
 
     class Meta:
         model = FHRPGroupAssignment
         fields = ['id', 'group_id', 'interface_type', 'interface_id', 'priority']
 
+    def filter_device(self, queryset, name, value):
+        devices = Device.objects.filter(**{'{}__in'.format(name): value})
+        if not devices.exists():
+            return queryset.none()
+        interface_ids = []
+        for device in devices:
+            interface_ids.extend(device.vc_interfaces().values_list('id', flat=True))
+        return queryset.filter(
+            Q(interface_type=ContentType.objects.get_for_model(Interface), interface_id__in=interface_ids)
+        )
+
+    def filter_virtual_machine(self, queryset, name, value):
+        virtual_machines = VirtualMachine.objects.filter(**{'{}__in'.format(name): value})
path: netbox/ipam/filtersets.py
text: @user1:
```suggestion
        virtual_machines = VirtualMachine.objects.filter(**{'f{name}__in': value})
```

diff_hunk: @@ -1024,6 +1024,24 @@ def test_priority(self):
         params = {'priority': [10, 20]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
 
+    def test_device(self):
+        device = Device.objects.all()[0]
+
+        params = {'device': [device.name]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
+        params = {'device_id': [device.pk]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
+    def test_virtual_machine(self):
+        vm = VirtualMachine.objects.all()[0]
+
+        params = {'virtual_machine': [vm.name]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
path: netbox/ipam/tests/test_filtersets.py
text: @user1:
```suggestion
```

diff_hunk: @@ -1024,6 +1024,24 @@ def test_priority(self):
         params = {'priority': [10, 20]}
         self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4)
 
+    def test_device(self):
+        device = Device.objects.all()[0]
+
+        params = {'device': [device.name]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
+        params = {'device_id': [device.pk]}
+        self.assertEqual(self.filterset(params, self.queryset).qs.count(), 3)
+
+    def test_virtual_machine(self):
+        vm = VirtualMachine.objects.all()[0]
+
path: netbox/ipam/tests/test_filtersets.py
text: @user1:
Same as above.

```suggestion
        vm = VirtualMachine.objects.first()
```

diff_hunk: @@ -681,11 +681,53 @@ class FHRPGroupAssignmentFilterSet(ChangeLoggedModelFilterSet):
         queryset=FHRPGroup.objects.all(),
         label='Group (ID)',
     )
+    device = MultiValueCharFilter(
+        method='filter_device',
+        field_name='name',
+        label='Device (name)',
+    )
+    device_id = MultiValueNumberFilter(
+        method='filter_device',
+        field_name='pk',
+        label='Device (ID)',
+    )
+    virtual_machine = MultiValueCharFilter(
+        method='filter_virtual_machine',
+        field_name='name',
+        label='Virtual machine (name)',
+    )
+    virtual_machine_id = MultiValueNumberFilter(
+        method='filter_virtual_machine',
+        field_name='pk',
+        label='Virtual machine (ID)',
+    )
 
     class Meta:
         model = FHRPGroupAssignment
         fields = ['id', 'group_id', 'interface_type', 'interface_id', 'priority']
 
+    def filter_device(self, queryset, name, value):
+        devices = Device.objects.filter(**{'{}__in'.format(name): value})
path: netbox/ipam/filtersets.py
text: @user1:
We're trying to move to [f-strings](https://realpython.com/python-f-strings/) for better readability.

```suggestion
        devices = Device.objects.filter(**{f'{name}__in': value})
```