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

fix: 첨부링크가 post 요청하지 못하는 이슈를 해결한다 #123

Merged
merged 11 commits into from
Mar 3, 2024

Conversation

baegyeong
Copy link
Collaborator

@baegyeong baegyeong commented Mar 3, 2024

주요 변경사항

  • 포트폴리오, 파일 등의 링크를 post 요청 보내지 못하는 이슈를 해결했습니다.
  • 그 과정에서 발견한 복수전공, 부전공을 post 요청 보내지 못하는 이슈를 해결했습니다.
  • 포트폴리오 및 파일 링크들을 tf가 열람하려고 할 때, 여러 개의 링크가 각각 이동하지 못하고 한번에 이동하는 이슈도 같이 해결했습니다.
  • pdf viewer에 포트폴리오를 볼 수 있도록 포트폴리오 컴포넌트 분리 후 추가했습니다.

리뷰어에게...

수정이 필요한 부분이 있다면 알려주세요.

관련 이슈

closes #101

체크리스트

  • reviewers 설정
  • label 설정
  • milestone 설정

@baegyeong baegyeong self-assigned this Mar 3, 2024
Copy link
Collaborator

@loopy-lim loopy-lim left a comment

Choose a reason for hiding this comment

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

,로 나눈다라... 사실 사용자가 그렇게 해줄지는 모르지만... 조금더 케이스가 나오겠죠 아마도?

Copy link
Collaborator

@geongyu09 geongyu09 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

frontend/src/functions/getApplication.ts Outdated Show resolved Hide resolved
@baegyeong
Copy link
Collaborator Author

getApplicationNames function에서 필수인 질문만 집합에 담는 구문이 있어서, 처음에는 필수일 때만 담는다는 조건을 해제했습니다. 그랬더니 필수가 아닌 질문들이 예외처리에 걸려서 다음 질문으로 넘어가지 않아서, useSendApplication에서 필수가 아닌 항목들은 localStorage에 접근해서 따로 추가해주는 식으로 바꿨습니다.

@baegyeong
Copy link
Collaborator Author

,로 나눈다라... 사실 사용자가 그렇게 해줄지는 모르지만... 조금더 케이스가 나오겠죠 아마도?

931fe5e 커밋 추가했는데.. 어떤가요?

Comment on lines 59 to 78
portfolio &&
sendValues.push({
name: "portfolio",
answer: portfolio,
});
fileUrl &&
sendValues.push({
name: "fileUrl",
answer: fileUrl,
});
doubleMajor &&
sendValues.push({
name: "doubleMajor",
answer: doubleMajor,
});
minor &&
sendValues.push({
name: "minor",
answer: minor,
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

@baegyeong
이 코드는 다시보니... 네... 좀 그렇습니다... ㅋㅋㅋ 그래서 조금 다른 방식을 추천 해드립니다.
getApplicationNames(node)를 받는 부분에서 requirement라는 파라미터를 추가하여 default는 true로 하여 만들면 크게 코드 안고치고 할 수 있지 않을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저도 하면서 좀...ㅋㅋㅋㅋㅋ 바꿔보겠읍니다..

@loopy-lim
Copy link
Collaborator

931fe5e 커밋 추가했는데.. 어떤가요?

미ㅏ쳐ㅑㅆ다.... ㄷㄷㄷㄷ

Copy link
Collaborator

@2yunseong 2yunseong left a comment

Choose a reason for hiding this comment

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

👍 👍

- pdf viewer에 포트폴리오를 추가한다
Copy link
Collaborator

@2yunseong 2yunseong left a comment

Choose a reason for hiding this comment

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

컴포넌트가 깔끔해졌네요 👍 👍

Copy link
Collaborator

@loopy-lim loopy-lim left a comment

Choose a reason for hiding this comment

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

좋네요!! 혹시.. 가능하시다면 component postfix를 제거해주실 수 있을까요?(파일명 입니다)

@baegyeong baegyeong merged commit ab8e37c into main Mar 3, 2024
1 check passed
@2yunseong 2yunseong deleted the fix/101-portfolio-link branch September 9, 2024 05:56
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.

[FE/BE] bug: pdf viewer와 지원서 열람 시 첨부링크를 띄운다.
4 participants