7025 review comments

docs/models/circuits/circuitgroupassignment.md

@@ -16,4 +16,4 @@ The [Circuit](./circuit.md) that is being assigned to the group.
 
 ### Priority
 
-A value between 0 and 255 indicating the circuit's priority for being elected as the master/primary node in the group.
+A selection (Primary, Secondary, Tertiary) indicating the circuit's priority for being elected as the master/primary node in the group.

netbox/circuits/api/nested_serializers.py

@@ -7,7 +7,6 @@ from netbox.api.serializers import WritableNestedSerializer
 
 __all__ = [
     'NestedCircuitSerializer',
-    'NestedCircuitGroupSerializer',
     'NestedCircuitTerminationSerializer',
     'NestedCircuitTypeSerializer',
     'NestedProviderNetworkSerializer',
@@ -81,10 +80,3 @@ class NestedCircuitTerminationSerializer(WritableNestedSerializer):
     class Meta:
         model = CircuitTermination
         fields = ['id', 'url', 'display_url', 'display', 'circuit', 'term_side', 'cable', '_occupied']
-
-
-class NestedCircuitGroupSerializer(WritableNestedSerializer):
-
-    class Meta:
-        model = CircuitGroup
-        fields = ['id', 'url', 'display_url', 'display', 'name']

netbox/circuits/api/serializers_/circuits.py

@@ -81,7 +81,8 @@ class CircuitTerminationSerializer(NetBoxModelSerializer, CabledObjectSerializer
 
 class CircuitGroupSerializer(NetBoxModelSerializer):
     # Related object counts
-    circuit_group_assignment_count = RelatedObjectCountField('circuit_group_assignments')
+    circuit_group_assignment_count = RelatedObjectCountField('assignments')
+    tenant = TenantSerializer(nested=True, required=False, allow_null=True)
 
     class Meta:
         model = CircuitGroup
@@ -95,7 +96,7 @@ class CircuitGroupSerializer(NetBoxModelSerializer):
 class CircuitGroupAssignmentSerializer(NetBoxModelSerializer):
     group = CircuitGroupSerializer(nested=True)
     circuit = CircuitSerializer(nested=True)
-    priority = ChoiceField(choices=CircuitPriorityChoices, allow_blank=True, required=False, default=lambda: '')
+    priority = ChoiceField(choices=CircuitPriorityChoices, allow_blank=True, required=False)
 
     class Meta:
         model = CircuitGroupAssignment

netbox/circuits/forms/bulk_edit.py

@@ -229,12 +229,6 @@ class CircuitGroupBulkEditForm(NetBoxModelBulkEditForm):
         max_length=200,
         required=False
     )
-
-    priority = forms.ChoiceField(
-        label=_('Priority'),
-        choices=add_blank_choice(CircuitPriorityChoices),
-        required=False
-    )
     tenant = DynamicModelChoiceField(
         label=_('Tenant'),
         queryset=Tenant.objects.all(),
@@ -243,7 +237,7 @@ class CircuitGroupBulkEditForm(NetBoxModelBulkEditForm):
 
     model = CircuitGroup
     nullable_fields = (
-        'priority', 'description', 'tenant',
+        'description', 'tenant',
     )
 
 

netbox/circuits/forms/model_forms.py

@@ -178,6 +178,11 @@ class CircuitTerminationForm(NetBoxModelForm):
 class CircuitGroupForm(TenancyForm, NetBoxModelForm):
     slug = SlugField()
 
+    fieldsets = (
+        FieldSet('name', 'slug', 'description', 'tags', name=_('Circuit Group')),
+        FieldSet('tenant_group', 'tenant', name=_('Tenancy')),
+    )
+
     class Meta:
         model = CircuitGroup
         fields = [
@@ -198,6 +203,5 @@ class CircuitGroupAssignmentForm(NetBoxModelForm):
     class Meta:
         model = CircuitGroupAssignment
         fields = [
-            'group', 'circuit', 'priority',
+            'group', 'circuit', 'priority', 'tags',
-            'tags',
         ]

netbox/circuits/migrations/0044_circuitgroup_circuitgroupassignment_and_more.py

@@ -35,7 +35,7 @@ class Migration(migrations.Migration):
                         blank=True,
                         null=True,
                         on_delete=django.db.models.deletion.PROTECT,
-                        related_name='circuitgroups',
+                        related_name='circuit_groups',
                         to='tenancy.tenant',
                     ),
                 ),
@@ -43,7 +43,7 @@ class Migration(migrations.Migration):
             options={
                 'verbose_name': 'Circuit group',
                 'verbose_name_plural': 'Circuit group',
-                'ordering': ('name', 'pk'),
+                'ordering': ('name',),
             },
         ),
         migrations.CreateModel(
@@ -61,7 +61,7 @@ class Migration(migrations.Migration):
                     'circuit',
                     models.ForeignKey(
                         on_delete=django.db.models.deletion.CASCADE,
-                        related_name='circuit_group_assignments',
+                        related_name='assignments',
                         to='circuits.circuit',
                     ),
                 ),
