Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification Enum Field 수정 #32

Merged
merged 1 commit into from
Nov 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions backend/notifications/models.py
Original file line number Diff line number Diff line change
@@ -1,32 +1,31 @@
from django.db import models
from enum import Enum


# TODO - state definition for notification, notification log, and reservation
class EnumNotifcationStatus(Enum):
class EnumNotifcationStatus(models.TextChoices):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaeyoungleeee
수고하셨습니다!
Python native인 Enum과 어떤 차이가 있는 걸까요?
기존에 구현해주신 Enum을 iterate하면서 직접 field 의 choice 값을 넣어주는 구현해주시는 것도 문제가 없어 보였는데, 왜 바꿔주셨는지 궁금하네요!

Copy link
Collaborator Author

@given131 given131 Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keetae423

오 좋은 리뷰 감사합니다!

우선 기존에 issue(#29) 로 정리해둔 부분의 근본 원인이 바로, enum field 를 rest_framework 에서 제대로 serialize 해주지 못했던 것으로 확인했습니다.

이를 위해, 애초에 choice 를 넣어둘 때, choice element 의 쌍을 (str, str) 으로 넣어주는 것들도 방법이지만, 이 경우 enum 을 쓰는 것이 무의미해진다고 판단하였습니다. singleton 으로서의 enum 의 기능을 상실하고, 결국 literal 이 되기 때문입니다. serialize 를 해줄 때만 저절로 enum 으로 해주는 선택지들도 있긴 했습니다만 (다른 외부 라이브러리 사용), star 수를 확인했을 때 범용적으로 사용되는 라이브러리는 별로 없더라고요.

다행히 찾던 중 django 에서 defaut 로 enum 의 usecase 와 유사한 상황에서 쓸 수 있는 TextChoices 를 제공하는 것을 확인했습니다. (https://docs.djangoproject.com/en/dev/ref/models/fields/#enumeration-types)
하여, 이 부분을 적용하고자 했습니다!

PENDING = 'PENDING'
SENDING = 'SENDING'
SUCCESS = 'SUCCESS'
PARTIAL_SUCCESS = 'PARTIAL_SUCCESS'
FAILURE = 'FAILURE'


class EnumNotificationType(Enum):
class EnumNotificationType(models.TextChoices):
EMAIL = 'EMAIL'
SMS = 'SMS'
API = 'API'


class Notification(models.Model):
message = models.TextField() # TODO - should be changed to foreign key to message
message = models.TextField() # TODO - should be changed to foreign key to message
project = models.ForeignKey('project.Project', on_delete=models.CASCADE)
status = models.CharField(max_length=20, choices=[(tag, tag.value) for tag in EnumNotifcationStatus])
type = models.CharField(max_length=20, choices=[(tag, tag.value) for tag in EnumNotificationType])
status = models.CharField(max_length=20, choices=EnumNotifcationStatus.choices)
type = models.CharField(max_length=20, choices=EnumNotificationType.choices)


class NotificationLog(models.Model):
notification = models.ForeignKey('Notification', on_delete=models.CASCADE)
status = models.CharField(max_length=20, choices=[(tag, tag.value) for tag in EnumNotifcationStatus])
status = models.CharField(max_length=20, choices=EnumNotificationType.choices)
request = models.JSONField()
response = models.JSONField()
created_at = models.DateTimeField(auto_now_add=True)
Expand All @@ -37,5 +36,5 @@ class NotificationLog(models.Model):
class Reservation(models.Model):
notifcation = models.ForeignKey('Notification', on_delete=models.CASCADE)
reserved_at = models.DateTimeField(auto_now_add=True)
status = models.CharField(max_length=20, choices=[(tag, tag.value) for tag in EnumNotifcationStatus])
status = models.CharField(max_length=20, choices=EnumNotifcationStatus.choices)
# TODO - add target user
1 change: 1 addition & 0 deletions backend/notifications/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ class NotificationSerializer(serializers.ModelSerializer):
class Meta:
model = Notification
fields = ('id', 'message', 'project', 'status', 'type',)