Skip to content

PLUMS PoT Frontend 과제 수행#1

Open
syj0396 wants to merge 14 commits intojungmir:masterfrom
syj0396:yjsong
Open

PLUMS PoT Frontend 과제 수행#1
syj0396 wants to merge 14 commits intojungmir:masterfrom
syj0396:yjsong

Conversation

@syj0396
Copy link

@syj0396 syj0396 commented Oct 1, 2024

Backend

  • 사용자 관리 API 구현 (조회, 추가, 수정, 삭제)
  • 전체 사용자 조회 시 검색, 필터링, 페이지네이션 사용 가능하도록 구현
  • username, password validation 추가
  • middleware를 사용하여 API Response Formatting (응답 성공/실패 공통)
    { "success": false, "result": null, "message": { "password": [ "password must be ..." ] } }
  • logging 추가- file에 로그 저장
  • API 테스트 코드 작성- 각 API 별 성공/실패 테스트

Frontend

  • 전체 목록 조회, 사용자 추가, 수정, 삭제 기능 구현
  • 사용자 추가, 수정 시 validation 구현
  • msw를 사용하여 서버 mocking

<Route path="/users/add" element={<AddUserPage />} />
<Route path="/users/:id" element={<UserPage />} />
<Route path="/users/edit/:id" element={<EditUserPage />} />
</Routes>
Copy link
Owner

Choose a reason for hiding this comment

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

라우팅이 테이블에 포함되지 않은 path에 대한 처리 부족

import PropTypes from 'prop-types';
import Sidebar from './Sidebar';
import CustomHeader from './CustomHeader';
//import UserPage from '../pages/UserPage';
Copy link
Owner

Choose a reason for hiding this comment

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

불필요 주석 제거 필요

<Router>
<AppLayout>
<Routes>
<Route path="/" element={<UserListPage />} />
Copy link
Owner

Choose a reason for hiding this comment

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

사용자 목록 페이지의 라우팅 path를 root로 지정한 이유

<AppLayout>
<Routes>
<Route path="/" element={<UserListPage />} />
<Route path="/users/add" element={<AddUserPage />} />
Copy link
Owner

Choose a reason for hiding this comment

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

페이지별로 라우팅을 관리하는 컴포넌트가 존재하면 좋을듯

@@ -0,0 +1,44 @@
// // src/components/Header.jsx
Copy link
Owner

Choose a reason for hiding this comment

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

주석 제거


const CustomHeader = () => {
return (
<div style={{ display: 'flex', justifyContent: 'space-between', alignItems: 'center' }}>
Copy link
Owner

Choose a reason for hiding this comment

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

antd의 Flex 컴포넌트로 대체 가능

<Sidebar />
</Sider>
<Layout>
<Header style={{paddingTop: '12px', background: '#fff'}}>
Copy link
Owner

Choose a reason for hiding this comment

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

Header child로 CustomHeader가 있어 중첩된 Header 구조가 되어 있음 CustomHeader가 Header를 대체하는 것이 좋을 듯

trigger={null}
className="sider"
style={{
height: '100vh',
Copy link
Owner

Choose a reason for hiding this comment

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

'와 "의 혼용

const navigate = useNavigate();
const items=[
{key: '1', label: 'Dashboard'},
{key: '2', label: '메일링'},
Copy link
Owner

Choose a reason for hiding this comment

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

요구 사항에 없는 메뉴 기능

);
};

export default UserSearchFilter;
Copy link
Owner

Choose a reason for hiding this comment

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

미사용 컴포넌트

setSearchValue(e.target.value);
}

const handleSearch = () => {
Copy link
Owner

Choose a reason for hiding this comment

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

없는 이름으로 검색 후 초기화 하지 않으면 검색 기능이 동작하지 않음

response = None
if not response:
response = self.get_response(request)
if hasattr(self, 'process_response'):
Copy link
Owner

Choose a reason for hiding this comment

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

process_response가 내부적으로 구현되어 있는지 체크가 정말 필요한지

'message': None
}

if hasattr(response, 'data') and getattr(response, 'data') is not None:
Copy link
Owner

Choose a reason for hiding this comment

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

walrus operation과 getattr default 옵션을 사용하면 좋을 것 같습니다.
예시) if response_data := getattr(response, "data", None):

}

useEffect(() => {
const fetchUser = async () => {
Copy link
Owner

Choose a reason for hiding this comment

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

Jotai나 hook을 사용해서 관리하면 좋을 것 같습니다.

class UserViewSet(viewsets.ModelViewSet):
queryset = User.objects.all().order_by('id')
serializer_class = UserSerializer
filter_backends = [SearchFilter, DjangoFilterBackend]
Copy link
Owner

Choose a reason for hiding this comment

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

퍼블릭하게 사용할 필터 백엔드는 settings.py에 DEFAULT_FILTER_BACKENDS 옵션으로 관리하면 좋습니다.

class UserSerializer(serializers.ModelSerializer):
class Meta:
model = User
fields = ['id', 'username', 'email', 'password', 'is_active', 'is_admin']
Copy link
Owner

Choose a reason for hiding this comment

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

index는 read-only로 설정하는게 안전합니다.

)
email = models.EmailField(unique=True)
password = models.CharField(
max_length=16,
Copy link
Owner

@jungmir jungmir Oct 9, 2024

Choose a reason for hiding this comment

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

Django에서는 패스워드 저장할 때 해싱하여 저장하기 떄문에 글자 수가 16글자를 넘습니다.
migration 파일 보시면 max_length가 128로 설정되어 있습니다.

"""
logger = logging.getLogger(__name__)

class ResponseFormattingMiddleware:
Copy link
Owner

Choose a reason for hiding this comment

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

응답에 대한 formatting은 일반적으로 middleware보단 view / viewset에서 하는 편입니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants