Create Entities #57

astronik00 · 2023-04-28T09:37:18Z

Task merge request

Related Issue	#45
Reviewer requrements	Знание принципов работы ORM фреймворков, умение проектировать схемы баз данных

Code changes

Созданы следующие модули python:

task.py
server.py
label.py
size.py
status.py
type.py
priority.py

Созданы следующие новые классы:

Task - класс задачи VTODO
Server - класс сервера, который содержит в себе данные для подключения
Label - промежуточный класс, который агрегирует в себе лейблы
Size - класс лейблов для размера задачи
Status - класс лейблов для статуса задачи
Type - класс лейблов для типа задачи
Priotiry - класс лейблов для приоритета задачи

How to test

Просмотреть схему баз данных:
Ознакомиться с документацией по работе с relationship в SQLAlchemy, а также эту документацию по работе с Adjacency List
Соотнести соответствие схемы и всех полей классов, проверить правильность расставленных отношений, в том числе и ключей

aleksandra-shchegoleva

@astronik00 Закончила ревью по текущему коду. Считаю, что нужно провести следующие тесты для проверки связей и поиска возможных ошибок:

Удаление сервера - должны быть удалены все задачи и категории
Удаление задачи - удаление связанного label
Если возможно удалять или изменять категории из доступных - удаление или изменения соотвествующего атрибута в label

aleksandra-shchegoleva · 2023-04-28T11:05:41Z

Code/entities/local/task.py

+
+    # keys
+    task_id = Column(Integer, primary_key=True, autoincrement=True)
+    server_id = Column(Integer, ForeignKey('server.server_id'))


Нужно поставить ondelete=cascade тоже, при удалении сервера должны удаляться все задачи, связанные с этим сервером

aleksandra-shchegoleva · 2023-04-28T11:24:10Z

Code/entities/local/task.py

+        self.due = due
+        self.last_mod = last_mod
+        self.status = status
+        self.labels = labels


Поле labels - как я вижу это не атрибут таблицы task. Судя по конструктору оно хранит список (список сущностей Label?). Для чего нужно это поле? Возможно имелось ввиду label? Почему на вход подается список, а не сущность Label?

Это призрак прошлой версии, не заметила лишнего поля. Его быть не должно

Code/entities/local/Server.py

aleksandra-shchegoleva · 2023-04-28T11:29:51Z

Code/entities/local/label.py

+    type_id = Column(Integer, ForeignKey('type.type_id'))
+    status_id = Column(Integer, ForeignKey('status.status_id'))
+
+    task = relationship("Task", back_populates="label")


Такой же вопрос - в конструкторе Task указан labels, тут label

Не прописан cascade на случай удаления задачи

labels - лишнее поле, которое я уберу
должен быть лишь label, который является отношением для создания связи один ко одному

ок, cascade посмотрю и добавлю

aleksandra-shchegoleva · 2023-04-28T11:37:53Z

Code/entities/local/label.py

+    priority = relationship("Priority", back_populates="labels")
+    size = relationship("Size", back_populates="labels")
+    type = relationship("Type", back_populates="labels")
+    status = relationship("Status", back_populates="labels")


В текущей версии предусмотрен случай удаления или изменения какой-то категории из доступного набора?

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

aleksandra-shchegoleva · 2023-04-28T11:38:49Z

Code/entities/local/labels/priority.py

+
+    __tablename__ = 'priority'
+
+    priority_id = Column(Integer, primary_key=True)


Здесь не нужен автоинкремент? В конструкторе id тоже не назначается

Такой же вопрос к сущностям Size, Status, Type

автоинкремент добавлю

aleksandra-shchegoleva · 2023-04-28T11:39:42Z

Code/entities/local/labels/priority.py

+    name = Column(String)
+
+    # M:1
+    server = relationship("Server", back_populates="priorities")


Нужен cascade при удалении сервера

Такой же вопрос к сущностям Size, Status, Type

aleksandra-shchegoleva · 2023-04-28T12:22:42Z

Code/entities/local/label.py

+
+    def __init__(self, task, priority=None, size=None, type=None, status=None):
+        self.task = task
+        self.prioriry = priority


Опечатка: должно быть "priority"

aleksandra-shchegoleva · 2023-04-28T12:33:25Z

Code/entities/local/task.py

+    due = Column(DateTime)
+    last_mod = Column(DateTime)
+
+    status = Column(Integer)


Атрибут status лишний, он уже есть в сущности Label

astronik00 · 2023-04-28T13:01:02Z

@astronik00 Закончила ревью по текущему коду. Считаю, что нужно провести следующие тесты для проверки связей и поиска возможных ошибок:

Удаление сервера - должны быть удалены все задачи и категории

Удаление задачи - удаление связанного label

Если возможно удалять или изменять категории из доступных - удаление или изменения соотвествующего атрибута в label

Не думаю, что эти тесты входят именно в эту задачу.

У меня есть задача "написать репозиторий задач", я считаю, что это должно фигурировать там. Ну и если у меня что-то не будет работать в репозитории, то я просто сюда вернусь и исправлю.

А так все тесты объективны и понятны, только тесты сервера - это в принципе даже к моей задаче не относится... Я пишу функции работы с задачами, а то что-то слишком мне много делать :/

astronik00 · 2023-04-28T14:33:38Z

@aleksandra-shchegoleva

Я внесла все исправления

aleksandra-shchegoleva · 2023-04-28T18:01:14Z

@astronik00 Хорошо, просто при чтении кода возникли такие мысли для тестов. Может быть тесты вынести в отдельную задачу? Почему удаление сервера не относится? Имеется ввиду удаление сущности Server. Я могу помочь с этой задачей, после того, как доделаю валидацию.

aleksandra-shchegoleva · 2023-04-28T18:01:42Z

@aleksandra-shchegoleva

Я внесла все исправления

@astronik00 Сейчас посмотрю

aleksandra-shchegoleva

@astronik00 Других замечаний нет. В целом все хорошо

aleksandra-shchegoleva · 2023-04-28T18:17:07Z

Code/entities/local/labels/status.py

+    __tablename__ = 'status'
+
+    status_id = Column(Integer, primary_key=True, autoincrement=True)
+    server_id = Column(Integer, ForeignKey('server.server_id')) # ForeignKey('server.server_id'), ondelete='CASCADE')) 


Я верно понимаю, что добавление каскадного удаления не относится к задаче по созданию сущностей? Или почему тут закомментирован альтернативный внешний ключ?

Я верно понимаю, что добавление каскадного удаления не относится к задаче по созданию сущностей? Или почему тут закомментирован альтернативный внешний ключ?

Я не понимаю, КАК эта версия попала на гитхаб, если я залила следующую версию кода и на локалке она у меня отображается запушенной (т.е с каскадным удалением):

У меня кстати аналогичная проблема - на локальной версии файл "Server.py" написан "server.py", но заливая каждый раз сюда, он становится с большой буквы. Я не понимаю, как это происходит

Added Labels: Size, Status, Type, Priority, Label Base Entities: Server, Task

astronik00 added the documentation Improvements or additions to documentation label Apr 28, 2023

astronik00 requested a review from aleksandra-shchegoleva April 28, 2023 09:37

astronik00 self-assigned this Apr 28, 2023

astronik00 removed the documentation Improvements or additions to documentation label Apr 28, 2023

astronik00 changed the title ~~Created Entities~~ Create Entities Apr 28, 2023

astronik00 force-pushed the 45-create-entity-classes branch from 53dfb78 to 6935d84 Compare April 28, 2023 10:12

aleksandra-shchegoleva reviewed Apr 28, 2023

View reviewed changes

astronik00 force-pushed the 45-create-entity-classes branch 3 times, most recently from d72c146 to 4f4d055 Compare April 28, 2023 14:32

astronik00 force-pushed the 45-create-entity-classes branch from 4f4d055 to 3a67d71 Compare April 28, 2023 14:35

aleksandra-shchegoleva reviewed Apr 28, 2023

View reviewed changes

astronik00 force-pushed the 45-create-entity-classes branch from 67d571a to 74a5845 Compare April 30, 2023 07:00

created entities

a885312

Added Labels: Size, Status, Type, Priority, Label Base Entities: Server, Task

astronik00 force-pushed the 45-create-entity-classes branch from 74a5845 to a885312 Compare April 30, 2023 07:23

astronik00 merged commit f95d1dd into 46-create-local-taskservice Apr 30, 2023

astronik00 deleted the 45-create-entity-classes branch May 24, 2023 20:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create Entities #57

Create Entities #57

astronik00 commented Apr 28, 2023 •

edited

Loading

aleksandra-shchegoleva left a comment

aleksandra-shchegoleva Apr 28, 2023

aleksandra-shchegoleva Apr 28, 2023

astronik00 Apr 28, 2023 •

edited

Loading

aleksandra-shchegoleva Apr 28, 2023

aleksandra-shchegoleva Apr 28, 2023

astronik00 Apr 28, 2023

aleksandra-shchegoleva Apr 28, 2023

astronik00 Apr 28, 2023

aleksandra-shchegoleva Apr 28, 2023

aleksandra-shchegoleva Apr 28, 2023

astronik00 Apr 28, 2023

aleksandra-shchegoleva Apr 28, 2023

aleksandra-shchegoleva Apr 28, 2023

aleksandra-shchegoleva Apr 28, 2023

aleksandra-shchegoleva Apr 28, 2023

astronik00 commented Apr 28, 2023 •

edited

Loading

astronik00 commented Apr 28, 2023

aleksandra-shchegoleva commented Apr 28, 2023

aleksandra-shchegoleva commented Apr 28, 2023

aleksandra-shchegoleva left a comment

aleksandra-shchegoleva Apr 28, 2023

astronik00 Apr 29, 2023 •

edited

Loading


		__tablename__ = 'priority'

		priority_id = Column(Integer, primary_key=True)

Create Entities #57

Create Entities #57

Conversation

astronik00 commented Apr 28, 2023 • edited Loading

Task merge request

Code changes

How to test

aleksandra-shchegoleva left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astronik00 Apr 28, 2023 • 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

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

astronik00 commented Apr 28, 2023 • edited Loading

astronik00 commented Apr 28, 2023

aleksandra-shchegoleva commented Apr 28, 2023

aleksandra-shchegoleva commented Apr 28, 2023

aleksandra-shchegoleva left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

astronik00 Apr 29, 2023 • edited Loading

Choose a reason for hiding this comment

astronik00 commented Apr 28, 2023 •

edited

Loading

astronik00 Apr 28, 2023 •

edited

Loading

astronik00 commented Apr 28, 2023 •

edited

Loading

astronik00 Apr 29, 2023 •

edited

Loading