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

[FE] 행사 생성 시 관리자 이름 유효성 검사가 올바르게 수행되지 않는 버그와 그 외 #860

Merged
merged 3 commits into from
Dec 18, 2024
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
3 changes: 2 additions & 1 deletion client/src/constants/errorMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ export const SERVER_ERROR_MESSAGES: ErrorMessage = {
export const ERROR_MESSAGE = {
eventName: SERVER_ERROR_MESSAGES.EVENT_NAME_LENGTH_INVALID,
eventPasswordType: SERVER_ERROR_MESSAGES.EVENT_PASSWORD_FORMAT_INVALID,
memberName: `이름은 ${RULE.maxMemberNameLength}자까지 입력 가능해요.`,
memberNameLength: `이름은 ${RULE.maxMemberNameLength}자까지 입력 가능해요.`,
Copy link
Contributor

Choose a reason for hiding this comment

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

웨디가 반영해준 pr덕분에 예전에 남겨뒀던 pr이 하나 떠올랐는데요!

백엔드 측에서는 멤버 이름 글자수 제한이 4자에서 8자로 변경된 것으로 알고 있어요! 혹시 maxMemberNameLength 상수 값을 8로 변경하는 것도 이번 이슈에서 같이 반영해주실 수 있을까요~?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 반영하겠씁니다. 인지하지 못했었는데 고마워요

memberNameFormat: `이름은 한글, 영어만 가능해요.`,
Copy link
Contributor

Choose a reason for hiding this comment

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

사람들이 동명이인을 입력할 경우 어떻게 처리하는 지 궁금해요.. 이를 알 수 있는 방법이 있다면 좋을텐데..
동명이인일 때 숫자를 활용할지 아니면 영어나 다른 한글로 처리할 지 궁금해요.. 이에 따라 정책을 정하면 좋을 것 같아서요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

제가 수정한 로직 자체는 다양한 이름이 아니라 단일 이름 입력에 대해서만 검증하는 함수입니다.
그래서 다양한 이름이 들어올 경우는 이 함수가 아니라 이 함수를 이용하는 다른 검증 함수를 제작하는게 좋을 것 같아요! 하나로 다 하기에는 너무 많은 책임이 부여될 것 같네요..!

purchasePrice: `${RULE.maxPrice.toLocaleString('ko-kr')}원 이하의 숫자만 입력이 가능해요`,
purchaseTitle: `지출 이름은 ${RULE.maxBillNameLength}자 이하의 한글, 영어, 숫자만 가능해요`,
preventEmpty: '값은 비어있을 수 없어요',
Expand Down
11 changes: 4 additions & 7 deletions client/src/hooks/createEvent/useSetNicknameStep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,15 @@ const useSetNicknameStep = () => {

const handleNicknameChange = (event: React.ChangeEvent<HTMLInputElement>) => {
const name = event.target.value;
const {isValid, errorMessage: errorMessageResult} = validateMemberName(name);

const {memberName: nicknameResult, isValid, errorMessage: errorMessageResult} = validateMemberName(name);
setErrorMessage(errorMessageResult);

if (isValid) {
setNickname(name);
if (isValid || name.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

name.length === 0을 검증하는 부분에서
trim을 사용해서 길이가 0인지 확인하는 건 어떨까요?? 입력값이 ' '(공백 2개)이라면 현재 코드에서는 0이 맞기 때문에 조건문에 통과될 것 같아서요!

물론 validate 함수에서 검증을 진행해서 위와 같은 값이 들어와도 괜찮을 것 같긴 한데용..
흠.. 두번 제약을 거는게 좋을지?는 좀 고민이네요🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

꼼꼼하게 체크해줘서 고맙읍니다!!@

소하의 말은 공백 2개 입력이 들어올 경우 위 조건문이 실행된다는 뜻이 맞을까요?

isValid라는 값은 검증 함수에서 토출되는 값인데 이 isValid는 validateLength라는 내부 함수에 의해서 결정이 됩니다. 이때 validateLength는 trim 수행 후의 이름 길이가 0 초과인지 검증하고 있어요.

공백 2개 입력을 trim하면 0이기 때문에 validateLength가 false가 되어 isValid도 false가 됩니다..! 그래서 조건문에 진입하지 못해요.

혹시 설명이 되었을까요?!

setNickname(nicknameResult);
setCanSubmit(isValid);
}

setCanSubmit(name.length !== 0);
};

return {handleNicknameChange, canSubmit, nickname, errorMessage};
};

export {useSetNicknameStep, type UseSetNicknameStepProps};
22 changes: 14 additions & 8 deletions client/src/utils/validate/validateMemberName.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,28 @@ import RULE from '@constants/rule';

import {ValidateResult} from './type';

const validateMemberName = (name: string): ValidateResult => {
let errorMessage = null;
const validateMemberName = (name: string) => {
const slicedName = name.trim().slice(0, RULE.maxMemberNameLength);
Copy link
Contributor

Choose a reason for hiding this comment

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

저도 이 부분 아예 인지하지 못 했어요.. 역시 에러 처리에 꼼꼼한 웨디

Copy link
Contributor Author

Choose a reason for hiding this comment

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

사실 저도 몰랐습니다 ㅋㅋㅋㅋㅋ 맨날 하듯이 숫자입력으로 넘어가려다가 빈칸인 채로 넘어가져서 알게됐네여


const validateOnlyString = () => {
return REGEXP.memberName.test(name);
return REGEXP.memberName.test(slicedName);
};

const validateLength = () => {
return name.length <= RULE.maxMemberNameLength;
return slicedName.length > 0;
};

if (validateOnlyString() && validateLength()) {
return {isValid: true, errorMessage: null};
}
const getErrorMessage = () => {
if (!validateOnlyString()) return ERROR_MESSAGE.memberNameFormat;
if (name.length > RULE.maxMemberNameLength) return ERROR_MESSAGE.memberNameLength;
return null;
};

return {isValid: false, errorMessage: errorMessage || ERROR_MESSAGE.memberName};
return {
memberName: slicedName,
isValid: validateLength() && validateOnlyString(),
errorMessage: getErrorMessage(),
};
};

export default validateMemberName;
Loading