Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: facilitate FE display of environment version from audit log #4077

Merged
merged 4 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions api/audit/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class Meta:
"environment",
"project",
"related_object_id",
"related_object_uuid",
"related_object_type",
"is_system_event",
)
Expand Down Expand Up @@ -52,6 +53,7 @@ class Meta:
"environment",
"project",
"related_object_id",
"related_object_uuid",
"related_object_type",
"is_system_event",
"change_details",
Expand Down
7 changes: 7 additions & 0 deletions api/features/versioning/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ def has_object_permission(
)


class EnvironmentFeatureVersionRetrievePermissions(BasePermission):
def has_object_permission(self, request, view, obj):
return request.user.has_environment_permission(
VIEW_ENVIRONMENT, obj.environment
)


class EnvironmentFeatureVersionFeatureStatePermissions(BasePermission):
def has_permission(self, request: Request, view: GenericViewSet) -> bool:
environment_pk = view.kwargs["environment_pk"]
Expand Down
6 changes: 6 additions & 0 deletions api/features/versioning/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from features.versioning.views import (
EnvironmentFeatureVersionFeatureStatesViewSet,
EnvironmentFeatureVersionRetrieveAPIView,
EnvironmentFeatureVersionViewSet,
)

Expand Down Expand Up @@ -34,4 +35,9 @@
"environments/<int:environment_pk>/features/<int:feature_pk>/",
include(ef_version_fs_router.urls),
),
path(
"environment-feature-versions/<str:pk>/",
EnvironmentFeatureVersionRetrieveAPIView.as_view(),
name="get-efv-by-uuid",
),
]
20 changes: 18 additions & 2 deletions api/features/versioning/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@
from django.utils.decorators import method_decorator
from drf_yasg.utils import swagger_auto_schema
from rest_framework.decorators import action
from rest_framework.generics import RetrieveAPIView
from rest_framework.mixins import (
CreateModelMixin,
DestroyModelMixin,
ListModelMixin,
RetrieveModelMixin,
UpdateModelMixin,
)
from rest_framework.permissions import IsAuthenticated
Expand All @@ -26,6 +26,7 @@
from features.versioning.permissions import (
EnvironmentFeatureVersionFeatureStatePermissions,
EnvironmentFeatureVersionPermissions,
EnvironmentFeatureVersionRetrievePermissions,
)
from features.versioning.serializers import (
EnvironmentFeatureVersionFeatureStateSerializer,
Expand All @@ -46,7 +47,6 @@
)
class EnvironmentFeatureVersionViewSet(
GenericViewSet,
RetrieveModelMixin,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in the description, this is technically a breaking API change but, since the versioning feature has not been released to anyone yet, I don't see this as an issue.

ListModelMixin,
CreateModelMixin,
DestroyModelMixin,
Expand Down Expand Up @@ -135,6 +135,22 @@ def publish(self, request: Request, **kwargs) -> Response:
return Response(serializer.data)


class EnvironmentFeatureVersionRetrieveAPIView(RetrieveAPIView):
"""
This is an additional endpoint to retrieve a specific version without needing
to provide the environment or feature as part of the URL.
"""

permission_classes = [
IsAuthenticated,
EnvironmentFeatureVersionRetrievePermissions,
]
serializer_class = EnvironmentFeatureVersionRetrieveSerializer

def get_queryset(self):
return EnvironmentFeatureVersion.objects.all()


class EnvironmentFeatureVersionFeatureStatesViewSet(
GenericViewSet,
ListModelMixin,
Expand Down
40 changes: 40 additions & 0 deletions api/tests/unit/audit/test_unit_audit_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@
from rest_framework import status
from rest_framework.test import APIClient

from audit.constants import ENVIRONMENT_FEATURE_VERSION_PUBLISHED_MESSAGE
from audit.models import AuditLog
from audit.related_object_type import RelatedObjectType
from environments.models import Environment
from features.models import Feature
from features.versioning.models import EnvironmentFeatureVersion
from organisations.models import Organisation, OrganisationRole
from projects.models import Project
from users.models import FFAdminUser


def test_audit_log_can_be_filtered_by_environments(
Expand Down Expand Up @@ -146,3 +151,38 @@ def test_admin_user_cannot_list_audit_log_of_another_organisation(

# Then
assert response.json()["count"] == 0


def test_retrieve_environment_feature_version_published_audit_log_record_includes_required_fields(
admin_client: APIClient,
admin_user: FFAdminUser,
environment_v2_versioning: Environment,
feature: Feature,
) -> None:
# Given
new_version = EnvironmentFeatureVersion.objects.create(
feature=feature,
environment=environment_v2_versioning,
)
new_version.publish(published_by=admin_user)

audit_log = (
AuditLog.objects.filter(related_object_type=RelatedObjectType.EF_VERSION.name)
.order_by("-created_date")
.first()
)
url = reverse("api-v1:audit-detail", args=[audit_log.id])

# When
response = admin_client.get(url)

# Then
assert response.status_code == status.HTTP_200_OK

response_json = response.json()
assert response_json["related_object_uuid"] == str(new_version.uuid)
assert response_json["related_object_type"] == RelatedObjectType.EF_VERSION.name
assert (
response_json["log"]
== ENVIRONMENT_FEATURE_VERSION_PUBLISHED_MESSAGE % feature.name
)
34 changes: 20 additions & 14 deletions api/tests/unit/features/versioning/test_unit_versioning_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,23 @@ def test_delete_feature_version(
assert environment_feature_version.deleted is True


def test_retrieve_environment_feature_version_permission_denied(
feature: Feature,
environment_v2_versioning: Environment,
staff_client: APIClient,
) -> None:
# Given
efv = EnvironmentFeatureVersion.objects.first()

url = reverse("api-v1:versioning:get-efv-by-uuid", args=[str(efv.uuid)])

# When
response = staff_client.get(url)

# Then
assert response.status_code == status.HTTP_403_FORBIDDEN


def test_retrieve_feature_version_with_no_previous_version(
feature: Feature,
environment_v2_versioning: Environment,
Expand All @@ -139,12 +156,7 @@ def test_retrieve_feature_version_with_no_previous_version(
)

url = reverse(
"api-v1:versioning:environment-feature-versions-detail",
args=[
environment_v2_versioning.id,
feature.id,
environment_feature_version.uuid,
],
"api-v1:versioning:get-efv-by-uuid", args=[environment_feature_version.uuid]
)

with_environment_permissions([VIEW_ENVIRONMENT])
Expand Down Expand Up @@ -181,10 +193,7 @@ def test_retrieve_feature_version_with_previous_version(
)
version_2.publish(published_by=staff_user)

url = reverse(
"api-v1:versioning:environment-feature-versions-detail",
args=[environment_v2_versioning.id, feature.id, version_2.uuid],
)
url = reverse("api-v1:versioning:get-efv-by-uuid", args=[version_2.uuid])

# When
response = staff_client.get(url)
Expand Down Expand Up @@ -216,10 +225,7 @@ def test_retrieve_feature_version_for_unpublished_version(
feature=feature, environment=environment_v2_versioning
)

url = reverse(
"api-v1:versioning:environment-feature-versions-detail",
args=[environment_v2_versioning.id, feature.id, version_2.uuid],
)
url = reverse("api-v1:versioning:get-efv-by-uuid", args=[version_2.uuid])

# When
response = staff_client.get(url)
Expand Down