Create api files for IPR #38

Arti1946 · 2024-01-29T19:25:00Z

No description provided.

Reagent992

Хорошая работа 💪
Оставил несколько комментариев и вопросов.

Скажи мне что у тебя по времени, фронтенд уже пробует подключатся к беку, этот PR желательно не затягивать.

Reagent992 · 2024-01-29T20:26:31Z

api/v1/serializers/ipr_serializers.py

+
+class IPRSerializer(serializers.ModelSerializer):
+    creator = serializers.SlugRelatedField(
+        slug_field="username",


Почему тут username возвращается, а не id?

Reagent992 · 2024-01-29T20:30:14Z

api/v1/serializers/ipr_serializers.py

+        slug_field="username",
+        read_only=True,
+    )
+    executor = serializers.RelatedField(read_only=True)


Это что делает? Почему не id просто?

Reagent992 · 2024-01-29T20:37:26Z

api/v1/permissions.py

+            return True
+        if user.is_authenticated:
+            team_id = get_object_or_404(Team, boss_id=request.user.id)
+            if MiddleUsersTeams.objects.filter(


3 дня назад упростил модель user, там уже нету модели MiddleUsersTeams

Reagent992 · 2024-01-29T20:38:08Z

api/v1/permissions.py

+        executor = get_object_or_404(User, id=executor_id)
+        if (
+            user.is_authenticated
+            and user.get_team() == executor.get_team()


Тоже самое, User изменился, там теперь просто поле team

Reagent992 · 2024-01-29T20:45:05Z

api/v1/views/ipr.py

+        methods=["get"],
+        permission_classes=[permissions.IsAuthenticated],
+    )
+    def status(self, request, ipr_id):


Сюда бы докстринг

Reagent992 · 2024-01-29T20:45:55Z

api/v1/views/ipr.py

+        ipr = get_object_or_404(IPR, id=ipr_id)
+        executor_id = self.kwargs.get("user_id")
+        executor = get_object_or_404(User, id=executor_id)
+        tasks_without_trail = ipr.tasks.exclude(status="trail").count()


Тут можно не хардкодить trail и complete, а взять из статусов.

Reagent992 · 2024-01-29T20:47:25Z

ipr/models.py

    creator = models.ForeignKey(
        User,
        on_delete=models.CASCADE,
        verbose_name="Создатель ИПР",
        related_name="created_ipr",
    )
    creation_date = models.DateField("Дата создания ИПР", auto_now_add=True)
-    start_date = models.DateField("Дата начала работ по ИПР")
+    start_date = models.DateField(


Почему тут появились blank=True, null=True ?

Reagent992 · 2024-01-29T20:51:25Z

ipr/models.py

    end_date = models.DateField("Дедлайн ИПР")
-    status = models.CharField("Статус ИПР", max_length=20, choices=STATUSES)
+    status = models.CharField(
+        "Статус ИПР", max_length=20, choices=STATUSES, default=STATUSES[0]


Сделай Объединить статусы задач и ипр(они одинаковые). Взять вариант из задач. Вынести в приложение core или settings.py #20 чтобы не обращаться по индексу STATUSES[0]

Стандартный статус выполнен? На первый взгляд лучше "в работе"

Reagent992 · 2024-01-29T21:36:58Z

Еще, т.к. проект уже на сервере:

PR делаем в dev
Есть изменения в моделях -> нужен новый файл миграции. ( внимательно, от файла миграции который сейчас есть в dev )

Reagent992

Хорошо.👍
Оставил вопросы и комментарии.

Reagent992 · 2024-01-30T14:33:55Z

api/v1/serializers/api/ipr_serializers.py

+class ExecutorSerializer(CustomUserSerializer):
+    class Meta:
+        fields = (
+            "first_name",


надо добавить поле id

Reagent992 · 2024-01-30T14:38:33Z

api/v1/serializers/api/ipr_serializers.py

+    def get_status(self, obj):
+        if obj.status == Status.IN_PROGRESS and obj.start_date > obj.end_date:
+            obj.status = Status.TRAIL
+        return obj.status


Это лучше сделать через сигналы, @greenpandorik взялся это делать.

Таким образом, без obj.save() наверно даже значение в БД не сохранится.

мне тогда пока оставить как есть? как появятся сигналы поменяю

Да, сегодня скорее всего добью это. Сорри приболел просто , выпал на пару дней

Reagent992 · 2024-01-30T14:52:49Z

ipr/models.py

@@ -20,13 +14,10 @@ class IPR(models.Model):
        verbose_name="Создатель ИПР",
        related_name="created_ipr",
    )
-    creation_date = models.DateField("Дата создания ИПР", auto_now_add=True)


Дату создания не надо удалять в модели.
ПМ в чате имел ввиду "удалить из дизайна", он не знает чего у тебя там в модели происходит.
Для БД это полезная информация, с ней много чего можно сделать.

Reagent992 · 2024-01-30T14:57:46Z

api/v1/views/ipr_views.py

@@ -24,36 +27,39 @@ class Meta:
    def get_queryset(self):
        return IPR.objects.select_related("executor", "creator")


При запросе на api/v1/iprs/ мы выдаем все ИПР пользователя делающего запрос.
При запросе с параметром /api/v1/iprs/?user_id={id} мы выдаем ИПР указанного пользователя.
Проверь эту логику.

Reagent992 · 2024-01-30T15:01:39Z

api/v1/views/ipr_views.py

@@ -24,36 +27,39 @@ class Meta:
    def get_queryset(self):


Ты удалил creation_date в модели, но оставил тут сортировку.
Надо вернуть поле в модель и сортировку тоже.
Тут она просто дублирует это значение из модели, можно удалить.

class Meta: ordering = ["-creation_date"]

Так же во viewset можно ограничить количество доступных HTTP-методов.
Нам хватит `http_method_names = ["get", "post", "patch", "head", "options"]

Reagent992 · 2024-01-30T15:11:51Z

api/v1/serializers/api/ipr_serializers.py

+
+class IPRSerializerPost(serializers.ModelSerializer):
+    creator = serializers.PrimaryKeyRelatedField(read_only=True)
+    executor = serializers.PrimaryKeyRelatedField(read_only=True)


Ты проверил создание ИПР ручками?
На первый взгляд это поля и def perform_create конфликтуют, но тут я могу ошибаться.

Reagent992 · 2024-01-30T15:15:24Z

api/v1/serializers/api/ipr_serializers.py

 class IPRSerializer(serializers.ModelSerializer):
    tasks = TaskSerializer(many=True, read_only=True)
+    creator = serializers.PrimaryKeyRelatedField(read_only=True)
+    executor = ExecutorSerializer(read_only=True)
+    status = serializers.SerializerMethodField()


Не понимаю, зачем тут переопределять status и creator?

Creato - переопределил, что бы при создании ИПР он не запрашивался, а автоматически проставлялся в методе performe_create во view.

Status - что бы у статуса было актуальное значение. Но с учетом того что мы делаем это через сигналы, то я могу либо оставить пока как есть либо удалить и ждать пока будут готовы сигналы

В смысле, у тебя же для создания ИПР другой сериализатор.

Да, а этот для отображения при GET запросе, при создании статус ставится автоматически в работе

Все понял, затупил

Reagent992

Отлично⭐

Если у тебя нету аргументов в пользу глобального фильтра, то его стоит убрать.
Остальное по желанию.

Reagent992 · 2024-01-30T20:44:58Z

config/settings.py

@@ -149,6 +150,9 @@
    "DEFAULT_AUTHENTICATION_CLASSES": [
        "rest_framework_simplejwt.authentication.JWTAuthentication",
    ],
+    'DEFAULT_FILTER_BACKENDS': [


https://ilyachch.gitbook.io/django-rest-framework-russian-documentation/overview/navigaciya-po-api/filtering#nastroika-bekendov-filtrov
Это установка фильтра глобально.
Зачем?

согласен

Reagent992 · 2024-01-30T20:46:37Z

api/v1/views/ipr_views.py

+        serializer.save(creator=self.request.user, executor=executor)
+
+    def get_permissions(self):
+        if self.request.method in permissions.SAFE_METHODS:


У нас весь проект на IsAuthenticated, можно сократить до:

if self.request.method not in permissions.SAFE_METHODS: self.permission_classes = [TeamBossPermission] return super(IPRViewSet, self).get_permissions()

Reagent992 · 2024-01-30T20:46:57Z

api/v1/views/ipr_views.py

+    @action(
+        detail=True,
+        methods=["get"],
+        permission_classes=[permissions.IsAuthenticated],


Тут тоже можно не указывать permission_classes=[permissions.IsAuthenticated],

Reagent992 · 2024-01-30T20:47:34Z

api/v1/views/ipr_views.py

+        return IPR.objects.filter(executor=self.request.user)
+
+    def get_serializer_class(self):
+        if self.request.method == "POST" or self.request.method == "PATCH":


Можно короче:

if self.request.method in ("POST", "PATH"):

Reagent992 · 2024-01-30T20:54:20Z

core/statuses_for_ipr_tests.py

@@ -0,0 +1,8 @@
+from django.db import models
+


Название файла с ложным смыслом, это не единственное его использование.
Предлагаю просто оставить статусы

Reagent992 · 2024-01-30T20:55:50Z

ipr/admin.py

    )
-    ordering = ("-creation_date",)
+    ordering = ("-id",)



В админку тоже можно вернуть creation_date

* API for IPR (#38) * Create api files for IPR * Небольшой ренейминг * Fix for ipr serializers, views, model and fixture * Fix IPR model and some other improvements * fix serializer * fix filtering --------- Co-authored-by: Miron Sadykov <[email protected]> * Улучшение уведомлений, тесты уведомлений. (#41) 1. Изменены ситуации создания уведомлений: - Изменения в задаче не создают уведомления. - Изменения даты ИПР создает уведомление. - Руководитель получает уведомление о закрытом ИПР. 2. Добавлена библиотека [django-dirtyfields](https://django-dirtyfields.readthedocs.io/en/stable/quickstart.html) | Для доступа к измененной информации в джанго сигнале `post_save` . 3. В связи с пунктом 2, в модели(Задачи и ИПР) добавлены миксины. 4. Тесты для автоматического создания уведомлений. 5. Тесты для API уведомлений. 6. Баш-скрипт для загрузки фикстур. * Добавление функций в админку Задач (#42) * Feature/tests ci (#43) * В CI добавлен запуск тестов * fix/ipr-api (#45) * Fix view, permission, and add def_to_representation() to IPRSerializerPost * Добавлено поле ruled_team в ответ users (#48) * Добавил руссифицированную документацию для ИПР (#47) --------- Co-authored-by: Артур Галиаскаров <[email protected]>

Create api files for IPR

f4ee39e

Reagent992 changed the base branch from main to dev January 29, 2024 20:20

Reagent992 requested changes Jan 29, 2024

View reviewed changes

Reagent992 and others added 3 commits January 30, 2024 13:41

Merge remote-tracking branch 'origin/dev' into Feature-IPR-api

b5bbdfe

Небольшой ренейминг

f079680

Fix for ipr serializers, views, model and fixture

066bd1a

Reagent992 reviewed Jan 30, 2024

View reviewed changes

Reagent992 approved these changes Jan 30, 2024

View reviewed changes

Arti1946 added 2 commits January 30, 2024 21:30

Fix IPR model and some other improvements

8fb3254

fix serializer

33d51ce

Reagent992 requested changes Jan 30, 2024

View reviewed changes

fix filtering

2a6c661

Reagent992 approved these changes Jan 31, 2024

View reviewed changes

Reagent992 merged commit c071e96 into dev Jan 31, 2024
1 check passed

Reagent992 deleted the Feature-IPR-api branch January 31, 2024 12:57

		@@ -24,36 +27,39 @@ class Meta:
		def get_queryset(self):
		return IPR.objects.select_related("executor", "creator")

Create api files for IPR #38

Create api files for IPR #38

Conversation

Arti1946 commented Jan 29, 2024

Reagent992 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Reagent992 commented Jan 29, 2024

Reagent992 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Reagent992 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Reagent992 left a comment •

edited

Loading