-
Notifications
You must be signed in to change notification settings - Fork 4
fix: when make new template, cannot find default icons #56
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
Conversation
Turtle-Hwan
commented
Dec 6, 2025
- default icon api token 없이도 호출 가능하도록 권한 변경
Summary of ChangesHello @Turtle-Hwan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue preventing default icons from loading when creating new templates by modifying API access permissions. It introduces a dedicated function for unauthenticated API requests and updates the default icon fetching mechanism to use it. Additionally, the icon loading process in the editor context has been optimized to fetch both default and user icons in parallel, enhancing performance and reliability. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
이 PR은 새 템플릿 생성 시 기본 아이콘을 불러오지 못하는 문제를 해결하기 위해, 인증 없이 기본 아이콘 API를 호출할 수 있도록 publicRequest 함수를 추가하고 이를 적용했습니다. 전반적으로 올바른 방향의 수정이지만, 새로 추가된 publicRequest 함수에서 코드 중복과 불안정한 오류 처리 방식이 발견되었습니다. 또한, EditorContext에서 Promise.allSettled를 사용하면서 API 실패 케이스를 일부 누락하여 에러가 무시될 수 있는 문제가 있습니다. 코드의 안정성과 유지보수성을 높이기 위해 제안된 사항들을 검토해주시기 바랍니다.
| const data = await response.json(); | ||
|
|
||
| if (!response.ok) { | ||
| return { success: false, status: response.status, error: { code: "ERROR", message: data.message } }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
!response.ok일 때의 오류 처리가 data.message가 항상 존재한다고 가정하고 있어 불안정합니다. 만약 에러 응답의 본문이 JSON이 아니거나 message 프로퍼티가 없다면, 에러가 발생하거나 사용자에게 도움이 되지 않는 에러 메시지가 표시될 수 있습니다. 기존 request 함수(292-298라인)처럼 상태 코드와 상태 텍스트를 포함한 폴백(fallback) 메시지를 생성하는 더 안정적인 방식으로 개선하는 것이 좋습니다.
| return { success: false, status: response.status, error: { code: "ERROR", message: data.message } }; | |
| return { success: false, status: response.status, error: { code: String(data?.code || response.status), message: data?.message || `HTTP Error: ${response.status} ${response.statusText}` } }; |
| if (iconsResult.status === 'fulfilled' && iconsResult.value.success && iconsResult.value.data) { | ||
| if (Array.isArray(iconsResult.value.data)) { | ||
| defaultIcons = iconsResult.value.data; | ||
| } else if (typeof iconsResult.value.data === 'object' && 'items' in iconsResult.value.data) { | ||
| // Handle paginated response format | ||
| defaultIcons = (iconsResult.data as { items: Icon[] }).items; | ||
| defaultIcons = (iconsResult.value.data as { items: Icon[] }).items; | ||
| } else { | ||
| console.error('[EditorContext] Unexpected response structure:', iconsResult.data); | ||
| console.error('[EditorContext] Unexpected response structure:', iconsResult.value.data); | ||
| } | ||
| } else { | ||
| console.error('[EditorContext] Icons API failed:', iconsResult.error); | ||
| } else if (iconsResult.status === 'rejected') { | ||
| console.error('[EditorContext] Icons API failed:', iconsResult.reason); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Promise.allSettled의 결과를 처리할 때, API 요청이 성공적으로 처리되었지만 비즈니스 로직상 실패한 경우(즉, status가 'fulfilled'이지만 value.success가 false인 경우)에 대한 처리가 누락되었습니다. 이로 인해 API 에러가 발생해도 조용히 무시될 수 있습니다. iconsResult와 userIconsResult 모두에 대해, fulfilled 상태이면서 success가 false일 때 에러를 기록하는 로직을 추가해야 합니다.
| if (iconsResult.status === 'fulfilled' && iconsResult.value.success && iconsResult.value.data) { | |
| if (Array.isArray(iconsResult.value.data)) { | |
| defaultIcons = iconsResult.value.data; | |
| } else if (typeof iconsResult.value.data === 'object' && 'items' in iconsResult.value.data) { | |
| // Handle paginated response format | |
| defaultIcons = (iconsResult.data as { items: Icon[] }).items; | |
| defaultIcons = (iconsResult.value.data as { items: Icon[] }).items; | |
| } else { | |
| console.error('[EditorContext] Unexpected response structure:', iconsResult.data); | |
| console.error('[EditorContext] Unexpected response structure:', iconsResult.value.data); | |
| } | |
| } else { | |
| console.error('[EditorContext] Icons API failed:', iconsResult.error); | |
| } else if (iconsResult.status === 'rejected') { | |
| console.error('[EditorContext] Icons API failed:', iconsResult.reason); | |
| } | |
| if (iconsResult.status === 'fulfilled') { | |
| if (iconsResult.value.success && iconsResult.value.data) { | |
| if (Array.isArray(iconsResult.value.data)) { | |
| defaultIcons = iconsResult.value.data; | |
| } else if (typeof iconsResult.value.data === 'object' && 'items' in iconsResult.value.data) { | |
| // Handle paginated response format | |
| defaultIcons = (iconsResult.value.data as { items: Icon[] }).items; | |
| } else { | |
| console.error('[EditorContext] Unexpected response structure:', iconsResult.value.data); | |
| } | |
| } else { | |
| console.error('[EditorContext] Default icons API failed:', iconsResult.value.error); | |
| } | |
| } else if (iconsResult.status === 'rejected') { | |
| console.error('[EditorContext] Default icons API failed:', iconsResult.reason); | |
| } |
| export async function publicRequest<T = unknown>( | ||
| url: string, | ||
| method: string, | ||
| body?: unknown, | ||
| config?: RequestConfig | ||
| ): Promise<ApiResponse<T>> { | ||
| try { | ||
| const { headers = {}, params } = config || {}; | ||
|
|
||
| const urlWithParams = buildUrl(url, params); | ||
| const fullUrl = url.startsWith("http") ? urlWithParams : `${API_BASE_URL}${urlWithParams}`; | ||
|
|
||
| const response = await fetch(fullUrl, { | ||
| method, | ||
| headers: { "Content-Type": "application/json", ...headers }, | ||
| body: body ? JSON.stringify(body) : undefined, | ||
| }); | ||
|
|
||
| const data = await response.json(); | ||
|
|
||
| if (!response.ok) { | ||
| return { success: false, status: response.status, error: { code: "ERROR", message: data.message } }; | ||
| } | ||
|
|
||
| // Extract 'result' field if present (backend response format) | ||
| const resultData = data && typeof data === 'object' && 'result' in data ? data.result : data; | ||
|
|
||
| return { success: true, status: response.status, data: resultData as T }; | ||
| } catch (error) { | ||
| return { success: false, error: { code: "NETWORK_ERROR", message: String(error) } }; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.