Skip to content

Commit

Permalink
feat: Site and Project audit logs (#998)
Browse files Browse the repository at this point in the history
* fix: Added logger to Delete mutations
* feat: Added site delete and transfer audit logs
* feat: Added project delete audit log
  • Loading branch information
josebui authored Nov 28, 2023
1 parent a37123f commit 149c47a
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 13 deletions.
21 changes: 12 additions & 9 deletions terraso_backend/apps/graphql/schema/commons.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,18 @@ def not_found(cls, model=None, field=None, msg=None):
raise super().not_found(msg, field=field, model=model or cls.model_class)


class BaseWriteMutation(BaseAuthenticatedMutation):
class LoggerMixin:
logger: Optional[audit_log_api.AuditLog] = None

@classmethod
def get_logger(cls):
"""Returns the logger instance."""
if not cls.logger:
cls.logger = audit_log_services.new_audit_logger()
return cls.logger


class BaseWriteMutation(BaseAuthenticatedMutation, LoggerMixin):
skip_field_validation: Optional[str] = None

@classmethod
Expand Down Expand Up @@ -254,13 +264,6 @@ def mutate_and_get_payload(cls, root, info, **kwargs):
def is_update(cls, data):
return "id" in data

@classmethod
def get_logger(cls):
"""Returns the logger instance."""
if not cls.logger:
cls.logger = audit_log_services.new_audit_logger()
return cls.logger

@staticmethod
def remove_null_fields(kwargs, options=[str]):
"""It seems like for some fields, if the frontend does not pass an argument, the
Expand All @@ -278,7 +281,7 @@ def remove_null_fields(kwargs, options=[str]):
return kwargs


class BaseDeleteMutation(BaseAuthenticatedMutation):
class BaseDeleteMutation(BaseAuthenticatedMutation, LoggerMixin):
@classmethod
def mutate_and_get_payload(cls, root, info, **kwargs):
if "model_instance" in kwargs:
Expand Down
24 changes: 23 additions & 1 deletion terraso_backend/apps/graphql/schema/sites.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,13 +239,23 @@ class Input:
@classmethod
@transaction.atomic
def mutate_and_get_payload(cls, root, info, **kwargs):
log = cls.get_logger()
user = info.context.user
site_id = kwargs["id"]
site = cls.get_or_throw(Site, "id", site_id)
if not user.has_perm(Site.get_perm("delete"), site):
cls.not_allowed(MutationTypes.DELETE)

return super().mutate_and_get_payload(root, info, **kwargs)
result = super().mutate_and_get_payload(root, info, **kwargs)
metadata = {"project_id": str(site.project.id)} if site.project else {}
log.log(
user=user,
action=audit_log_api.DELETE,
resource=site,
metadata=metadata,
client_time=datetime.now(),
)
return result


class TransferredSite(graphene.ObjectType):
Expand All @@ -267,6 +277,7 @@ class Input:

@classmethod
def mutate_and_get_payload(cls, root, info, **kwargs):
log = cls.get_logger()
user = info.context.user

project = cls.get_or_throw(Project, "project_id", kwargs["project_id"])
Expand All @@ -291,6 +302,17 @@ def mutate_and_get_payload(cls, root, info, **kwargs):

Site.bulk_change_project(to_change, project)

log.log(
user=user,
action=audit_log_api.CHANGE,
resource=project,
metadata={
"project_id": str(project.id),
"transfered_sites": [str(site.id) for site in to_change],
},
client_time=datetime.now(),
)

return SiteTransferMutation(
project=project,
updated=[
Expand Down
12 changes: 12 additions & 0 deletions terraso_backend/apps/project_management/graphql/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ class Input:
@classmethod
@transaction.atomic
def mutate_and_get_payload(cls, root, info, **kwargs):
logger = cls.get_logger()
user = info.context.user
project_id = kwargs["id"]
project = cls.get_or_throw(Project, "id", project_id)
Expand All @@ -242,6 +243,17 @@ def mutate_and_get_payload(cls, root, info, **kwargs):
site.project = transfer_project
Site.objects.bulk_update(project_sites, ["project"])
result = super().mutate_and_get_payload(root, info, **kwargs)

logger.log(
user=user,
action=log_api.DELETE,
resource=result.project,
client_time=datetime.now(),
metadata={
"name": project.name,
"privacy": project.privacy,
},
)
return result


Expand Down
10 changes: 8 additions & 2 deletions terraso_backend/tests/graphql/mutations/test_projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from graphene_django.utils.testing import graphql_query
from mixer.backend.django import mixer

from apps.audit_logs.api import CHANGE, CREATE
from apps.audit_logs.api import CHANGE, CREATE, DELETE
from apps.audit_logs.models import Log
from apps.core.models.users import User
from apps.project_management.models import Project
Expand Down Expand Up @@ -88,7 +88,7 @@ def test_create_project(client, user):
"""


def test_delete_project(project_with_sites, client, project_manager):
def test_delete_project(project_with_sites, client, project_manager, project):
site_ids = [site.id for site in project_with_sites.site_set.all()]
input = {"id": str(project_with_sites.id)}
client.force_login(project_manager)
Expand All @@ -98,6 +98,12 @@ def test_delete_project(project_with_sites, client, project_manager):
assert not Project.objects.filter(id=project_with_sites.id).exists()
assert not Site.objects.filter(id__in=site_ids).exists()

logs = Log.objects.all()
assert len(logs) == 1
log_result = logs[0]
assert log_result.event == DELETE.value
assert log_result.resource_object == project


def test_delete_project_user_not_manager(project, client, project_user):
input = {"id": str(project.id)}
Expand Down
30 changes: 29 additions & 1 deletion terraso_backend/tests/graphql/mutations/test_sites.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from mixer.backend.django import mixer
from tests.utils import match_json

from apps.audit_logs.api import CHANGE, CREATE
from apps.audit_logs.api import CHANGE, CREATE, DELETE
from apps.audit_logs.models import Log
from apps.core.models import User
from apps.project_management.models import Project, Site
Expand Down Expand Up @@ -294,6 +294,26 @@ def linked_site(request, project_manager):
return site


@pytest.mark.parametrize("linked_site", ["manager"], indirect=True)
def test_delete_linked_site(client, linked_site, project_manager):
client.force_login(project_manager)
response = graphql_query(
DELETE_SITE_QUERY,
variables={"input": {"id": str(linked_site.id)}},
client=client,
)

assert response.json()["data"]["deleteSite"]["errors"] is None
assert len(Site.objects.filter(id=linked_site.id)) == 0

logs = Log.objects.all()
assert len(logs) == 1
log_result = logs[0]
assert log_result.event == DELETE.value
assert log_result.resource_object == linked_site
assert log_result.metadata["project_id"] == str(linked_site.project.id)


@pytest.mark.parametrize("linked_site", ["linked", "manager"], indirect=True)
def test_site_transfer_success(linked_site, client, project, project_manager):
input_data = {"siteIds": [str(linked_site.id)], "projectId": str(project.id)}
Expand All @@ -308,6 +328,14 @@ def test_site_transfer_success(linked_site, client, project, project_manager):
linked_site.refresh_from_db()
assert linked_site.project == project

logs = Log.objects.all()
assert len(logs) == 1
log_result = logs[0]
assert log_result.event == CHANGE.value
assert log_result.resource_object == project
assert log_result.metadata["project_id"] == str(project.id)
assert log_result.metadata["transfered_sites"] == [str(linked_site.id)]


def test_site_transfer_unlinked_site_user_contributor_success(client, user, site, project):
project.add_user_with_role(user, "contributor")
Expand Down

0 comments on commit 149c47a

Please sign in to comment.