diff --git a/netbox/extras/models/staging.py b/netbox/extras/models/staging.py index a879ca3dc..0f2623bb2 100644 --- a/netbox/extras/models/staging.py +++ b/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, diff --git a/netbox/extras/views.py b/netbox/extras/views.py index 32c992a4f..eeb794564 100644 --- a/netbox/extras/views.py +++ b/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__}', diff --git a/netbox/netbox/views/generic/object_views.py b/netbox/netbox/views/generic/object_views.py index 215eebb8e..65910fc21 100644 --- a/netbox/netbox/views/generic/object_views.py +++ b/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()) diff --git a/netbox/templates/extras/reviewrequest.html b/netbox/templates/extras/reviewrequest.html index 193a72cf1..d0f0ddfb5 100644 --- a/netbox/templates/extras/reviewrequest.html +++ b/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 %} +
+
+ {% 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 %} +
+
+{% endblock %} {% block subtitle %}{% endblock %} {% block content %}
-
-
-
- Review Request -
-
- - - - - - - - - - - - - - - - - - - - - - - - - -
Created at - {{ object.created|annotated_date }} -
Last Updated - {{ object.last_updated|annotated_date }} -
Requester - {% if object.owner.get_full_name %} - {{ object.owner.get_full_name }} ({{ object.owner }}) - {% else %} - {{ object.owner }} - {% endif %} -
Reviewer - {% if object.reviewer.get_full_name %} - {{ object.reviewer.get_full_name }} ({{ object.reviewer }}) - {% else %} - {{ object.reviewer }} - {% endif %} -
Status - {{ object.status }} -
State - {{ object.state }} -
-
-
+
+
+
+ Review Request +
+
+ + + + + + + + + + + + + + + + + + + + + + + + + +
Created at + {{ object.created|annotated_date }} +
Last Updated + {{ object.last_updated|annotated_date }} +
Requester + {% if object.owner.get_full_name %} + {{ object.owner.get_full_name }} ({{ object.owner }}) + {% else %} + {{ object.owner }} + {% endif %} +
Reviewer + {% if object.reviewer.get_full_name %} + {{ object.reviewer.get_full_name }} ({{ object.reviewer }}) + {% else %} + {{ object.reviewer }} + {% endif %} +
Status + {{ object.status }} +
State + {{ object.state }} +
+
+
-
-
-
Staged Changes
-
- {% render_table staged_change_table 'inc/table.html' %} - {% include 'inc/paginator.html' with paginator=staged_change_table.paginator page=staged_change_table.page %} -
-
- {% plugin_full_width_page object %} +
+
+
Staged Changes
+
+ {% render_table staged_change_table 'inc/table.html' %} + {% include 'inc/paginator.html' with paginator=staged_change_table.paginator page=staged_change_table.page %} +
+ {% plugin_full_width_page object %} +
{% endblock %} \ No newline at end of file diff --git a/netbox/templates/generic/object.html b/netbox/templates/generic/object.html index 023726a30..6ffb942ec 100644 --- a/netbox/templates/generic/object.html +++ b/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 %} diff --git a/netbox/utilities/templates/buttons/approve.html b/netbox/utilities/templates/buttons/approve.html new file mode 100644 index 000000000..df86b5464 --- /dev/null +++ b/netbox/utilities/templates/buttons/approve.html @@ -0,0 +1,4 @@ +
+ {% csrf_token %} + +
\ No newline at end of file diff --git a/netbox/utilities/templates/buttons/deny.html b/netbox/utilities/templates/buttons/deny.html new file mode 100644 index 000000000..df0354b2c --- /dev/null +++ b/netbox/utilities/templates/buttons/deny.html @@ -0,0 +1,4 @@ +
+ {% csrf_token %} + +
\ No newline at end of file diff --git a/netbox/utilities/templates/buttons/suggest.html b/netbox/utilities/templates/buttons/suggest.html new file mode 100644 index 000000000..a34752591 --- /dev/null +++ b/netbox/utilities/templates/buttons/suggest.html @@ -0,0 +1,3 @@ + +  Suggest Change + diff --git a/netbox/utilities/templatetags/buttons.py b/netbox/utilities/templatetags/buttons.py index bcdb099d8..07cf5f38a 100644 --- a/netbox/utilities/templatetags/buttons.py +++ b/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') diff --git a/netbox/utilities/templatetags/perms.py b/netbox/utilities/templatetags/perms.py index f1bbf7549..02fcff620 100644 --- a/netbox/utilities/templatetags/perms.py +++ b/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')