feat: Creat notification FastAPI #465
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hyukychang
reviewed
Apr 2, 2024
There was a problem hiding this comment.
이거 제가 올린 board 쪽 refactoring PR 봐주세요
이해가 안가는 부분은 Slack에 멘션 주세요!
Comment on lines
+1
to
+6
| from fastapi import APIRouter, Depends, HTTPException | ||
| from ara.controller.authentication import get_current_user | ||
| from ara.service.notification_service import NotificationService | ||
| from ara.domain.user import User | ||
| from ara.domain.exceptions import EntityDoesNotExist | ||
| from pydantic import BaseModel |
There was a problem hiding this comment.
해당 파일은 ara/controller/notification/notification_controller.py로 빼주세요
| @@ -0,0 +1,24 @@ | |||
| from fastapi import HTTPException | |||
There was a problem hiding this comment.
해당 파일 도 경로가 틀렸습니다 제가 올린 board refactoring PR 봐주세요
| @@ -0,0 +1,24 @@ | |||
| from fastapi import HTTPException | |||
| from sqlalchemy.orm import Session | |||
There was a problem hiding this comment.
sqlalchemy 안쓰는걸로 알고 있는데.... 요건 뭔가요?
| def __init__(self, db: Session): | ||
| self.db = db | ||
|
|
||
| def get(self, notification_id: int) -> Notification: |
There was a problem hiding this comment.
Suggested change
| def get(self, notification_id: int) -> Notification: | |
| def get_by_id(self, notification_id: int) -> Notification: |
함수이름은 파라미터를 보지 않아도 딱봐도 알 수 있게 작성하는게 좋아요 👍
| self.db.add(notification) | ||
| self.db.commit() | ||
|
|
||
| def get_notifications_for_user(self, user: User) -> List[Notification]: |
There was a problem hiding this comment.
Suggested change
| def get_notifications_for_user(self, user: User) -> List[Notification]: | |
| def get_notifications_by_user(self, user: User) -> List[Notification]: |
| self.db.commit() | ||
|
|
||
| def get_notifications_for_user(self, user: User) -> List[Notification]: | ||
| notifications = self.db.query(Notification).filter(Notification.user_id == user.id).all() |
|
|
||
| def get_notifications_for_user(self, user: User) -> List[Notification]: | ||
| notifications = self.db.query(Notification).filter(Notification.user_id == user.id).all() | ||
| return notifications |
There was a problem hiding this comment.
파일 끝에 엔터로 끝내주세요!!
@injoonH 요거 우리 precommit에서 체크 안하나요?
| elif article.name_type == NameType.REGULAR: | ||
| return profile.nickname | ||
| else: | ||
| return "익명" |
There was a problem hiding this comment.
이런 상수는 constant.py에다가 넣어서 작성해 주세요 나중에 못찾습니다!
Comment on lines
+8
to
+10
| from ara.domain.article import Article # Import Article and UserProfile from appropriate paths | ||
| from ara.domain.user_profile import UserProfile | ||
| from ara.domain.comment import Comment |
There was a problem hiding this comment.
이거 domain이 안보이는데...github branch 꼬인거 같아요
Qndndn
force-pushed
the
fastapi/notification
branch
from
7442a29
to
3f00b35
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Create notification FastAPI