Feature/permission NEW permission system #440
Conversation
apps/user/models/group.py
Outdated
| @staticmethod | ||
| def search_by_name(name: str) -> list["Group"]: | ||
| return Group.objects.filter(name=name) |
There was a problem hiding this comment.
group name이 같을 수도 있나요? name에 unique가 없는 이유가 있을까요
There was a problem hiding this comment.
코드 상에 group name을 사용해서 구분하는 경우가 없을것 같아서 unique를 빼뒀습니다.
같을 일은 없을 것 같은데, name은 원래 기획한것은 display name으로 생각하고 만든거긴 합니다. 따로 slug를 추가하는게 좋을까요?
There was a problem hiding this comment.
아 저도 unique가 필요해보이진 않는데, list로 반환하길래 물어봤어요
name이 같은 group이 없는게 보장되면 search_by_name도 Group 만 반환하는건 어떨까요?
사용하는 곳에서도 group으로 사용하는데 list["group"]을 반환하고 있었네요. 이건 문제 없나요?
There was a problem hiding this comment.
(디스코드 Group기능을 참고하여 생각했음)
There was a problem hiding this comment.
현재 코드에서는 Group.search_by_name()이 사용되는 곳이 없군요.
apps/user/models/group.py
Outdated
| def search_by_name(name: str) -> list["Group"]: | ||
| return Group.objects.filter(name=name) | ||
|
|
There was a problem hiding this comment.
그리고 search_by를 method명으로 많이 사용하나요? 뭔가 어색하네.. get_by? find_by? 모르겠다..
apps/user/migrations/0022_group_remove_userprofile_group_and_more.py
Outdated
Show resolved
Hide resolved
| default_groups = { | ||
| 1: ("Unauthorized user", "뉴아라 계정을 만들지 않은 사람들", False), | ||
| 2: ("KAIST member", "카이스트 메일을 가진 사람 (학생, 교직원)", False), | ||
| 3: ("Store employee", "교내 입주 업체 직원", False), | ||
| 4: ("Other member", "카이스트 메일이 없는 개인 (특수한 관련자 등)", False), | ||
| 5: ("KAIST organization", "교내 학생 단체들", True), | ||
| 6: ("External organization", "외부인 (홍보 계정 등)", True), | ||
| 7: ("Communication board admin", "소통게시판 관리인", False), | ||
| 8: ("News board admin", "뉴스게시판 관리인", False), | ||
| } | ||
|
|
There was a problem hiding this comment.
그리고 row로 관리하는 건 좋은데, group constant를 어디 만들어두고 관리하면 좋지 않을까요? magic number를 쓰는 곳이 많이 보여서, 나중에 관리가 어렵지 않을까 싶습니다
There was a problem hiding this comment.
원래 이게 기존 방식입니다.
class UserGroup(models.IntegerChoices):
# 뉴아라 계정을 만들지 않은 사람들
UNAUTHORIZED = 0, gettext_lazy("Unauthorized user")
# 카이스트 메일을 가진 사람 (학생, 교직원)
KAIST_MEMBER = 1, gettext_lazy("KAIST member")
# 교내 입주 업체 직원
STORE_EMPLOYEE = 2, gettext_lazy("Store employee")
# 카이스트 메일이 없는 개인 (특수한 관련자 등)
OTHER_MEMBER = 3, gettext_lazy("Other member")
# 교내 학생 단체들
KAIST_ORG = 4, gettext_lazy("KAIST organization")
# 외부인 (홍보 계정 등)
EXTERNAL_ORG = 5, gettext_lazy("External organization")
# 소통게시판 관리인
COMMUNICATION_BOARD_ADMIN = 6, gettext_lazy("Communication board admin")
# 뉴스게시판 관리인
NEWS_BOARD_ADMIN = 7, gettext_lazy("News board admin")하지만 이 방식으로 할 경우 그룹 추가될 경우에는 저것들에도 다 반영을 해줘야 됩니다.
이번에 그룹 작업한 이유가 요거 제거하고 board permission에서 사용하던 bitmask 제거하고 db상에서 바로 퍼미션 확인할 수 있도록, 그리고 그룹 추가해도 다른 시스템에 문제가 없도록 하기 위해서입니다.
이거는 다른분들 의견 한번 들어봐도 될 듯 하네요
There was a problem hiding this comment.
요건 얘기 해보시죵
DB에서 확인하는건 좋은데, 그러면 코드에서 Group을 특정해서 사용해야할 때는 1,2,3 계속 integer로 사용해야하다보니 실수가 생길 수 있을 것 같아서요
There was a problem hiding this comment.
최대한 코드상에서 id를 사용한 처리를 안하기로 하였습니다.
나중에 TF로 로그인 시 이메일에 따라서 어떤 역할(그룹)을 줘야되는지 처리되도록 만들어두겠습니다.
There was a problem hiding this comment.
또한 현재 ID 7번인 "소통게시판 관리인(카이스트 직원)"에 관련된 보드같은 경우에는 최대한 userprofile로 is_school_admin을 빼뒀지만, 거기는 특수 게시판 이므로 어쩔 수 없이 사용되어야만 합니다.
apps/user/migrations/0022_group_remove_userprofile_group_and_more.py
Outdated
Show resolved
Hide resolved
| group_id = models.AutoField( | ||
| verbose_name="Group ID", | ||
| primary_key=True, | ||
| db_index=True, | ||
| null=False, | ||
| ) |
There was a problem hiding this comment.
❗ Django에서 automatic primary key를 자동으로 설정하기도 하고 group.group_id보다 group.id라고 적는 게 보기 좋아서 이 부분은 삭제해도 될 것 같습니다.
| description = models.CharField( | ||
| verbose_name="Group description", | ||
| max_length=128, | ||
| null=True, |
There was a problem hiding this comment.
blank=True도 같이 설정해야 admin 페이지에서 필수값으로 설정되지 않습니다.
| @@ -0,0 +1,99 @@ | |||
| # Generated by Django 4.2.3 on 2024-01-10 17:16 | |||
There was a problem hiding this comment.
❗ 파일명 0059_remove_board_comment_access_mask_and_more.py로 수정해야 합니다.
No description provided.