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(tasks): Create a different task to update environment document #2807

Merged
merged 5 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
28 changes: 3 additions & 25 deletions api/audit/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@
from api_keys.models import MasterAPIKey
from audit.related_object_type import RelatedObjectType
from projects.models import Project
from sse import (
send_environment_update_message_for_environment,
send_environment_update_message_for_project,
)

RELATED_OBJECT_TYPES = ((tag.name, tag.value) for tag in RelatedObjectType)

Expand Down Expand Up @@ -113,12 +109,9 @@ def add_project(self):
when="environment_document_updated",
is_now=True,
)
def process_environment_update(self):
self.update_environments_updated_at()
self.send_environments_to_dynamodb()
self.send_environment_update_message()

def update_environments_updated_at(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

This method feels like it could be named better since it's triggering another task to handle the environment update logic. It might also benefit from a docstring I guess?

Copy link
Member Author

Choose a reason for hiding this comment

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

But it does update the updated_at

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess I can change it back to process_environment_update

from environments.tasks import process_environment_update

environments_filter = Q()
if self.environment_id:
environments_filter = Q(id=self.environment_id)
Expand All @@ -129,19 +122,4 @@ def update_environments_updated_at(self):
updated_at=self.created_date
)

def send_environments_to_dynamodb(self):
from environments.models import Environment

Environment.write_environments_to_dynamodb(
environment_id=self.environment_id, project_id=self.project_id
)

def send_environment_update_message(self):
if self.environment_id:
environment = self.environment
# Because we updated the environment `updated_at` in the previous hook in bulk
# update it manually here to save a `refresh_from_db` call
environment.updated_at = self.created_date
send_environment_update_message_for_environment(environment)
else:
send_environment_update_message_for_project(self.project)
process_environment_update.delay(args=(self.id,))
21 changes: 21 additions & 0 deletions api/environments/tasks.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
from audit.models import AuditLog
from environments.dynamodb import DynamoEnvironmentWrapper
from environments.models import Environment
from sse import (
send_environment_update_message_for_environment,
send_environment_update_message_for_project,
)
from task_processor.decorators import register_task_handler


Expand All @@ -9,3 +14,19 @@ def rebuild_environment_document(environment_id: int):
if wrapper.is_enabled:
environment = Environment.objects.get(id=environment_id)
wrapper.write_environment(environment)


@register_task_handler()
def process_environment_update(audit_log_id: int):
audit_log = AuditLog.objects.get(id=audit_log_id)

# Send environment document to dynamodb
Environment.write_environments_to_dynamodb(
environment_id=audit_log.environment_id, project_id=audit_log.project_id
)

# send environment update message
if audit_log.environment_id:
send_environment_update_message_for_environment(audit_log.environment)
else:
send_environment_update_message_for_project(audit_log.project)
59 changes: 12 additions & 47 deletions api/tests/unit/audit/test_unit_audit_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,74 +160,39 @@ def test_audit_log_save_project_is_added_if_not_set(environment):
assert audit_log.project == environment.project


def test_creating_audit_logs_triggers_environment_update_message(environment, mocker):
def test_creating_audit_logs_creates_process_environment_update_task(
environment, mocker
):
# Given
send_environment_update_message_for_environment = mocker.patch(
"audit.models.send_environment_update_message_for_environment"
process_environment_update = mocker.patch(
"environments.tasks.process_environment_update"
)

# When
audit_log = AuditLog.objects.create(environment=environment)

# Then
send_environment_update_message_for_environment.assert_called_once_with(environment)
assert environment.updated_at == audit_log.created_date

process_environment_update.delay.assert_called_once_with(args=(audit_log.id,))

def test_creating_audit_logs_triggers_environment_update_message_for_environment_if_environment_is_present(
environment, mocker, project
):
# Given
send_environment_update_message_for_environment = mocker.patch(
"audit.models.send_environment_update_message_for_environment"
)
send_environment_update_message_for_project = mocker.patch(
"audit.models.send_environment_update_message_for_project"
)
# When
audit_log = AuditLog.objects.create(environment=environment, project=project)

# Then
send_environment_update_message_for_environment.assert_called_once_with(environment)
send_environment_update_message_for_project.assert_not_called()
# and
environment.refresh_from_db()
assert environment.updated_at == audit_log.created_date


def test_creating_audit_logs_triggers_environment_update_message_for_project_if_environment_is_not_present(
def test_creating_audit_logs_for_change_request_does_not_trigger_process_environment_update(
environment, mocker, project
):
# Given
send_environment_update_message_for_environment = mocker.patch(
"audit.models.send_environment_update_message_for_environment"
process_environment_update = mocker.patch(
"environments.tasks.process_environment_update"
)
send_environment_update_message_for_project = mocker.patch(
"audit.models.send_environment_update_message_for_project"
)
# When
AuditLog.objects.create(project=project)

# Then
send_environment_update_message_for_project.assert_called_once_with(project)
send_environment_update_message_for_environment.assert_not_called()


def test_creating_audit_logs_for_change_request_does_not_trigger_environment_update_message(
environment, mocker, project
):
# Given
send_environment_update_message_for_environment = mocker.patch(
"audit.models.send_environment_update_message_for_environment"
)
send_environment_update_message_for_project = mocker.patch(
"audit.models.send_environment_update_message_for_project"
)
# When
audit_log = AuditLog.objects.create(
project=project,
related_object_type=RelatedObjectType.CHANGE_REQUEST.name,
)

# Then
send_environment_update_message_for_environment.assert_not_called()
send_environment_update_message_for_project.assert_not_called()
process_environment_update.delay.assert_not_called()
assert audit_log.created_date != environment.updated_at
64 changes: 63 additions & 1 deletion api/tests/unit/environments/test_unit_environment_tasks.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
from environments.tasks import rebuild_environment_document
from audit.models import AuditLog
from environments.tasks import (
process_environment_update,
rebuild_environment_document,
)


def test_rebuild_environment_document(environment, mocker):
Expand All @@ -13,3 +17,61 @@ def test_rebuild_environment_document(environment, mocker):

# Then
mock_dynamo_wrapper.write_environment.assert_called_once_with(environment)


def test_process_environment_update_with_environment_audit_log(environment, mocker):
# Given
audit_log = AuditLog.objects.create(
project=environment.project, environment=environment
)
mock_environment_model_class = mocker.patch(
"environments.tasks.Environment", autospec=True
)
mock_send_environment_update_message_for_environment = mocker.patch(
"environments.tasks.send_environment_update_message_for_environment",
autospec=True,
)
mock_send_environment_update_message_for_project = mocker.patch(
"environments.tasks.send_environment_update_message_for_project",
autospec=True,
)

# When
process_environment_update(audit_log_id=audit_log.id)

# Then
mock_environment_model_class.write_environments_to_dynamodb.assert_called_once_with(
environment_id=environment.id, project_id=environment.project.id
)
mock_send_environment_update_message_for_environment.assert_called_once_with(
environment
)
mock_send_environment_update_message_for_project.assert_not_called()


def test_process_environment_update_with_project_audit_log(environment, mocker):
# Given
audit_log = AuditLog.objects.create(project=environment.project)
mock_environment_model_class = mocker.patch(
"environments.tasks.Environment", autospec=True
)
mock_send_environment_update_message_for_environment = mocker.patch(
"environments.tasks.send_environment_update_message_for_environment",
autospec=True,
)
mock_send_environment_update_message_for_project = mocker.patch(
"environments.tasks.send_environment_update_message_for_project",
autospec=True,
)

# When
process_environment_update(audit_log_id=audit_log.id)

# Then
mock_environment_model_class.write_environments_to_dynamodb.assert_called_once_with(
environment_id=None, project_id=environment.project.id
)
mock_send_environment_update_message_for_environment.assert_not_called()
mock_send_environment_update_message_for_project.assert_called_once_with(
environment.project
)