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] 최신소식 추가/삭제 에러 해결 #198

Merged
merged 2 commits into from
Jan 18, 2025
Merged

[FIX] 최신소식 추가/삭제 에러 해결 #198

merged 2 commits into from
Jan 18, 2025

Conversation

wuzoo
Copy link
Member

@wuzoo wuzoo commented Jan 16, 2025

✨ 구현 기능 명세

fetcher 반영 및 axios 엔드포인트에 config.ORG_API_URL 반영

✅ PR Point

기존에 news post API를 axios로 보내고 있었는데, 엔드포인트에 env production을 적용하지 않았어서 발생하였습니다. 이는 fetcher 적용하면 해결되긴 하는데 news post 경우 fetcher 사용하면 자꾸 500에러 발생해서 일단 기존 axios 유지하되 config의 어드민 API 엔드포인트 받아와서 넣어주었어요.

@wuzoo wuzoo added the 🐛 bug Something isn't working label Jan 16, 2025
@wuzoo wuzoo self-assigned this Jan 16, 2025
Copy link

height bot commented Jan 16, 2025

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link
Member

@lydiacho lydiacho left a comment

Choose a reason for hiding this comment

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

저거 바꾸면 해결될 것 같네요
놀라운 사실, 어드민에서는 따로 추가 관리하는 환경변수가 없습니다... NEXT_PUBLIC_ORG_API 이거 검색해보면 주용오빠 혼자 쓰고있음요ㅠ
image

주용 혼자만의 환경변수를 쓰고있으니 주용 로컬에서는 잘되는게 다른데서는 터졌던거였네여

이제 해결되지 않을까 싶고.. 근데 post랑 delete 두개는 fetcher쓸지 axios 쓸지 통일하는게 낫지 않을까 싶네요. post가 fetcher 쓸 때 에러나면 그냥 post랑 delete 둘다 config.ORG_API_URL 가져와서 axios로 쏴주는게 나을 것 같아요. (아니면 500 해결해서 둘다 fetcher 쓰거나)

+) 추가로, 변경사항엔 없는데 getAdminInfo는 혹시 뭐하는 친구인가요? query param으로 34기 값이 하드하게 박혀있어서 뭔가 불필요한 함수거나 수정되어야할 코드로 보여요!

@wuzoo
Copy link
Member Author

wuzoo commented Jan 16, 2025

놀라운 사실, 어드민에서는 따로 추가 관리하는 환경변수가 없습니다... NEXT_PUBLIC_ORG_API 이거 검색해보면 주용오빠 혼자 쓰고있음요ㅠ

저도 놀랍네요 .. 하하.. 첫 어드민 개발할때 API 테스트해볼려구 env에 넣어놓고 계속 사용한거 같네요 ..

그냥 post랑 delete 둘다 config.ORG_API_URL 가져와서 axios로 쏴주는게 나을 것 같아요. (아니면 500 해결해서 둘다 fetcher 쓰거나)

넹 둘 다 axios로 변경할게요

getAdminInfo/admin 데이터 조회하는 api에요. 현재 최신소식을 리스트로 조회할 수 있는 api가 따로 존재하지 않아서, admin 데이터 받아온 다음에 NewsSection에 news 데이터들만 넘기고 있는 구조임니다

Copy link

cloudflare-workers-and-pages bot commented Jan 16, 2025

Deploying sopt-admin with  Cloudflare Pages  Cloudflare Pages

Latest commit: 51c4404
Status: ✅  Deploy successful!
Preview URL: https://2c08f3dc.sopt-admin.pages.dev
Branch Preview URL: https://bug-org-news.sopt-admin.pages.dev

View logs

Copy link
Member

@eonseok-jeon eonseok-jeon left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~

@wuzoo wuzoo merged commit 7b53e9b into dev Jan 18, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants