From 32267af4f4acb7a80ed934e3d41ea0d93d740e88 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Thu, 4 Aug 2016 14:08:34 +0200 Subject: [PATCH] Refactoring epics API --- taiga/base/api/viewsets.py | 19 +++ taiga/base/routers.py | 53 ++++++- taiga/projects/epics/admin.py | 9 +- taiga/projects/epics/api.py | 131 +++++++++--------- taiga/projects/epics/models.py | 2 +- taiga/projects/epics/permissions.py | 14 +- taiga/projects/epics/serializers.py | 5 + taiga/projects/epics/validators.py | 11 +- taiga/routers.py | 8 +- .../test_epics_resources.py | 12 +- tests/integration/test_epics.py | 9 +- 11 files changed, 183 insertions(+), 90 deletions(-) diff --git a/taiga/base/api/viewsets.py b/taiga/base/api/viewsets.py index 95b09055..d37bfc50 100644 --- a/taiga/base/api/viewsets.py +++ b/taiga/base/api/viewsets.py @@ -134,6 +134,25 @@ class ViewSetMixin(object): return super().check_permissions(request, action=action, obj=obj) +class NestedViewSetMixin(object): + def get_queryset(self): + return self._filter_queryset_by_parents_lookups(super().get_queryset()) + + def _filter_queryset_by_parents_lookups(self, queryset): + parents_query_dict = self._get_parents_query_dict() + if parents_query_dict: + return queryset.filter(**parents_query_dict) + else: + return queryset + + def _get_parents_query_dict(self): + result = {} + for kwarg_name in self.kwargs: + query_value = self.kwargs.get(kwarg_name) + result[kwarg_name] = query_value + return result + + class ViewSet(ViewSetMixin, views.APIView): """ The base ViewSet class does not provide any actions by default. diff --git a/taiga/base/routers.py b/taiga/base/routers.py index 56b80f8e..a7ccbdc4 100644 --- a/taiga/base/routers.py +++ b/taiga/base/routers.py @@ -318,7 +318,58 @@ class DRFDefaultRouter(SimpleRouter): return urls -class DefaultRouter(DRFDefaultRouter): +class NestedRegistryItem(object): + def __init__(self, router, parent_prefix, parent_item=None): + self.router = router + self.parent_prefix = parent_prefix + self.parent_item = parent_item + + def register(self, prefix, viewset, base_name, parents_query_lookups): + self.router._register( + prefix=self.get_prefix(current_prefix=prefix, parents_query_lookups=parents_query_lookups), + viewset=viewset, + base_name=base_name, + ) + return NestedRegistryItem( + router=self.router, + parent_prefix=prefix, + parent_item=self + ) + + def get_prefix(self, current_prefix, parents_query_lookups): + return "{0}/{1}".format( + self.get_parent_prefix(parents_query_lookups), + current_prefix + ) + + def get_parent_prefix(self, parents_query_lookups): + prefix = "/" + current_item = self + i = len(parents_query_lookups) - 1 + while current_item: + prefix = "{parent_prefix}/(?P<{parent_pk_kwarg_name}>[^/.]+)/{prefix}".format( + parent_prefix=current_item.parent_prefix, + parent_pk_kwarg_name=parents_query_lookups[i], + prefix=prefix + ) + i -= 1 + current_item = current_item.parent_item + return prefix.strip("/") + + +class NestedRouterMixin: + def _register(self, *args, **kwargs): + return super().register(*args, **kwargs) + + def register(self, *args, **kwargs): + self._register(*args, **kwargs) + return NestedRegistryItem( + router=self, + parent_prefix=self.registry[-1][0] + ) + + +class DefaultRouter(NestedRouterMixin, DRFDefaultRouter): pass __all__ = ["DefaultRouter"] diff --git a/taiga/projects/epics/admin.py b/taiga/projects/epics/admin.py index af299f37..69aea806 100644 --- a/taiga/projects/epics/admin.py +++ b/taiga/projects/epics/admin.py @@ -24,10 +24,17 @@ from taiga.projects.votes.admin import VoteInline from . import models +class RelatedUserStoriesInline(admin.TabularInline): + model = models.RelatedUserStory + sortable_field_name = "order" + raw_id_fields = ["user_story", ] + extra = 0 + + class EpicAdmin(admin.ModelAdmin): list_display = ["project", "ref", "subject"] list_display_links = ["ref", "subject"] - inlines = [WatchedInline, VoteInline] + inlines = [WatchedInline, VoteInline, RelatedUserStoriesInline] raw_id_fields = ["project"] search_fields = ["subject", "description", "id", "ref"] diff --git a/taiga/projects/epics/api.py b/taiga/projects/epics/api.py index c14e7792..f98e4303 100644 --- a/taiga/projects/epics/api.py +++ b/taiga/projects/epics/api.py @@ -25,6 +25,7 @@ from taiga.base import exceptions as exc from taiga.base.decorators import list_route, detail_route from taiga.base.api import ModelCrudViewSet, ModelListViewSet from taiga.base.api.mixins import BlockedByProjectMixin +from taiga.base.api.viewsets import NestedViewSetMixin from taiga.base.utils import json from taiga.projects.history.mixins import HistoryResourceMixin @@ -108,17 +109,16 @@ class EpicViewSet(OCCResourceMixin, VotedResourceMixin, HistoryResourceMixin, super().pre_save(obj) - def _reorder_if_needed(self, obj, old_order_key, order_key, order_attr, project): + def _reorder_if_needed(self, obj, old_order_key, order_key): # Executes the extra ordering if there is a difference in the ordering keys if old_order_key != order_key: extra_orders = json.loads(self.request.META.get("HTTP_SET_ORDERS", "{}")) - data = [{"epic_id": obj.id, "order": getattr(obj, order_attr)}] + data = [{"epic_id": obj.id, "order": getattr(obj, "epics_order")}] for id, order in extra_orders.items(): data.append({"epic_id": int(id), "order": order}) return services.update_epics_order_in_bulk(data, - field=order_attr, - project=project) + project=obj.project) return {} def post_save(self, obj, created=False): @@ -126,9 +126,7 @@ class EpicViewSet(OCCResourceMixin, VotedResourceMixin, HistoryResourceMixin, # Let's reorder the related stuff after edit the element orders_updated = self._reorder_if_needed(obj, self._old_epics_order_key, - self._epics_order_key(obj), - "epics_order", - obj.project) + self._epics_order_key(obj)) self.headers["Taiga-Info-Order-Updated"] = json.dumps(orders_updated) super().post_save(obj, created) @@ -227,77 +225,78 @@ class EpicViewSet(OCCResourceMixin, VotedResourceMixin, HistoryResourceMixin, return response.BadRequest(validator.errors) - @detail_route(methods=["POST"]) - def bulk_create_related_userstories(self, request, **kwargs): + +class EpicRelatedUserStoryViewSet(NestedViewSetMixin, BlockedByProjectMixin, ModelCrudViewSet): + queryset = models.RelatedUserStory.objects.all() + serializer_class = serializers.EpicRelatedUserStorySerializer + validator_class = validators.EpicRelatedUserStoryValidator + model = models.RelatedUserStory + permission_classes = (permissions.EpicRelatedUserStoryPermission,) + + """ + Updating the order attribute can affect the ordering of another userstories in the epic + This method generate a key for the userstory and can be used to be compared before and after + saving + If there is any difference it means an extra ordering update must be done + """ + def _order_key(self, obj): + return "{}-{}".format(obj.user_story.project_id, obj.order) + + def pre_save(self, obj): + if not obj.id: + obj.epic_id = self.kwargs["epic"] + else: + self._old_order_key = self._order_key(self.get_object()) + + super().pre_save(obj) + + def _reorder_if_needed(self, obj, old_order_key, order_key): + # Executes the extra ordering if there is a difference in the ordering keys + if old_order_key != order_key: + extra_orders = json.loads(self.request.META.get("HTTP_SET_ORDERS", "{}")) + data = [{"us_id": obj.id, "order": getattr(obj, "order")}] + for id, order in extra_orders.items(): + data.append({"epic_id": int(id), "order": order}) + + return services.update_epic_related_userstories_order_in_bulk( + data, + epic=obj.epic + ) + + return {} + + def post_save(self, obj, created=False): + if not created: + # Let's reorder the related stuff after edit the element + orders_updated = self._reorder_if_needed(obj, + self._old_epics_order_key, + self._epics_order_key(obj)) + self.headers["Taiga-Info-Order-Updated"] = json.dumps(orders_updated) + + super().post_save(obj, created) + + @list_route(methods=["POST"]) + def bulk_create(self, request, **kwargs): validator = validators.CrateRelatedUserStoriesBulkValidator(data=request.DATA) if validator.is_valid(): data = validator.data - obj = self.get_object() - project = obj.project - self.check_permissions(request, 'bulk_create_userstories', project) + + epic = get_object_or_404(models.Epic, id=kwargs["epic"]) + project = epic.project + + self.check_permissions(request, 'bulk_create', project) if project.blocked_code is not None: raise exc.Blocked(_("Blocked element")) services.create_related_userstories_in_bulk( data["userstories"], - obj, + epic, project=project, owner=request.user ) - obj = self.get_queryset().get(id=obj.id) - epic_serialized = self.get_serializer_class()(obj) - return response.Ok(epic_serialized.data) - return response.BadRequest(validator.errors) - - @detail_route(methods=["POST"]) - def set_related_userstory(self, request, **kwargs): - validator = validators.SetRelatedUserStoryValidator(data=request.DATA) - if validator.is_valid(): - data = validator.data - epic = self.get_object() - project = epic.project - user_story = UserStory.objects.get(id=data["us_id"]) - self.check_permissions(request, "update", epic) - self.check_permissions(request, "select_related_userstory", user_story.project) - - if project.blocked_code is not None: - raise exc.Blocked(_("Blocked element")) - - obj, created = models.RelatedUserStory.objects.update_or_create( - epic=epic, - user_story=user_story, - defaults={ - "order": data["order"] - }) - epic = self.get_queryset().get(id=epic.id) - epic_serialized = self.get_serializer_class()(epic) - return response.Ok(epic_serialized.data) - - return response.BadRequest(validator.errors) - - @detail_route(methods=["POST"]) - def unset_related_userstory(self, request, **kwargs): - validator = validators.UnsetRelatedUserStoryValidator(data=request.DATA) - if validator.is_valid(): - data = validator.data - epic = self.get_object() - project = epic.project - user_story = UserStory.objects.get(id=data["us_id"]) - self.check_permissions(request, "update", epic) - - if project.blocked_code is not None: - raise exc.Blocked(_("Blocked element")) - - related_us = get_object_or_404( - models.RelatedUserStory, - epic=epic, - user_story=user_story - ) - related_us.delete() - epic = self.get_queryset().get(id=epic.id) - epic_serialized = self.get_serializer_class()(epic) - return response.Ok(epic_serialized.data) + related_uss_serialized = self.get_serializer_class()(epic.relateduserstory_set.all(), many=True) + return response.Ok(related_uss_serialized.data) return response.BadRequest(validator.errors) diff --git a/taiga/projects/epics/models.py b/taiga/projects/epics/models.py index eb18c6c4..de501ffc 100644 --- a/taiga/projects/epics/models.py +++ b/taiga/projects/epics/models.py @@ -105,4 +105,4 @@ class RelatedUserStory(models.Model): ordering = ["user_story", "order", "id"] def __str__(self): - return "{0} - {1}".format(self.epic, self.user_story) + return "{0} - {1}".format(self.epic_id, self.user_story_id) diff --git a/taiga/projects/epics/permissions.py b/taiga/projects/epics/permissions.py index 3cd31b07..fd473e18 100644 --- a/taiga/projects/epics/permissions.py +++ b/taiga/projects/epics/permissions.py @@ -34,14 +34,24 @@ class EpicPermission(TaigaResourcePermission): filters_data_perms = AllowAny() csv_perms = AllowAny() bulk_create_perms = HasProjectPerm('add_epic') - bulk_create_userstories_perms = HasProjectPerm('modify_epic') & (HasProjectPerm('add_us_to_project') | HasProjectPerm('add_us')) - select_related_userstory_perms = HasProjectPerm('view_us') upvote_perms = IsAuthenticated() & HasProjectPerm('view_epics') downvote_perms = IsAuthenticated() & HasProjectPerm('view_epics') watch_perms = IsAuthenticated() & HasProjectPerm('view_epics') unwatch_perms = IsAuthenticated() & HasProjectPerm('view_epics') +class EpicRelatedUserStoryPermission(TaigaResourcePermission): + enought_perms = IsProjectAdmin() | IsSuperUser() + global_perms = None + retrieve_perms = HasProjectPerm('view_epics') + create_perms = HasProjectPerm('modify_epic') + update_perms = HasProjectPerm('modify_epic') + partial_update_perms = HasProjectPerm('modify_epic') + destroy_perms = HasProjectPerm('modify_epic') + list_perms = AllowAny() + bulk_create_perms = HasProjectPerm('modify_epic') + + class EpicVotersPermission(TaigaResourcePermission): enought_perms = IsProjectAdmin() | IsSuperUser() global_perms = None diff --git a/taiga/projects/epics/serializers.py b/taiga/projects/epics/serializers.py index 9b845bbe..29a94348 100644 --- a/taiga/projects/epics/serializers.py +++ b/taiga/projects/epics/serializers.py @@ -78,3 +78,8 @@ class EpicSerializer(EpicListSerializer): class EpicNeighborsSerializer(NeighborsSerializerMixin, EpicSerializer): pass + + +class EpicRelatedUserStorySerializer(serializers.LightSerializer): + user_story = Field(attr="user_story_id") + order = Field() diff --git a/taiga/projects/epics/validators.py b/taiga/projects/epics/validators.py index 975f1e4d..88214aa6 100644 --- a/taiga/projects/epics/validators.py +++ b/taiga/projects/epics/validators.py @@ -60,10 +60,7 @@ class CrateRelatedUserStoriesBulkValidator(ProjectExistsValidator, EpicExistsVal userstories = serializers.CharField() -class SetRelatedUserStoryValidator(UserStoryExistsValidator, validators.Validator): - us_id = serializers.IntegerField() - order = serializers.IntegerField(required=False, default=10000) - - -class UnsetRelatedUserStoryValidator(UserStoryExistsValidator, validators.Validator): - us_id = serializers.IntegerField() +class EpicRelatedUserStoryValidator(validators.ModelValidator): + class Meta: + model = models.RelatedUserStory + read_only_fields = ('id', 'epic', 'user_story') diff --git a/taiga/routers.py b/taiga/routers.py index e0d0baa1..92f2d58c 100644 --- a/taiga/routers.py +++ b/taiga/routers.py @@ -146,6 +146,7 @@ from taiga.projects.milestones.api import MilestoneViewSet from taiga.projects.milestones.api import MilestoneWatchersViewSet from taiga.projects.epics.api import EpicViewSet +from taiga.projects.epics.api import EpicRelatedUserStoryViewSet from taiga.projects.epics.api import EpicVotersViewSet from taiga.projects.epics.api import EpicWatchersViewSet @@ -170,8 +171,11 @@ router.register(r"milestones", MilestoneViewSet, router.register(r"milestones/(?P\d+)/watchers", MilestoneWatchersViewSet, base_name="milestone-watchers") -router.register(r"epics", EpicViewSet, - base_name="epics") +router.register(r"epics", EpicViewSet, base_name="epics")\ + .register(r"related_userstories", EpicRelatedUserStoryViewSet, + base_name="epics-related-userstories", + parents_query_lookups=["epic"]) + router.register(r"epics/(?P\d+)/voters", EpicVotersViewSet, base_name="epic-voters") router.register(r"epics/(?P\d+)/watchers", EpicWatchersViewSet, diff --git a/tests/integration/resources_permissions/test_epics_resources.py b/tests/integration/resources_permissions/test_epics_resources.py index 89b56bb5..dff20c09 100644 --- a/tests/integration/resources_permissions/test_epics_resources.py +++ b/tests/integration/resources_permissions/test_epics_resources.py @@ -676,10 +676,10 @@ def test_epic_action_bulk_create(client, data): def test_bulk_create_related_userstories(client, data): - public_url = reverse('epics-bulk-create-related-userstories', kwargs={"pk": data.public_epic.pk}) - private_url1 = reverse('epics-bulk-create-related-userstories', kwargs={"pk": data.private_epic1.pk}) - private_url2 = reverse('epics-bulk-create-related-userstories', kwargs={"pk": data.private_epic2.pk}) - blocked_url = reverse('epics-bulk-create-related-userstories', kwargs={"pk": data.blocked_epic.pk}) + public_url = reverse('epics-related-userstories-bulk-create', args=[data.public_epic.pk]) + private_url1 = reverse('epics-related-userstories-bulk-create', args=[data.private_epic1.pk]) + private_url2 = reverse('epics-related-userstories-bulk-create', args=[data.private_epic2.pk]) + blocked_url = reverse('epics-related-userstories-bulk-create', args=[data.blocked_epic.pk]) users = [ None, @@ -698,9 +698,9 @@ def test_bulk_create_related_userstories(client, data): results = helper_test_http_method(client, 'post', private_url1, bulk_data, users) assert results == [401, 403, 403, 200, 200] results = helper_test_http_method(client, 'post', private_url2, bulk_data, users) - assert results == [404, 404, 404, 200, 200] + assert results == [401, 403, 403, 200, 200] results = helper_test_http_method(client, 'post', blocked_url, bulk_data, users) - assert results == [404, 404, 404, 451, 451] + assert results == [401, 403, 403, 451, 451] def test_set_related_user_story(client, data): diff --git a/tests/integration/test_epics.py b/tests/integration/test_epics.py index dd15a28d..01f1e483 100644 --- a/tests/integration/test_epics.py +++ b/tests/integration/test_epics.py @@ -98,7 +98,7 @@ def test_bulk_create_related_userstories(client): epic = f.EpicFactory.create(project=project) f.MembershipFactory.create(project=project, user=user, is_admin=True) - url = reverse('epics-bulk-create-related-userstories', kwargs={"pk": epic.pk}) + url = reverse('epics-related-userstories-bulk-create', args=[epic.pk]) data = { "userstories": "test1\ntest2" @@ -106,7 +106,7 @@ def test_bulk_create_related_userstories(client): client.login(user) response = client.json.post(url, json.dumps(data)) assert response.status_code == 200 - assert response.data['user_stories_counts'] == {'opened': 2, 'closed': 0} + assert len(response.data) == 2 def test_set_related_userstory(client): @@ -116,13 +116,14 @@ def test_set_related_userstory(client): f.MembershipFactory.create(project=epic.project, user=user, is_admin=True) f.MembershipFactory.create(project=us.project, user=user, is_admin=True) - url = reverse('epics-set-related-userstory', kwargs={"pk": epic.pk}) + url = reverse('epics-related-userstories-list', args=[epic.pk]) data = { - "us_id": us.id + "user_story": us.id } client.login(user) response = client.json.post(url, json.dumps(data)) + print(response.data) assert response.status_code == 200 assert response.data['user_stories_counts'] == {'opened': 1, 'closed': 0}