From f49d143c22af4905d10d7c92d49a328df91e1306 Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Sat, 15 Nov 2014 01:39:29 +0100 Subject: [PATCH 01/10] Fixing neighbors hell --- taiga/base/neighbors.py | 115 ++++++---------------------- tests/integration/test_neighbors.py | 75 ++++++++---------- 2 files changed, 55 insertions(+), 135 deletions(-) diff --git a/taiga/base/neighbors.py b/taiga/base/neighbors.py index 14223487..7b4adbfb 100644 --- a/taiga/base/neighbors.py +++ b/taiga/base/neighbors.py @@ -19,63 +19,11 @@ from functools import partial from collections import namedtuple from django.db.models import Q - +from django.db import connection Neighbor = namedtuple("Neighbor", "left right") -def disjunction_filters(filters): - """From a list of queryset filters, it returns a disjunction (OR) Q object. - - :param filters: List of filters where each item is a dict where the keys are the lookups and - the values the values of those lookups. - - :return: :class:`django.db.models.Q` instance representing the disjunction of the filters. - """ - result = Q() - for filter in filters: - result |= Q(**{lookup: value for lookup, value in filter.items()}) - return result - - -def get_attribute(obj, attr): - """Finds `attr` in obj. - - :param obj: Object where to look for the attribute. - :param attr: Attribute name as a string. It can be a Django lookup field such as - `project__owner__name`, in which case it will look for `obj.project.owner.name`. - - :return: The attribute value. - :raises: `AttributeError` if some attribute doesn't exist. - """ - chunks = attr.lstrip("-").split("__") - attr, chunks = chunks[0], chunks[1:] - obj = value = getattr(obj, attr) - for path in chunks: - value = getattr(obj, path) - obj = value - - return value - - -def transform_field_into_lookup(name, value, operator="", operator_if_desc=""): - """From a field name and value, return a dict that may be used as a queryset filter. - - :param name: Field name as a string. - :param value: Field value. - :param operator: Operator to use in the lookup. - :param operator_if_desc: If the field is reversed (a "-" in front) use this operator - instead. - - :return: A dict that may be used as a queryset filter. - """ - if name.startswith("-"): - name = name[1:] - operator = operator_if_desc - lookup = "{}{}".format(name, operator) - return {lookup: value} - - def get_neighbors(obj, results_set=None): """Get the neighbors of a model instance. @@ -89,51 +37,32 @@ def get_neighbors(obj, results_set=None): """ if results_set is None or results_set.count() == 0: results_set = type(obj).objects.get_queryset() + + compiler = results_set.query.get_compiler('default') + base_sql, base_params = compiler.as_sql(with_col_aliases=True) + + query = """ + SELECT * FROM + (SELECT "id" as id, ROW_NUMBER() OVER() + FROM (%s) as ID_AND_ROW) + AS SELECTED_ID_AND_ROW + """%(base_sql) + query += " WHERE id=%s;" + params = list(base_params) + [obj.id] + + cursor = connection.cursor() + cursor.execute(query, params) + row = cursor.fetchone() + obj_position = row[1] - 1 + try: - left = _left_candidates(obj, results_set).reverse()[0] + left = obj_position > 0 and results_set[obj_position - 1] or None except IndexError: left = None + try: - right = _right_candidates(obj, results_set)[0] + right = results_set[obj_position + 1] except IndexError: right = None return Neighbor(left, right) - - -def _get_candidates(obj, results_set, reverse=False): - ordering = (results_set.query.order_by or []) + list(obj._meta.ordering) - main_ordering, rest_ordering = ordering[0], ordering[1:] - try: - filters = obj.get_neighbors_additional_filters(results_set, ordering, reverse) - if filters is None: - raise AttributeError - except AttributeError: - filters = [order_field_as_filter(obj, main_ordering, reverse)] - filters += [ordering_fields_as_filter(obj, main_ordering, rest_ordering, reverse)] - - return (results_set - .filter(~Q(id=obj.id), disjunction_filters(filters)) - .distinct() - .order_by(*ordering)) -_left_candidates = partial(_get_candidates, reverse=True) -_right_candidates = partial(_get_candidates, reverse=False) - - -def order_field_as_filter(obj, order_field, reverse=None, operator=None): - value = get_attribute(obj, order_field) - if reverse is not None: - if operator is None: - operator = ("__gt", "__lt") - operator = (operator[1], operator[0]) if reverse else operator - else: - operator = () - return transform_field_into_lookup(order_field, value, *operator) - - -def ordering_fields_as_filter(obj, main_order_field, ordering_fields, reverse=False): - """Transform a list of ordering fields into a queryset filter.""" - filter = order_field_as_filter(obj, main_order_field) - for field in ordering_fields: - filter.update(order_field_as_filter(obj, field, reverse, ("__gte", "__lte"))) - return filter diff --git a/tests/integration/test_neighbors.py b/tests/integration/test_neighbors.py index 3281dcee..8cb37e90 100644 --- a/tests/integration/test_neighbors.py +++ b/tests/integration/test_neighbors.py @@ -36,48 +36,6 @@ def teardown_module(): reconnect_signals() -class TestGetAttribute: - def test_no_attribute(self, object): - object.full_name = "name" - with pytest.raises(AttributeError): - n.get_attribute(object, "name") - - with pytest.raises(AttributeError): - n.get_attribute(object, "full_name__last_name") - - def test_one_level(self, object): - object.name = "name" - assert n.get_attribute(object, "name") == object.name - - def test_two_levels(self, object): - object.name = object - object.name.full_name = "first name" - assert n.get_attribute(object, "name__full_name") == object.name.full_name - - def test_three_levels(self, object): - object.info = object - object.info.name = object - object.info.name.full_name = "first name" - assert n.get_attribute(object, "info__name__full_name") == object.info.name.full_name - - -def test_transform_field_into_lookup(): - transform = partial(n.transform_field_into_lookup, value="chuck", operator="__lt", - operator_if_desc="__gt") - - assert transform(name="name") == {"name__lt": "chuck"} - assert transform(name="-name") == {"name__gt": "chuck"} - - -def test_disjunction_filters(): - filters = [{"age__lt": 21, "name__eq": "chuck"}] - result_str = str(n.disjunction_filters(filters)) - - assert result_str.startswith("(OR: ") - assert "('age__lt', 21)" in result_str - assert "('name__eq', 'chuck')" in result_str - - @pytest.mark.django_db class TestUserStories: def test_no_filters(self): @@ -338,3 +296,36 @@ class TestIssues: assert issue1_neighbors.right == issue3 assert issue2_neighbors.left == issue3 assert issue2_neighbors.right is None + + def test_ordering_by_assigned_to_desc_with_none_values(self): + project = f.ProjectFactory.create() + + issue1 = f.IssueFactory.create(project=project, assigned_to=None) + issue2 = f.IssueFactory.create(project=project, assigned_to=None) + issue3 = f.IssueFactory.create(project=project, assigned_to=None) + + issues = Issue.objects.filter(project=project).order_by("-assigned_to__full_name") + issue1_neighbors = n.get_neighbors(issue1, results_set=issues) + issue2_neighbors = n.get_neighbors(issue2, results_set=issues) + + assert issue1_neighbors.left == issue2 + assert issue1_neighbors.right is None + assert issue2_neighbors.left == issue3 + assert issue2_neighbors.right == issue1 + + def test_ordering_by_assigned_to_desc_with_none_and_normal_values(self): + project = f.ProjectFactory.create() + assigned_to1 = f.UserFactory.create(full_name="Chuck Norris") + issue1 = f.IssueFactory.create(project=project, assigned_to=None) + issue2 = f.IssueFactory.create(project=project, assigned_to=assigned_to1) + issue3 = f.IssueFactory.create(project=project, assigned_to=None) + + issues = Issue.objects.filter(project=project).order_by("-assigned_to__full_name") + + issue1_neighbors = n.get_neighbors(issue1, results_set=issues) + issue2_neighbors = n.get_neighbors(issue2, results_set=issues) + + assert issue1_neighbors.left == issue3 + assert issue1_neighbors.right == issue2 + assert issue2_neighbors.left == issue1 + assert issue2_neighbors.right is None From c844e595409641b284059c6078db56dba5d58815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Barrag=C3=A1n=20Merino?= Date: Sun, 16 Nov 2014 12:51:27 +0100 Subject: [PATCH 02/10] Add copyright terms in some files --- taiga/__init__.py | 16 ++++++++++++++++ taiga/celery.py | 16 ++++++++++++++++ taiga/deferred.py | 16 ++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/taiga/__init__.py b/taiga/__init__.py index a10bec52..b62e0bd0 100644 --- a/taiga/__init__.py +++ b/taiga/__init__.py @@ -1 +1,17 @@ +# Copyright (C) 2014 Andrey Antukh +# Copyright (C) 2014 Jesús Espino +# Copyright (C) 2014 David Barragán +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + from . import celery diff --git a/taiga/celery.py b/taiga/celery.py index 01a93494..ef9b7d06 100644 --- a/taiga/celery.py +++ b/taiga/celery.py @@ -1,3 +1,19 @@ +# Copyright (C) 2014 Andrey Antukh +# Copyright (C) 2014 Jesús Espino +# Copyright (C) 2014 David Barragán +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + import os from celery import Celery diff --git a/taiga/deferred.py b/taiga/deferred.py index eec20909..62080a77 100644 --- a/taiga/deferred.py +++ b/taiga/deferred.py @@ -1,3 +1,19 @@ +# Copyright (C) 2014 Andrey Antukh +# Copyright (C) 2014 Jesús Espino +# Copyright (C) 2014 David Barragán +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + from django.conf import settings from .celery import app From c34ec5b50c6bff695c6ffdbdaaa02853b3e4149b Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Mon, 17 Nov 2014 13:52:52 +0100 Subject: [PATCH 03/10] Fix limit case for non existant neighbors --- taiga/base/neighbors.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/taiga/base/neighbors.py b/taiga/base/neighbors.py index 7b4adbfb..d803b4ae 100644 --- a/taiga/base/neighbors.py +++ b/taiga/base/neighbors.py @@ -53,6 +53,9 @@ def get_neighbors(obj, results_set=None): cursor = connection.cursor() cursor.execute(query, params) row = cursor.fetchone() + if row is None: + return Neighbor(None, None) + obj_position = row[1] - 1 try: From 5129c1f1e196ab774a00c3599d986606312288c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Barrag=C3=A1n=20Merino?= Date: Mon, 17 Nov 2014 19:11:53 +0100 Subject: [PATCH 04/10] @Xaviju wins :trophy:: Convert new lines in br tags --- taiga/mdrender/service.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/taiga/mdrender/service.py b/taiga/mdrender/service.py index baba2349..1bd4568d 100644 --- a/taiga/mdrender/service.py +++ b/taiga/mdrender/service.py @@ -74,7 +74,8 @@ def _make_extensions_list(wikilinks_config=None, project=None): MentionsExtension(), TaigaReferencesExtension(project), "extra", - "codehilite"] + "codehilite", + "nl2br"] import diff_match_patch From 058195d6709affbf5948d653d3da1c1467b27a8b Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 18 Nov 2014 10:16:18 +0100 Subject: [PATCH 05/10] User story points are undefined if the task points are undefined --- taiga/projects/userstories/models.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/taiga/projects/userstories/models.py b/taiga/projects/userstories/models.py index f9688380..54458b3c 100644 --- a/taiga/projects/userstories/models.py +++ b/taiga/projects/userstories/models.py @@ -126,9 +126,15 @@ class UserStory(OCCModelMixin, WatchedModelMixin, BlockedMixin, TaggedMixin, mod return self.role_points def get_total_points(self): + not_null_role_points = self.role_points.select_related("points").\ + exclude(points__value__isnull=True) + + #If we only have None values the sum should be None + if not not_null_role_points: + return None + total = 0.0 - for rp in self.role_points.select_related("points"): - if rp.points.value: - total += rp.points.value + for rp in not_null_role_points: + total += rp.points.value return total From adb2e24423eaa046bcdd900b13ab0f147b221a76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Tue, 18 Nov 2014 12:52:53 +0100 Subject: [PATCH 06/10] Added thead and tbody to allowed tags on mdrender service --- taiga/mdrender/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taiga/mdrender/service.py b/taiga/mdrender/service.py index baba2349..9ab347f7 100644 --- a/taiga/mdrender/service.py +++ b/taiga/mdrender/service.py @@ -52,7 +52,7 @@ from .extensions.references import TaigaReferencesExtension # Bleach configuration -bleach.ALLOWED_TAGS += ["p", "table", "th", "tr", "td", "h1", "h2", "h3", +bleach.ALLOWED_TAGS += ["p", "table", "thead", "tbody", "th", "tr", "td", "h1", "h2", "h3", "div", "pre", "span", "hr", "dl", "dt", "dd", "sup", "img", "del", "br", "ins"] From 18e38792b52ecb436cd563c34acf123123fe4b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Tue, 18 Nov 2014 12:14:40 +0100 Subject: [PATCH 07/10] Changed how external reference is obtained --- taiga/github_hook/event_hooks.py | 16 ++++++++-------- taiga/projects/issues/serializers.py | 3 ++- taiga/projects/tasks/serializers.py | 3 ++- taiga/projects/userstories/serializers.py | 3 ++- tests/integration/test_github_hook.py | 14 +++++++------- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/taiga/github_hook/event_hooks.py b/taiga/github_hook/event_hooks.py index 7f60d7c2..bf7745f0 100644 --- a/taiga/github_hook/event_hooks.py +++ b/taiga/github_hook/event_hooks.py @@ -110,11 +110,11 @@ class IssuesEventHook(BaseEventHook): subject = self.payload.get('issue', {}).get('title', None) description = self.payload.get('issue', {}).get('body', None) - github_reference = self.payload.get('issue', {}).get('number', None) + github_url = self.payload.get('issue', {}).get('html_url', None) github_user = self.payload.get('issue', {}).get('user', {}).get('id', None) project_url = self.payload.get('repository', {}).get('html_url', None) - if not all([subject, github_reference, project_url]): + if not all([subject, github_url, project_url]): raise ActionSyntaxException(_("Invalid issue information")) issue = Issue.objects.create( @@ -125,7 +125,7 @@ class IssuesEventHook(BaseEventHook): type=self.project.default_issue_type, severity=self.project.default_severity, priority=self.project.default_priority, - external_reference=['github', github_reference], + external_reference=['github', github_url], owner=get_github_user(github_user) ) take_snapshot(issue, user=get_github_user(github_user)) @@ -139,18 +139,18 @@ class IssueCommentEventHook(BaseEventHook): if self.payload.get('action', None) != "created": raise ActionSyntaxException(_("Invalid issue comment information")) - github_reference = self.payload.get('issue', {}).get('number', None) + github_url = self.payload.get('issue', {}).get('html_url', None) comment_message = self.payload.get('comment', {}).get('body', None) github_user = self.payload.get('sender', {}).get('id', None) project_url = self.payload.get('repository', {}).get('html_url', None) comment_message = replace_github_references(project_url, comment_message) - if not all([comment_message, github_reference, project_url]): + if not all([comment_message, github_url, project_url]): raise ActionSyntaxException(_("Invalid issue comment information")) - issues = Issue.objects.filter(external_reference=["github", github_reference]) - tasks = Task.objects.filter(external_reference=["github", github_reference]) - uss = UserStory.objects.filter(external_reference=["github", github_reference]) + issues = Issue.objects.filter(external_reference=["github", github_url]) + tasks = Task.objects.filter(external_reference=["github", github_url]) + uss = UserStory.objects.filter(external_reference=["github", github_url]) for item in list(issues) + list(tasks) + list(uss): snapshot = take_snapshot(item, diff --git a/taiga/projects/issues/serializers.py b/taiga/projects/issues/serializers.py index d3f5d740..cc025185 100644 --- a/taiga/projects/issues/serializers.py +++ b/taiga/projects/issues/serializers.py @@ -16,7 +16,7 @@ from rest_framework import serializers -from taiga.base.serializers import Serializer, PickleField, NeighborsSerializerMixin +from taiga.base.serializers import Serializer, PickleField, NeighborsSerializerMixin, PgArrayField from taiga.mdrender.service import render as mdrender from taiga.projects.validators import ProjectExistsValidator from taiga.projects.notifications.validators import WatchersValidator @@ -26,6 +26,7 @@ from . import models class IssueSerializer(WatchersValidator, serializers.ModelSerializer): tags = PickleField(required=False) + external_reference = PgArrayField(required=False) is_closed = serializers.Field(source="is_closed") comment = serializers.SerializerMethodField("get_comment") generated_user_stories = serializers.SerializerMethodField("get_generated_user_stories") diff --git a/taiga/projects/tasks/serializers.py b/taiga/projects/tasks/serializers.py index 20bd17dc..2dcf6097 100644 --- a/taiga/projects/tasks/serializers.py +++ b/taiga/projects/tasks/serializers.py @@ -16,7 +16,7 @@ from rest_framework import serializers -from taiga.base.serializers import Serializer, PickleField, NeighborsSerializerMixin +from taiga.base.serializers import Serializer, PickleField, NeighborsSerializerMixin, PgArrayField from taiga.mdrender.service import render as mdrender from taiga.projects.validators import ProjectExistsValidator, TaskStatusExistsValidator from taiga.projects.milestones.validators import SprintExistsValidator @@ -28,6 +28,7 @@ from . import models class TaskSerializer(WatchersValidator, serializers.ModelSerializer): tags = PickleField(required=False, default=[]) + external_reference = PgArrayField(required=False) comment = serializers.SerializerMethodField("get_comment") milestone_slug = serializers.SerializerMethodField("get_milestone_slug") blocked_note_html = serializers.SerializerMethodField("get_blocked_note_html") diff --git a/taiga/projects/userstories/serializers.py b/taiga/projects/userstories/serializers.py index 06aaed22..c768f909 100644 --- a/taiga/projects/userstories/serializers.py +++ b/taiga/projects/userstories/serializers.py @@ -18,7 +18,7 @@ import json from django.apps import apps from rest_framework import serializers -from taiga.base.serializers import Serializer, PickleField, NeighborsSerializerMixin +from taiga.base.serializers import Serializer, PickleField, NeighborsSerializerMixin, PgArrayField from taiga.mdrender.service import render as mdrender from taiga.projects.validators import ProjectExistsValidator, UserStoryStatusExistsValidator from taiga.projects.userstories.validators import UserStoryExistsValidator @@ -39,6 +39,7 @@ class RolePointsField(serializers.WritableField): class UserStorySerializer(WatchersValidator, serializers.ModelSerializer): tags = PickleField(default=[], required=False) + external_reference = PgArrayField(required=False) points = RolePointsField(source="role_points", required=False) total_points = serializers.SerializerMethodField("get_total_points") comment = serializers.SerializerMethodField("get_comment") diff --git a/tests/integration/test_github_hook.py b/tests/integration/test_github_hook.py index 0032f0b5..168e44ef 100644 --- a/tests/integration/test_github_hook.py +++ b/tests/integration/test_github_hook.py @@ -219,7 +219,7 @@ def test_issues_event_opened_issue(client): "issue": { "title": "test-title", "body": "test-body", - "number": 10, + "html_url": "http://github.com/test/project/issues/11", }, "assignee": {}, "label": {}, @@ -249,7 +249,7 @@ def test_issues_event_other_than_opened_issue(client): "issue": { "title": "test-title", "body": "test-body", - "number": 10, + "html_url": "http://github.com/test/project/issues/11", }, "assignee": {}, "label": {}, @@ -291,17 +291,17 @@ def test_issues_event_bad_issue(client): def test_issue_comment_event_on_existing_issue_task_and_us(client): - issue = f.IssueFactory.create(external_reference=["github", "10"]) + issue = f.IssueFactory.create(external_reference=["github", "http://github.com/test/project/issues/11"]) take_snapshot(issue, user=issue.owner) - task = f.TaskFactory.create(project=issue.project, external_reference=["github", "10"]) + task = f.TaskFactory.create(project=issue.project, external_reference=["github", "http://github.com/test/project/issues/11"]) take_snapshot(task, user=task.owner) - us = f.UserStoryFactory.create(project=issue.project, external_reference=["github", "10"]) + us = f.UserStoryFactory.create(project=issue.project, external_reference=["github", "http://github.com/test/project/issues/11"]) take_snapshot(us, user=us.owner) payload = { "action": "created", "issue": { - "number": 10, + "html_url": "http://github.com/test/project/issues/11", }, "comment": { "body": "Test body", @@ -346,7 +346,7 @@ def test_issue_comment_event_on_not_existing_issue_task_and_us(client): payload = { "action": "created", "issue": { - "number": 11, + "html_url": "http://github.com/test/project/issues/11", }, "comment": { "body": "Test body", From 7b498924e72256b400a1b2da286cdcbb3b3548b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20Espino?= Date: Tue, 18 Nov 2014 13:32:32 +0100 Subject: [PATCH 08/10] Add tests to bug #1553 resolution --- tests/integration/test_userstories.py | 31 +++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/integration/test_userstories.py b/tests/integration/test_userstories.py index 8fbb33ab..83c35c65 100644 --- a/tests/integration/test_userstories.py +++ b/tests/integration/test_userstories.py @@ -200,3 +200,34 @@ def test_archived_filter(client): data = {"is_archived": 1} response = client.get(url, data) assert len(json.loads(response.content)) == 1 + +def test_get_total_points(client): + project = f.ProjectFactory.create() + + role1 = f.RoleFactory.create(project=project) + role2 = f.RoleFactory.create(project=project) + + points1 = f.PointsFactory.create(project=project, value=None) + points2 = f.PointsFactory.create(project=project, value=1) + points3 = f.PointsFactory.create(project=project, value=2) + + us_with_points = f.UserStoryFactory.create(project=project) + us_with_points.role_points.all().delete() + f.RolePointsFactory.create(user_story=us_with_points, role=role1, points=points2) + f.RolePointsFactory.create(user_story=us_with_points, role=role2, points=points3) + + assert us_with_points.get_total_points() == 3.0 + + us_without_points = f.UserStoryFactory.create(project=project) + us_without_points.role_points.all().delete() + f.RolePointsFactory.create(user_story=us_without_points, role=role1, points=points1) + f.RolePointsFactory.create(user_story=us_without_points, role=role2, points=points1) + + assert us_without_points.get_total_points() is None + + us_mixed = f.UserStoryFactory.create(project=project) + us_mixed.role_points.all().delete() + f.RolePointsFactory.create(user_story=us_mixed, role=role1, points=points1) + f.RolePointsFactory.create(user_story=us_mixed, role=role2, points=points2) + + assert us_mixed.get_total_points() == 1.0 From 99244b8393ec406f1ee6fc9b6d4c603df53f7fd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Barrag=C3=A1n=20Merino?= Date: Tue, 18 Nov 2014 13:23:20 +0100 Subject: [PATCH 09/10] Re-do github login --- taiga/auth/services.py | 37 +++++++++------ tests/integration/test_auth_api.py | 73 +++++++++++++++++++++++++++++- 2 files changed, 96 insertions(+), 14 deletions(-) diff --git a/taiga/auth/services.py b/taiga/auth/services.py index 26f3d0db..dee8c606 100644 --- a/taiga/auth/services.py +++ b/taiga/auth/services.py @@ -34,6 +34,7 @@ from djmail.template_mail import MagicMailBuilder from taiga.base import exceptions as exc from taiga.users.serializers import UserSerializer from taiga.users.services import get_and_validate_user +from taiga.base.utils.slug import slugify_uniquely from .tokens import get_token_for_user from .signals import user_registered as user_registered_signal @@ -50,7 +51,7 @@ def send_register_email(user) -> bool: return bool(email.send()) -def is_user_already_registered(*, username:str, email:str, github_id:int=None) -> (bool, str): +def is_user_already_registered(*, username:str, email:str) -> (bool, str): """ Checks if a specified user is already registred. @@ -65,9 +66,6 @@ def is_user_already_registered(*, username:str, email:str, github_id:int=None) - if user_model.objects.filter(email=email): return (True, _("Email is already in use.")) - if github_id and user_model.objects.filter(github_id=github_id): - return (True, _("GitHub id is already in use")) - return (False, None) @@ -182,20 +180,33 @@ def github_register(username:str, email:str, full_name:str, github_id:int, bio:s :returns: User """ user_model = apps.get_model("users", "User") - user, created = user_model.objects.get_or_create(github_id=github_id, - defaults={"username": username, - "email": email, - "full_name": full_name, - "bio": bio}) + + try: + # Github user association exist? + user = user_model.objects.get(github_id=github_id) + except user_model.DoesNotExist: + try: + # Is a user with the same email as the github user? + user = user_model.objects.get(email=email) + user.github_id = github_id + user.save(update_fields=["github_id"]) + except user_model.DoesNotExist: + # Create a new user + username_unique = slugify_uniquely(username, user_model, slugfield="username") + user = user_model.objects.create(email=email, + username=username_unique, + github_id=github_id, + full_name=full_name, + bio=bio) + + send_register_email(user) + user_registered_signal.send(sender=user.__class__, user=user) + if token: membership = get_membership_by_token(token) membership.user = user membership.save(update_fields=["user"]) - if created: - send_register_email(user) - user_registered_signal.send(sender=user.__class__, user=user) - return user diff --git a/tests/integration/test_auth_api.py b/tests/integration/test_auth_api.py index 60dced10..d322fa32 100644 --- a/tests/integration/test_auth_api.py +++ b/tests/integration/test_auth_api.py @@ -115,6 +115,78 @@ def test_response_200_in_registration_with_github_account(client, settings): assert response.data["bio"] == "time traveler" assert response.data["github_id"] == 1955 +def test_response_200_in_registration_with_github_account_and_existed_user_by_email(client, settings): + settings.PUBLIC_REGISTER_ENABLED = False + form = {"type": "github", + "code": "xxxxxx"} + user = factories.UserFactory() + user.email = "mmcfly@bttf.com" + user.github_id = None + user.save() + + with patch("taiga.base.connectors.github.me") as m_me: + m_me.return_value = ("mmcfly@bttf.com", + github.User(id=1955, + username="mmcfly", + full_name="martin seamus mcfly", + bio="time traveler")) + + response = client.post(reverse("auth-list"), form) + assert response.status_code == 200 + assert response.data["username"] == user.username + assert response.data["auth_token"] != "" and response.data["auth_token"] != None + assert response.data["email"] == user.email + assert response.data["full_name"] == user.full_name + assert response.data["bio"] == user.bio + assert response.data["github_id"] == 1955 + +def test_response_200_in_registration_with_github_account_and_existed_user_by_github_id(client, settings): + settings.PUBLIC_REGISTER_ENABLED = False + form = {"type": "github", + "code": "xxxxxx"} + user = factories.UserFactory() + user.github_id = 1955 + user.save() + + with patch("taiga.base.connectors.github.me") as m_me: + m_me.return_value = ("mmcfly@bttf.com", + github.User(id=1955, + username="mmcfly", + full_name="martin seamus mcfly", + bio="time traveler")) + + response = client.post(reverse("auth-list"), form) + assert response.status_code == 200 + assert response.data["username"] != "mmcfly" + assert response.data["auth_token"] != "" and response.data["auth_token"] != None + assert response.data["email"] != "mmcfly@bttf.com" + assert response.data["full_name"] != "martin seamus mcfly" + assert response.data["bio"] != "time traveler" + assert response.data["github_id"] == user.github_id + +def test_response_200_in_registration_with_github_account_and_change_github_username(client, settings): + settings.PUBLIC_REGISTER_ENABLED = False + form = {"type": "github", + "code": "xxxxxx"} + user = factories.UserFactory() + user.username = "mmcfly" + user.save() + + with patch("taiga.base.connectors.github.me") as m_me: + m_me.return_value = ("mmcfly@bttf.com", + github.User(id=1955, + username="mmcfly", + full_name="martin seamus mcfly", + bio="time traveler")) + + response = client.post(reverse("auth-list"), form) + assert response.status_code == 200 + assert response.data["username"] == "mmcfly-1" + assert response.data["auth_token"] != "" and response.data["auth_token"] != None + assert response.data["email"] == "mmcfly@bttf.com" + assert response.data["full_name"] == "martin seamus mcfly" + assert response.data["bio"] == "time traveler" + assert response.data["github_id"] == 1955 def test_response_200_in_registration_with_github_account_in_a_project(client, settings): settings.PUBLIC_REGISTER_ENABLED = False @@ -171,7 +243,6 @@ def test_respond_400_if_username_or_email_is_duplicate(client, settings, registe response = client.post(reverse("auth-register"), register_form) assert response.status_code == 201 - register_form["username"] = "username" register_form["email"] = "ff@dd.com" response = client.post(reverse("auth-register"), register_form) From f11cb3edaa0689b4c313f272e81a45ed8b38925f Mon Sep 17 00:00:00 2001 From: Alejandro Alonso Date: Tue, 18 Nov 2014 16:04:07 +0100 Subject: [PATCH 10/10] Updating changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d84dd8ba..074e016a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog # -## 1.3.0 Dryas hookeriana (Unreleased) +## 1.3.0 Dryas hookeriana (2014-11-18) ### Features - GitHub integration (Phase I):