actions on review request view, more tweaks

netbox/extras/models/staging.py

@@ -62,7 +62,6 @@ class Branch(ChangeLoggedModel):
         with transaction.atomic():
             for change in self.staged_changes.all():
                 change.apply()
-        self.staged_changes.all().delete()
 
 
 class StagedChange(ChangeLoggedModel):
@@ -187,6 +186,9 @@ class ReviewRequest(ChangeLoggedModel):
         related_name='assigned_review_requests'
     )
 
+    # TODO: Make sure its corresponding branch and staged
+    #       changes are deleted when this entry is deleted.
+    #       atm this is not happening.
     branch = models.ForeignKey(
         to=Branch,
         on_delete=models.CASCADE,

netbox/extras/views.py

@@ -11,7 +11,8 @@ from django_rq.queues import get_connection
 from rq import Worker
 
 from netbox.views import generic
-from extras.choices import ChangeActionChoices
+from extras.choices import ChangeActionChoices, \
+    ReviewRequestStateChoices, ReviewRequestStatusChoices
 from extras.models.staging import ReviewRequest
 from utilities.htmx import is_htmx
 from utilities.templatetags.builtins.filters import render_markdown
@@ -917,7 +918,7 @@ class ReviewRequestListView(generic.ObjectListView):
     table = tables.ReviewRequestTable
     actions = ('bulk_delete')
 
-    def get_queryset(self, request):
+    def get_queryset(self, _):
         return ReviewRequest.objects.filter(
             Q(owner__id=self.request.user.id) |
             Q(reviewer__id=self.request.user.id)
@@ -925,8 +926,9 @@ class ReviewRequestListView(generic.ObjectListView):
 
 
 @register_model_view(ReviewRequest)
-class ReviewRequestEditView(generic.ObjectView):
+class ReviewRequestView(generic.ObjectView):
     queryset = ReviewRequest.objects.all()
+    actions = ('delete', )
 
     def get_extra_context(self, request, instance):
         data = []
@@ -959,6 +961,46 @@ class ReviewRequestEditView(generic.ObjectView):
         }
 
 
+@register_model_view(ReviewRequest, 'approve')
+class ReviewRequestDeleteView(View):
+    queryset = ReviewRequest.objects.all()
+
+    def post(self, request, *args, **kwargs):
+        # TODO: Should this logic go into a form that can be
+        #       reused in the API?
+        id = kwargs.get('pk', 0)
+        qs = ReviewRequest.objects.filter(id=id)
+        rr = get_object_or_404(qs)
+        if rr.reviewer != request.user:
+            return HttpResponseForbidden()
+        rr.branch.merge()
+        rr.status = ReviewRequestStatusChoices.STATUS_CLOSED
+        rr.state = ReviewRequestStateChoices.STATE_APPROVED
+        rr.save()
+        return redirect(rr.get_absolute_url())
+
+
+@register_model_view(ReviewRequest, 'deny')
+class ReviewRequestDeleteView(View):
+    queryset = ReviewRequest.objects.all()
+
+    def post(self, request, *args, **kwargs):
+        id = kwargs.get('pk', 0)
+        qs = ReviewRequest.objects.filter(id=id)
+        rr = get_object_or_404(qs)
+        if rr.reviewer != request.user:
+            return HttpResponseForbidden()
+        rr.status = ReviewRequestStatusChoices.STATUS_CLOSED
+        rr.state = ReviewRequestStateChoices.STATE_DENIED
+        rr.save()
+        return redirect(rr.get_absolute_url())
+
+
+@register_model_view(ReviewRequest, 'delete')
+class ReviewRequestDeleteView(generic.ObjectDeleteView):
+    queryset = ReviewRequest.objects.all()
+
+
 def suggest_form_factory(obj_cls, form_cls):
     return type(
         f'dyn_suggest_{form_cls.__name__}',

netbox/netbox/views/generic/object_views.py

@@ -179,6 +179,8 @@ class ObjectEditView(GetReturnURLMixin, BaseObjectView):
         return getattr(self, 'is_suggest_view', False)
 
     def _create_review_request(self, request, obj, form):
+        # TODO: Validate that owner and reviewer can't be the same
+        #       user unless owner is a super user.
         obj_cls_name = self.queryset.model._meta.verbose_name
         owner = request.user
         now_utc = int(datetime.utcnow().timestamp())

netbox/templates/extras/reviewrequest.html

@@ -1,82 +1,96 @@
 {% extends 'generic/object.html' %}
 {% load helpers %}
 {% load plugins %}
+{% load buttons %}
+{% load perms %}
 {% load render_table from django_tables2 %}
 
 {% block title %}{{ object }}{% endblock %}
 
 {# ObjectChange does not support the default add/edit/delete controls #}
-{% block controls %}{% endblock %}
+{% block controls %}
+  <div class="controls">
+    <div class="control-group">
+      {% if object.status != 'closed' and request.user|is_reviewer:object %}
+        {% approve_button object %}
+        {% deny_button object %}
+      {% endif %}
+      {% if object.status != 'closed' and request.user|is_owner:object %}
+        {% delete_button object %}
+      {% endif %}
+    </div>
+  </div>
+{% endblock %}
 {% block subtitle %}{% endblock %}
 
 {% block content %}
 <div class="row mb-3">
-    <div class="col col-md-5">
+  <div class="col col-md-5">
-        <div class="card">
+    <div class="card">
-            <h5 class="card-header">
+      <h5 class="card-header">
-                Review Request
+        Review Request
-            </h5>
+      </h5>
-            <div class="card-body">
+      <div class="card-body">
-                <table class="table table-hover attr-table">
+        <table class="table table-hover attr-table">
-                    <tr>
+          <tr>
-                        <th scope="row">Created at</th>
+            <th scope="row">Created at</th>
-                        <td>
+            <td>
-                            {{ object.created|annotated_date }}
+              {{ object.created|annotated_date }}
-                        </td>
+            </td>
-                    </tr>
+          </tr>
-                    <tr>
+          <tr>
-                        <th scope="row">Last Updated</th>
+            <th scope="row">Last Updated</th>
-                        <td>
+            <td>
-                            {{ object.last_updated|annotated_date }}
+              {{ object.last_updated|annotated_date }}
-                        </td>
+            </td>
-                    </tr>
+          </tr>
-                    <tr>
+          <tr>
-                        <th scope="row">Requester</th>
+            <th scope="row">Requester</th>
-                        <td>
+            <td>
-                            {% if object.owner.get_full_name %}
+              {% if object.owner.get_full_name %}
-                              {{ object.owner.get_full_name }} ({{ object.owner }})
+                {{ object.owner.get_full_name }} ({{ object.owner }})
-                            {% else %}
+              {% else %}
-                              {{ object.owner }}
+                {{ object.owner }}
-                            {% endif %}
+              {% endif %}
-                        </td>
+            </td>
-                    </tr>
+          </tr>
-                    <tr>
+          <tr>
-                        <th scope="row">Reviewer</th>
+              <th scope="row">Reviewer</th>
-                        <td>
+              <td>
-                            {% if object.reviewer.get_full_name %}
+                {% if object.reviewer.get_full_name %}
-                              {{ object.reviewer.get_full_name }} ({{ object.reviewer }})
+                  {{ object.reviewer.get_full_name }} ({{ object.reviewer }})
-                            {% else %}
+                {% else %}
-                              {{ object.reviewer }}
+                  {{ object.reviewer }}
-                            {% endif %}
+                {% endif %}
-                        </td>
+              </td>
-                    </tr>
+          </tr>
-                    <tr>
+          <tr>
-                        <th scope="row">Status</th>
+            <th scope="row">Status</th>
-                        <td>
+            <td>
-                            {{ object.status }}
+              {{ object.status }}
-                        </td>
+            </td>
-                    </tr>
+          </tr>
-                    <tr>
+          <tr>
-                        <th scope="row">State</th>
+            <th scope="row">State</th>
-                        <td>
+            <td>
-                            {{ object.state }}
+              {{ object.state }}
-                        </td>
+            </td>
-                    </tr>
+          </tr>
-                </table>
+        </table>
-            </div>
+      </div>
-        </div>
     </div>
+  </div>
 </div>
 <div class="row">
-    <div class="col col-md-12">
+  <div class="col col-md-12">
-        <div class="card">
+    <div class="card">
-            <h5 class="card-header">Staged Changes</h5>
+      <h5 class="card-header">Staged Changes</h5>
-            <div class="card-body table-responsive">
+      <div class="card-body table-responsive">
-                {% render_table staged_change_table 'inc/table.html' %}
+        {% render_table staged_change_table 'inc/table.html' %}
-                {% include 'inc/paginator.html' with paginator=staged_change_table.paginator page=staged_change_table.page %}
+        {% include 'inc/paginator.html' with paginator=staged_change_table.paginator page=staged_change_table.page %}
-            </div>
+      </div>
-        </div>
-      {% plugin_full_width_page object %}
     </div>
+    {% plugin_full_width_page object %}
+  </div>
 </div>
 {% endblock %}

netbox/templates/generic/object.html

@@ -66,6 +66,9 @@ Context:
       {% if request.user|can_change:object %}
         {% edit_button object %}
       {% endif %}
+      {% if request.user|can_suggest:object %}
+        {% suggest_button object %}
+      {% endif %}
       {% if request.user|can_delete:object %}
         {% delete_button object %}
       {% endif %}

netbox/utilities/templates/buttons/approve.html

@@ -0,0 +1,4 @@
+<form action="{{ url }}" method="post">
+  {% csrf_token %}
+  <button type="submit" name="approve" class="btn btn-sm btn-success">Approve Change</button>
+</form>

netbox/utilities/templates/buttons/deny.html

@@ -0,0 +1,4 @@
+<form action="{{ url }}" method="post">
+  {% csrf_token %}
+  <button type="submit" name="deny" class="btn btn-sm btn-danger">Deny Change</button>
+</form>

netbox/utilities/templates/buttons/suggest.html

@@ -0,0 +1,3 @@
+<a href="{{ url }}" class="btn btn-sm btn-warning" role="button">
+    <span class="mdi mdi-pencil" aria-hidden="true"></span>&nbsp;Suggest Change
+</a>

netbox/utilities/templatetags/buttons.py

@@ -36,6 +36,36 @@ def edit_button(instance):
     }
 
 
+@register.inclusion_tag('buttons/suggest.html')
+def suggest_button(instance):
+    viewname = get_viewname(instance, 'suggest')
+    url = reverse(viewname, kwargs={'pk': instance.pk})
+
+    return {
+        'url': url,
+    }
+
+
+@register.inclusion_tag('buttons/approve.html')
+def approve_button(instance):
+    viewname = get_viewname(instance, 'approve')
+    url = reverse(viewname, kwargs={'pk': instance.pk})
+
+    return {
+        'url': url,
+    }
+
+
+@register.inclusion_tag('buttons/deny.html')
+def deny_button(instance):
+    viewname = get_viewname(instance, 'deny')
+    url = reverse(viewname, kwargs={'pk': instance.pk})
+
+    return {
+        'url': url,
+    }
+
+
 @register.inclusion_tag('buttons/delete.html')
 def delete_button(instance):
     viewname = get_viewname(instance, 'delete')

netbox/utilities/templatetags/perms.py

@@ -1,4 +1,7 @@
 from django import template
+from django.urls import NoReverseMatch, reverse
+from utilities.utils import get_viewname
+
 
 register = template.Library()
 
@@ -10,6 +13,15 @@ def _check_permission(user, instance, action):
     )
 
 
+def _check_view_exists(instance):
+    try:
+        viewname = get_viewname(instance, 'suggest')
+        reverse(viewname, kwargs={'pk': instance.pk})
+        return True
+    except NoReverseMatch:
+        return False
+
+
 @register.filter()
 def can_view(user, instance):
     return _check_permission(user, instance, 'view')
@@ -25,6 +37,22 @@ def can_change(user, instance):
     return _check_permission(user, instance, 'change')
 
 
+@register.filter()
+def can_suggest(user, instance):
+    # TODO: View check is temporary until we impl. this everywhere.
+    return _check_view_exists(instance) and _check_permission(user, instance, 'suggest')
+
+
+@register.filter()
+def is_owner(user, instance):
+    return instance.owner.id == user.id
+
+
+@register.filter()
+def is_reviewer(user, instance):
+    return instance.reviewer.id == user.id
+
+
 @register.filter()
 def can_delete(user, instance):
     return _check_permission(user, instance, 'delete')