Conversation
Add support for including this year BW leaderboards as well as last years. Remove code duplication and extract the spreadsheets interaction into a small services function.
elisescu
force-pushed
the
updateBWLeaderboards
branch
from
ae969e4 to
f6982c8
Compare
Contributor
📝 WalkthroughWalkthroughThis PR removes the 2024 Bridgewater leaderboard implementation and introduces a 2025 version. It simultaneously refactors the data-fetching logic into a centralized service function ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@front_end/src/services/google_spreadsheets.ts`:
- Around line 9-51: The catch block in the Google Sheets fetch logic currently
swallows all errors (credentialsBase64 parsing, GoogleAuth,
sheets.spreadsheets.get, sheets.spreadsheets.values.get) by logging and
returning an empty array; instead propagate the failure so callers can show an
error state — replace the console.error + return [] with rethrowing the original
error (or throw a new Error that wraps it) so the calling code receives the
exception and can handle/display it appropriately.
🧹 Nitpick comments (1)
front_end/src/services/google_spreadsheets.ts (1)
26-46: Filter blank sheet titles and handle per-sheet failures independently.Mapping missing titles to
""creates invalid ranges (like!A1:Z), and without per-sheet error handling, a single sheet request failure causes the entire load to fail and return empty results. Filter empty sheet names and wrap each sheet fetch in a try-catch so one bad sheet doesn't prevent retrieving data from others.♻️ Proposed refactor
- const sheetNames = spreadsheet.data.sheets?.map( - (sheet) => sheet.properties?.title || "" - ); + const sheetNames = (spreadsheet.data.sheets ?? []) + .map((sheet) => sheet.properties?.title || "") + .filter((name) => name.trim().length > 0); @@ - const allSheetsData = await Promise.all( - sheetNames.map(async (sheetName) => { - const response = await sheets.spreadsheets.values.get({ - spreadsheetId, - range: `${sheetName}!A1:Z`, - }); - return { - name: sheetName, - data: response.data.values || [], - }; - }) - ); + const allSheetsData = ( + await Promise.all( + sheetNames.map(async (sheetName) => { + try { + const response = await sheets.spreadsheets.values.get({ + spreadsheetId, + range: `${sheetName}!A1:Z`, + }); + return { + name: sheetName, + data: response.data.values || [], + }; + } catch (error) { + console.error(`Error fetching sheet "${sheetName}"`, error); + return null; + } + }) + ) + ).filter(Boolean) as { name: string; data: string[][] }[];
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/opengraph-image.jpgis excluded by!**/*.jpgfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/twitter-image.jpgis excluded by!**/*.jpg
📒 Files selected for processing (31)
front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2024/leaderboards/page.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/components/header.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/components/hero-section.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/components/pixels-tags.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/components/registration-forms.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/components/registration-status.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/components/results-announcement.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/components/results-dates.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/components/results-hero.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/components/results-leaderboard.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/components/results-prize.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/components/typography.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/constants.tsfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/contest-rules/page.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/how-it-works/page.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/layout.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/leaderboards/page.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/notice-at-collection/page.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/page.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/q1/components/announcement.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/q1/components/dates.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/q1/components/disclaimer.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/q1/components/hero.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/q1/components/openLeaderboard.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/q1/components/prize.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/q1/components/undergradLeaderboard.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/q1/page.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/leaderboards/leaderboard-tabs.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/leaderboards/page.tsxfront_end/src/app/(campaigns-registration)/(bridgewater)/components/leaderboard-tabs.tsxfront_end/src/services/google_spreadsheets.ts
💤 Files with no reviewable changes (2)
- front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/leaderboards/leaderboard-tabs.tsx
- front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2024/leaderboards/page.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/leaderboards/page.tsx (3)
front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/page.tsx (1)
Page(72-147)front_end/src/services/google_spreadsheets.ts (1)
getAllSheetsData(5-53)front_end/src/app/(campaigns-registration)/(bridgewater)/components/leaderboard-tabs.tsx (1)
LeaderboardTabs(26-182)
front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/leaderboards/page.tsx (1)
front_end/src/services/google_spreadsheets.ts (1)
getAllSheetsData(5-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build Docker Image
- GitHub Check: integration-tests
- GitHub Check: Backend Checks
🔇 Additional comments (2)
front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater/leaderboards/page.tsx (1)
10-41: LGTM: server-side fetch + shared tabs wiring is clean.
Nice consolidation with the shared sheets service and tabs component.front_end/src/app/(campaigns-registration)/(bridgewater)/bridgewater-2025/leaderboards/page.tsx (1)
10-41: LGTM for the 2025 leaderboard page flow.
Data fetch + highlighted user wiring into the shared tabs component looks consistent.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Comment on lines
+9
to
+51
| try { | ||
| const credentials = JSON.parse( | ||
| Buffer.from(credentialsBase64, "base64").toString() | ||
| ); | ||
|
|
||
| const auth = new google.auth.GoogleAuth({ | ||
| credentials, | ||
| scopes: ["https://www.googleapis.com/auth/spreadsheets.readonly"], | ||
| }); | ||
|
|
||
| const sheets = google.sheets({ version: "v4", auth }); | ||
|
|
||
| // First, get all sheet names | ||
| const spreadsheet = await sheets.spreadsheets.get({ | ||
| spreadsheetId, | ||
| }); | ||
|
|
||
| const sheetNames = spreadsheet.data.sheets?.map( | ||
| (sheet) => sheet.properties?.title || "" | ||
| ); | ||
|
|
||
| if (!sheetNames || sheetNames.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| // Then get data for all sheets | ||
| const allSheetsData = await Promise.all( | ||
| sheetNames.map(async (sheetName) => { | ||
| const response = await sheets.spreadsheets.values.get({ | ||
| spreadsheetId, | ||
| range: `${sheetName}!A1:Z`, | ||
| }); | ||
| return { | ||
| name: sheetName, | ||
| data: response.data.values || [], | ||
| }; | ||
| }) | ||
| ); | ||
|
|
||
| return allSheetsData; | ||
| } catch (error) { | ||
| console.error("Error fetching sheet data:", error); | ||
| return []; |
Contributor
There was a problem hiding this comment.
Surface fetch/auth failures instead of silently returning empty data.
Right now any error (bad credentials, API outage) is swallowed and the caller only sees “No data found,” masking a real failure. Consider rethrowing (or otherwise propagating) so the page can show a proper error state.
🐛 Suggested change
} catch (error) {
console.error("Error fetching sheet data:", error);
- return [];
+ throw error;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| const credentials = JSON.parse( | |
| Buffer.from(credentialsBase64, "base64").toString() | |
| ); | |
| const auth = new google.auth.GoogleAuth({ | |
| credentials, | |
| scopes: ["https://www.googleapis.com/auth/spreadsheets.readonly"], | |
| }); | |
| const sheets = google.sheets({ version: "v4", auth }); | |
| // First, get all sheet names | |
| const spreadsheet = await sheets.spreadsheets.get({ | |
| spreadsheetId, | |
| }); | |
| const sheetNames = spreadsheet.data.sheets?.map( | |
| (sheet) => sheet.properties?.title || "" | |
| ); | |
| if (!sheetNames || sheetNames.length === 0) { | |
| return []; | |
| } | |
| // Then get data for all sheets | |
| const allSheetsData = await Promise.all( | |
| sheetNames.map(async (sheetName) => { | |
| const response = await sheets.spreadsheets.values.get({ | |
| spreadsheetId, | |
| range: `${sheetName}!A1:Z`, | |
| }); | |
| return { | |
| name: sheetName, | |
| data: response.data.values || [], | |
| }; | |
| }) | |
| ); | |
| return allSheetsData; | |
| } catch (error) { | |
| console.error("Error fetching sheet data:", error); | |
| return []; | |
| try { | |
| const credentials = JSON.parse( | |
| Buffer.from(credentialsBase64, "base64").toString() | |
| ); | |
| const auth = new google.auth.GoogleAuth({ | |
| credentials, | |
| scopes: ["https://www.googleapis.com/auth/spreadsheets.readonly"], | |
| }); | |
| const sheets = google.sheets({ version: "v4", auth }); | |
| // First, get all sheet names | |
| const spreadsheet = await sheets.spreadsheets.get({ | |
| spreadsheetId, | |
| }); | |
| const sheetNames = spreadsheet.data.sheets?.map( | |
| (sheet) => sheet.properties?.title || "" | |
| ); | |
| if (!sheetNames || sheetNames.length === 0) { | |
| return []; | |
| } | |
| // Then get data for all sheets | |
| const allSheetsData = await Promise.all( | |
| sheetNames.map(async (sheetName) => { | |
| const response = await sheets.spreadsheets.values.get({ | |
| spreadsheetId, | |
| range: `${sheetName}!A1:Z`, | |
| }); | |
| return { | |
| name: sheetName, | |
| data: response.data.values || [], | |
| }; | |
| }) | |
| ); | |
| return allSheetsData; | |
| } catch (error) { | |
| console.error("Error fetching sheet data:", error); | |
| throw error; | |
| } |
🤖 Prompt for AI Agents
In `@front_end/src/services/google_spreadsheets.ts` around lines 9 - 51, The catch
block in the Google Sheets fetch logic currently swallows all errors
(credentialsBase64 parsing, GoogleAuth, sheets.spreadsheets.get,
sheets.spreadsheets.values.get) by logging and returning an empty array; instead
propagate the failure so callers can show an error state — replace the
console.error + return [] with rethrowing the original error (or throw a new
Error that wraps it) so the calling code receives the exception and can
handle/display it appropriately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add support for including this year BW leaderboards as well as last years. Remove code duplication and extract the spreadsheets interaction into a small services function.
Summary by CodeRabbit
New Features
Removed Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.