@@ -69,7 +69,7 @@ class Migration(migrations.Migration):
                     'group',
                     models.ForeignKey(
                         on_delete=django.db.models.deletion.CASCADE,
-                        related_name='circuit_group_assignments',
+                        related_name='assignments',
                         to='circuits.circuitgroup',
                     ),
                 ),

netbox/circuits/models/circuits.py

@@ -155,17 +155,18 @@ class Circuit(ContactsMixin, ImageAttachmentsMixin, PrimaryModel):
 
 class CircuitGroup(OrganizationalModel):
     """
+    An arbitrary collection of Circuits.
     """
     tenant = models.ForeignKey(
         to='tenancy.Tenant',
         on_delete=models.PROTECT,
-        related_name='circuitgroups',
+        related_name='circuit_groups',
         blank=True,
         null=True
     )
 
     class Meta:
-        ordering = ('name', 'pk')  # Name may be non-unique
+        ordering = ('name',)
         verbose_name = _('Circuit group')
         verbose_name_plural = _('Circuit group')
 
@@ -177,15 +178,18 @@ class CircuitGroup(OrganizationalModel):
 
 
 class CircuitGroupAssignment(CustomFieldsMixin, ExportTemplatesMixin, TagsMixin, ChangeLoggedModel):
+    """
+    Assignment of a Circuit to a CircuitGroup with an optional priority.
+    """
     circuit = models.ForeignKey(
         Circuit,
         on_delete=models.CASCADE,
-        related_name='circuit_group_assignments'
+        related_name='assignments'
     )
     group = models.ForeignKey(
         CircuitGroup,
         on_delete=models.CASCADE,
-        related_name='circuit_group_assignments'
+        related_name='assignments'
     )
     priority = models.CharField(
         verbose_name=_('priority'),

netbox/circuits/tables/circuits.py

@@ -140,10 +140,10 @@ class CircuitGroupTable(NetBoxTable):
     class Meta(NetBoxTable.Meta):
         model = CircuitGroup
         fields = (
-            'pk', 'name', 'circuit_group_assignment_count', 'tags',
+            'pk', 'name', 'description', 'circuit_group_assignment_count', 'tags',
             'created', 'last_updated', 'actions',
         )
-        default_columns = ('pk', 'name', 'circuit_group_assignment_count')
+        default_columns = ('pk', 'name', 'description', 'circuit_group_assignment_count')
 
 
 class CircuitGroupAssignmentTable(NetBoxTable):
@@ -162,6 +162,6 @@ class CircuitGroupAssignmentTable(NetBoxTable):
     class Meta(NetBoxTable.Meta):
         model = CircuitGroupAssignment
         fields = (
-            'pk', 'id', 'group', 'circuit', 'priority', 'created', 'last_updated', 'actions',
+            'pk', 'id', 'group', 'circuit', 'priority', 'created', 'last_updated', 'actions', 'tags',
         )
         default_columns = ('pk', 'group', 'circuit', 'priority')

netbox/templates/circuits/circuitgroup.html

@@ -28,6 +28,10 @@
             <th scope="row">{% trans "Name" %}</th>
             <td>{{ object.name }}</td>
           </tr>
+          <tr>
+            <th scope="row">{% trans "Description" %}</th>
+            <td>{{ object.description|placeholder }}</td>
+          </tr>
           <tr>
             <th scope="row">{% trans "Tenant" %}</th>
             <td>

netbox/templates/circuits/circuitgroupassignment.html

@@ -31,11 +31,8 @@
         </table>
       </div>
       {% include 'inc/panels/tags.html' %}
+      {% include 'inc/panels/custom_fields.html' %}
       {% plugin_left_page object %}
     </div>
-    <div class="col col-md-6">
-      {% include 'inc/panels/related_objects.html' %}
-      {% plugin_right_page object %}
-    </div>
   </div>
 {% endblock %}