feat(category): change news category page posts data source from GraphQL to JSON#1452
Conversation
…category is 'news'
Summary of ChangesHello, 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! 此拉取請求重構了新聞分類頁面 ( Highlights
Changelog
Activity
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.
Code Review
這項 Pull Request 將「新聞」分類頁面的文章資料來源從 GraphQL 改為靜態 JSON,以解決潛在的文章重複問題。整體改動方向正確,但有幾個地方需要注意:
- 新的 API 呼叫函式 (
fetchNewsCategoryInfo,fetchNewsCategoryPostsJSON) 在發生錯誤時沒有正確地將錯誤拋出,這會導致頁面在生產環境中崩潰。 - 在
getServerSideProps中,新的資料獲取邏輯缺乏錯誤處理,且與現有程式碼風格不一致。 - 判斷是否為「新聞」分類的邏輯可以更精確。
我已經在相關的程式碼片段提供了具體的修改建議,以提高程式碼的穩健性和可維護性。
| } catch (err) { | ||
| console.error('Error fetching news category info: ', JSON.stringify(err)) | ||
| } |
There was a problem hiding this comment.
catch 區塊中僅將錯誤印出至 console,但未將錯誤向上拋出。這會導致函式在發生錯誤時返回 undefined,而呼叫此函式的地方 (例如 pages/category/[slug].js 中的 getServerSideProps) 在試圖對 undefined 進行解構時 (如 const { data } = ...) 會觸發 TypeError,導致頁面崩潰。建議在 catch 區塊中重新拋出錯誤,讓上層的錯誤處理機制(如 Promise.allSettled 或 try/catch)能夠捕捉到。
} catch (err) {
console.error('Error fetching news category info: ', JSON.stringify(err))
throw err
}| } catch (err) { | ||
| console.error( | ||
| 'Failed to fetch JSON of URL_STATIC_NEWS_CATEGORY_POSTS: ', | ||
| JSON.stringify(err) | ||
| ) | ||
| } |
| if (isNewsCategory && !isPremium) { | ||
| const responses = await Promise.allSettled([ | ||
| fetchHeaderDataInDefaultPageLayout(), | ||
| ]) | ||
|
|
||
| // handle header data | ||
| ;[sectionsData, topicsData] = processSettledResult( | ||
| responses[0], | ||
| getSectionAndTopicFromDefaultHeaderData, | ||
| `Error occurs while getting header data in category page (categorySlug: ${categorySlug})`, | ||
| globalLogFields | ||
| ) | ||
|
|
||
| // handle fetch post data | ||
| const { data } = await fetchNewsCategoryPostsJSON() | ||
| posts = data.posts.items || [] | ||
| postsCount = data.posts.counts || 0 | ||
| } |
There was a problem hiding this comment.
此處直接呼叫 fetchNewsCategoryPostsJSON 並解構其結果,但沒有任何錯誤處理。如果 API 請求失敗,getServerSideProps 將會拋出錯誤,導致伺服器回傳 500 錯誤頁面。建議參考此檔案中其他資料請求的作法,使用 Promise.allSettled 搭配 processSettledResult 來處理,以確保在 API 請求失敗時能有優雅的降級處理(graceful degradation)。
if (isNewsCategory && !isPremium) {
const responses = await Promise.allSettled([
fetchHeaderDataInDefaultPageLayout(),
fetchNewsCategoryPostsJSON(),
])
// handle header data
;[sectionsData, topicsData] = processSettledResult(
responses[0],
getSectionAndTopicFromDefaultHeaderData,
`Error occurs while getting header data in category page (categorySlug: ${categorySlug})`,
globalLogFields
)
// handle fetch post data
const postsWrapper = processSettledResult(
responses[1],
(res) => res?.data?.posts,
`Error occurs while getting news category posts in category page (categorySlug: ${categorySlug})`,
globalLogFields
)
posts = postsWrapper?.items || []
postsCount = postsWrapper?.counts || 0
}| const isNewsCategory = | ||
| categorySlug.startsWith('news') || categorySlug.startsWith('news?') |
There was a problem hiding this comment.
判斷是否為「新聞」分類的邏輯 categorySlug.startsWith('news') || categorySlug.startsWith('news?') 可能不夠穩健。categorySlug 是從 URL 路徑中解析而來,不包含查詢參數(query string)。因此,對於 /category/news?page=2 這樣的 URL,categorySlug 的值會是 news。startsWith('news?') 這個條件可能永遠不會為真,或者會意外匹配到名為 news-something 的分類。建議將判斷條件修改為更精確的 categorySlug === 'news',以符合預期行為。
| const isNewsCategory = | |
| categorySlug.startsWith('news') || categorySlug.startsWith('news?') | |
| const isNewsCategory = categorySlug === 'news' |
| if (isNewsCategory) { | ||
| try { | ||
| const { data } = await fetchNewsCategoryInfo() | ||
| category = data.category || category | ||
| } catch (error) { | ||
| console.error('Error fetching news category:', error) | ||
| } | ||
| } else { | ||
| try { | ||
| const { data } = await fetchCategoryByCategorySlug(categorySlug) | ||
| category = data.category || category | ||
| } catch (error) { | ||
| logGqlError( | ||
| error, | ||
| `Error occurs while getting category data in category page (${categorySlug})`, | ||
| globalLogFields | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
這段程式碼透過 if/else 處理兩種不同來源的分類資料,但錯誤處理方式不一致(console.error vs logGqlError),且程式碼結構可以更簡潔。建議將其重構,使用 Promise.allSettled 和 processSettledResult 來統一處理資料獲取與錯誤記錄,這也符合此檔案中其他資料獲取邏輯的風格。
const fetchCategoryPromise = isNewsCategory
? fetchNewsCategoryInfo()
: fetchCategoryByCategorySlug(categorySlug)
const categoryResponses = await Promise.allSettled([fetchCategoryPromise])
const categoryResult = processSettledResult(
categoryResponses[0],
(res) => res?.data?.category,
`Error occurs while getting category data in category page (${categorySlug})`,
globalLogFields
)
category = categoryResult || category| console.error( | ||
| 'Failed to fetch JSON of URL_STATIC_NEWS_CATEGORY_POSTS: ', | ||
| JSON.stringify(err) | ||
| ) |
描述
/category/news或/category/news?網址的文章來源從 GraphQL endpoint 改成 JSON URLs參考資源
news(焦點)category 的基本資訊 JSON
https://storage.googleapis.com/v3-statics-dev.mirrormedia.mg/json/latest/category_news.json
category 是 news 的新聞(非會員可瀏覽)JSON
Asana
https://app.asana.com/1/614399484723017/project/1213553957874038/task/1211779788621415?focus=true