diff --git a/accounts/views.py b/accounts/views.py index 91e7829..47b72af 100644 --- a/accounts/views.py +++ b/accounts/views.py @@ -27,6 +27,8 @@ def create_account(request): email = req_data['email'] password = req_data['password'] except KeyError as e: + # One might argue that this response is a Presenter by itself + # But having the flexibility on mistyping "success" for example is a bit of a deal breaker for me. I'd rather have something which received the 3 params (success, message and status) and generated the messages return JsonResponse( { 'success': False, diff --git a/adapters/django_storage.py b/adapters/django_storage.py index 951ab10..a1a247a 100644 --- a/adapters/django_storage.py +++ b/adapters/django_storage.py @@ -14,6 +14,7 @@ class DjangoStorage(Storage): """Adapter to use Django ORM as a storage backend.""" def create_user(self, user, password): + # not injecting repo makes it less explicit when testing """Create user entity. Because Django provides password hashing functionality that we want to use, we perform this @@ -22,6 +23,7 @@ def create_user(self, user, password): django_user = accounts_models.User.objects.from_entity(user) django_user.set_password(password) django_user.save() + # nizar: unnecessary coupling with the return from the repo; return django_user.to_entity() def save_board(self, board): @@ -98,6 +100,7 @@ def delete_board_user(self, user_id, board_id): raise self.DoesNotExist('User {} is not joined to board {}'.format(user_id, board_id)) django_board_user.delete() + # nizar: interesting choice here, not sure why it returns this kind of object return django_board_user.asdict() def delete_board(self, id): diff --git a/adapters/memory_storage.py b/adapters/memory_storage.py index 4286b51..5d33bc8 100644 --- a/adapters/memory_storage.py +++ b/adapters/memory_storage.py @@ -5,6 +5,7 @@ from .storage import Storage +# nizar: very cool exercise for leveraging the advantages of having a common interface for a repo class MemoryStorage(Storage): """Adapter to use system memory as a storage backend.""" diff --git a/notes/actions.py b/notes/actions.py index 3210398..cd28c29 100644 --- a/notes/actions.py +++ b/notes/actions.py @@ -4,12 +4,13 @@ behavior to actions performaned by users. The examples we show are decorators for logging transation data and checking permissions on a per-action basis. """ +# This is a pretty cool concept, never had heard of it. from topsy.action_decorators import permission, log from .use_cases import NoteUseCases -class NoteActions(): +class NoteActions: def __init__(self, storage, logging): self.use_cases = NoteUseCases(storage) self.logging = logging diff --git a/notes/tests/test_use_cases.py b/notes/tests/test_use_cases.py index 5e00c64..8544a6e 100644 --- a/notes/tests/test_use_cases.py +++ b/notes/tests/test_use_cases.py @@ -4,6 +4,12 @@ As much as possible we will test use cases with MemoryStorage to enforce independence between the use cases and the Django ORM. """ +# Ohhhhhh, this was surprising! So the whole Memory Storage was not to make a point on showing the advantages on switching the repo in a easy way. It was to actually be able to use it in these tests. + +# The author mixes many types of test when testing the Use Cases/Interactors. He definitely takes advantage of the in memory storage, but we end up having a weird mixture of tests: we have component tests(which is awesome and the perfect use case for the in memory storage), integration tests that don't actually integrate and system tests which don't actually test the system. + +# Martin Fowler has a very cool diagram that defines the frontiers between the different kind of tests very well: https://martinfowler.com/articles/microservice-testing/meta-image.png + import unittest