Conversation
* fix: migrate Foursquare API to new Places API endpoints * docs: update readme * feat: add fetch error handling to the Foursquare API example --------- Co-authored-by: Yashar Fakhari <6448697+YasharF@users.noreply.github.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
| @@ -32,29 +32,41 @@ exports.getApi = (req, res) => { | |||
| */ | |||
| exports.getFoursquare = async (req, res, next) => { | |||
There was a problem hiding this comment.
The next parameter is declared but is no longer used within the getFoursquare function since the catch block now handles the error response directly. It's good practice to remove unused parameters to keep the code clean.
| exports.getFoursquare = async (req, res, next) => { | |
| exports.getFoursquare = async (req, res) => { |
There was a problem hiding this comment.
I've removed the unused next parameter from the getFoursquare function as suggested. The error handling is already managed within the catch block.
There was a problem hiding this comment.
Disclaimer: This feature is in preview.
I've addressed the review comment by removing the unused next parameter from the getFoursquare function in controllers/api.js. The error handling is already managed within the catch block, which renders the error page directly.
I've pushed the changes to the branch test-03.
Changes:
- Modified
controllers/api.js: Removednextfrom thegetFoursquarefunction signature.
Let me know if you have any other questions!
![gemini-code-assist-test[bot]](https://avatars.githubusercontent.com/u/182566621?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Foursquare integration to the Places API, refactors the Lob and PayPal examples, and introduces a Pinterest board creation feature. However, several critical issues were identified: a large block of code (Trakt, PubChem, and Wikipedia APIs) was accidentally deleted, and the removal of uploadMiddleware will cause a ReferenceError in the file upload route. Additionally, there are bugs in the Foursquare implementation regarding the API version header format, the authorization header prefix, and incorrect property access for coordinates in the view template. Finally, an environment variable mismatch was found in the Lob API configuration.
| }); | ||
| }; | ||
|
|
||
| exports.postFileUpload = (req, res) => { |
There was a problem hiding this comment.
The uploadMiddleware function has been removed from this file, but it is still referenced in the route definition within app.js (line 231). This will lead to a ReferenceError when the application attempts to handle file uploads. Additionally, the removal of validation logic for multerError and file presence in postFileUpload makes the feature less robust.
| method: 'GET', | ||
| headers: { | ||
| accept: 'application/json', | ||
| 'X-Places-Api-Version': '2025-06-17', |
There was a problem hiding this comment.
The X-Places-Api-Version header for the Foursquare Places API typically expects a date in YYYYMMDD format (e.g., 20231010). The value 2025-06-17 uses an unsupported format and refers to a future date, which will likely cause the API request to fail.
| 'X-Places-Api-Version': '2025-06-17', | |
| 'X-Places-Api-Version': '20231010', |
| pageTitle, | ||
| error, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
This commit deletes a large block of code (approximately 500 lines) containing several other API examples (Trakt, PubChem, Wikipedia). This appears to be an accidental deletion as it is not mentioned in the pull request description and significantly reduces the project's functionality.
| img.img-thumbnail(src=photo.prefix + '400x400' + photo.suffix, alt=venueDetail.name, width='100%') | ||
| else | ||
| p No photos available. | ||
| | located at #{ venueDetail.location.address || 'N/A' }, #{ venueDetail.location.locality || 'N/A' }, #{ venueDetail.location.region || 'N/A' }. (longitude: #{ venueDetail.longitude }, latitude: #{ venueDetail.latitude }) |
There was a problem hiding this comment.
The Foursquare Places API returns venue coordinates within a nested geocodes.main object. Accessing venueDetail.longitude and venueDetail.latitude directly will result in undefined values in the rendered view. The previous implementation correctly used the nested path.
| located at #{ venueDetail.location.address || 'N/A' }, #{ venueDetail.location.locality || 'N/A' }, #{ venueDetail.location.region || 'N/A' }. (longitude: #{ venueDetail.geocodes.main.longitude }, latitude: #{ venueDetail.geocodes.main.latitude })
| headers: { | ||
| accept: 'application/json', | ||
| 'X-Places-Api-Version': '2025-06-17', | ||
| authorization: `Bearer ${process.env.FOURSQUARE_APIKEY}`, |
There was a problem hiding this comment.
The Foursquare Places API (v3) expects the API key directly in the Authorization header without the Bearer prefix. Including this prefix may result in authentication errors.
| authorization: `Bearer ${process.env.FOURSQUARE_APIKEY}`, | |
| authorization: process.env.FOURSQUARE_APIKEY, |
| const config = new LobConfiguration({ | ||
| username: process.env.LOB_KEY, | ||
| const lobConfig = new LobConfiguration({ | ||
| username: process.env.LOB_API_KEY, |
There was a problem hiding this comment.
There is a mismatch between the environment variable name used here (LOB_API_KEY) and the one defined in .env.example (LOB_KEY). This inconsistency will cause the Lob API integration to fail for users following the provided configuration example.
| username: process.env.LOB_API_KEY, | |
| username: process.env.LOB_KEY, |
There was a problem hiding this comment.
Code Review
This pull request updates the Foursquare integration to the latest Places API, refactors the Lob and Google API controllers to use async/await, and introduces new Pinterest board creation functionality. However, several critical issues were identified: the removal of multiple exported functions will cause the application to crash due to existing route references in app.js, and several API integrations (Foursquare, Lob, Pinterest, Google Drive/Sheets) contain bugs ranging from incorrect authentication headers and environment variable mismatches to missing null checks and invalid property access in views. Additionally, error handling and session persistence logic were removed from the file upload process, which may lead to a degraded user experience.
| @@ -1033,655 +1036,167 @@ exports.getPayPalCancel = (req, res) => { | |||
| * Lob API example. | |||
| */ | |||
| exports.getLob = async (req, res, next) => { | |||
There was a problem hiding this comment.
This change removes several exported functions (e.g., getHereMaps, getGoogleMaps, getTrakt, getPubChem, getWikipedia) that are still referenced as routes in app.js (lines 232-240). This will cause the application to crash on startup with a callback error. Additionally, uploadMiddleware has been removed but is still required by the /api/upload route in app.js (line 231).
| headers: { | ||
| accept: 'application/json', | ||
| 'X-Places-Api-Version': '2025-06-17', | ||
| authorization: `Bearer ${process.env.FOURSQUARE_APIKEY}`, |
There was a problem hiding this comment.
The Foursquare Places API expects the API key to be provided directly in the Authorization header without the Bearer prefix. Including Bearer will likely result in authentication errors.
| authorization: `Bearer ${process.env.FOURSQUARE_APIKEY}`, | |
| authorization: process.env.FOURSQUARE_APIKEY, |
| const config = new LobConfiguration({ | ||
| username: process.env.LOB_KEY, | ||
| const lobConfig = new LobConfiguration({ | ||
| username: process.env.LOB_API_KEY, |
There was a problem hiding this comment.
There is a mismatch between the environment variable name used in the code (LOB_API_KEY) and the one defined in .env.example (LOB_KEY at line 66). This will cause the Lob API integration to fail for users following the example configuration. This also applies to line 1063.
| username: process.env.LOB_API_KEY, | |
| username: process.env.LOB_KEY, |
| if (!fs.existsSync(imageCacheDir)) { | ||
| fs.mkdirSync(imageCacheDir, { recursive: true }); // Ensures that parent directories are created | ||
| } | ||
| const token = req.user.tokens.find((token) => token.kind === 'pinterest'); |
There was a problem hiding this comment.
The code does not check if the pinterest token exists before attempting to access token.accessToken. If a user who hasn't linked their Pinterest account submits the form, the application will throw a TypeError and crash.
| const token = req.user.tokens.find((token) => token.kind === 'pinterest'); | |
| const token = req.user.tokens.find((token) => token.kind === 'pinterest'); | |
| if (!token) { | |
| req.flash('errors', { msg: 'Please link your Pinterest account first.' }); | |
| return res.redirect('/api/pinterest'); | |
| } |
| img.img-thumbnail(src=photo.prefix + '400x400' + photo.suffix, alt=venueDetail.name, width='100%') | ||
| else | ||
| p No photos available. | ||
| | located at #{ venueDetail.location.address || 'N/A' }, #{ venueDetail.location.locality || 'N/A' }, #{ venueDetail.location.region || 'N/A' }. (longitude: #{ venueDetail.longitude }, latitude: #{ venueDetail.latitude }) |
There was a problem hiding this comment.
The Foursquare Places API response structure places coordinates inside geocodes.main. Accessing venueDetail.longitude and venueDetail.latitude directly will result in undefined values being rendered.
| located at #{ venueDetail.location.address || 'N/A' }, #{ venueDetail.location.locality || 'N/A' }, #{ venueDetail.location.region || 'N/A' }. (longitude: #{ venueDetail.geocodes.main.longitude }, latitude: #{ venueDetail.geocodes.main.latitude })
| exports.postFileUpload = (req, res) => { | ||
| if (!req.file && req.multerError) { | ||
| if (req.multerError.code === 'LIMIT_FILE_SIZE') { | ||
| req.flash('errors', { | ||
| msg: 'File size is too large. Maximum file size allowed is 1MB', | ||
| }); | ||
| // Save the session to ensure flash is persisted before redirect to | ||
| // avoid race conditions with async session stores | ||
| return req.session.save(() => res.redirect('/api/upload')); | ||
| } | ||
| req.flash('errors', { msg: req.multerError.message }); | ||
| // Save the session to ensure flash is persisted before redirect | ||
| return req.session.save(() => res.redirect('/api/upload')); | ||
| } | ||
|
|
||
| req.flash('success', { msg: 'File was uploaded successfully.' }); | ||
| // Save the session to ensure flash is persisted before redirect | ||
| return req.session.save(() => res.redirect('/api/upload')); | ||
| }; | ||
|
|
||
| exports.uploadMiddleware = (req, res, next) => { | ||
| // configure Multer with a 1 MB limit | ||
| const upload = multer({ | ||
| dest: path.join(__dirname, '../uploads'), | ||
| limits: { fileSize: 1024 * 1024 * 1 }, | ||
| }); | ||
| upload.single('myFile')(req, res, (err) => { | ||
| if (err) { | ||
| req.multerError = err; | ||
| } | ||
| next(); | ||
| }); | ||
| }; | ||
|
|
||
| exports.getHereMaps = (req, res) => { | ||
| res.render('api/here-maps', { | ||
| apikey: process.env.HERE_API_KEY, | ||
| title: 'Here Maps API', | ||
| }); | ||
| }; | ||
|
|
||
| exports.getGoogleMaps = (req, res) => { | ||
| res.render('api/google-maps', { | ||
| title: 'Google Maps API', | ||
| google_map_api_key: process.env.GOOGLE_MAP_API_KEY, | ||
| }); | ||
| }; | ||
|
|
||
| exports.getGoogleDrive = (req, res) => { | ||
| const token = req.user.tokens.find((token) => token.kind === 'google'); | ||
| const authObj = new googledrive.auth.OAuth2({ | ||
| access_type: 'offline', | ||
| }); | ||
| authObj.setCredentials({ | ||
| access_token: token.accessToken, | ||
| }); | ||
| const drive = googledrive.drive({ | ||
| version: 'v3', | ||
| auth: authObj, | ||
| }); | ||
|
|
||
| const errorMsgPermission = 'Missing Google Drive access permission. Please unlink and relink your Google account with sufficient permissions under your account settings.'; | ||
| const errorMsgGeneric = 'There was an error while fetching Google Drive data.'; | ||
| drive.files.list({ fields: 'files(iconLink, webViewLink, name)' }, (err, response) => { | ||
| if (err) { | ||
| console.error('Google Drive API Error:', err); | ||
| const msg = err.message === 'Insufficient Permission' ? errorMsgPermission : errorMsgGeneric; | ||
| req.flash('errors', { msg }); | ||
| return res.redirect('/api'); | ||
| } | ||
| res.render('api/google-drive', { | ||
| title: 'Google Drive API', | ||
| files: response.data.files, | ||
| }); | ||
| }); | ||
| }; | ||
|
|
||
| exports.getGoogleSheets = (req, res) => { | ||
| const token = req.user.tokens.find((token) => token.kind === 'google'); | ||
| const authObj = new googlesheets.auth.OAuth2({ | ||
| access_type: 'offline', | ||
| }); | ||
| authObj.setCredentials({ | ||
| access_token: token.accessToken, | ||
| }); | ||
|
|
||
| const sheets = googlesheets.sheets({ | ||
| version: 'v4', | ||
| auth: authObj, | ||
| }); | ||
|
|
||
| const url = 'https://docs.google.com/spreadsheets/d/1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgvE2upms/edit#gid=0'; | ||
| const re = /spreadsheets\/d\/([a-zA-Z0-9-_]+)/; | ||
| const id = url.match(re)[1]; | ||
|
|
||
| const errorMsgPermission = 'Missing Google sheets access permission. Please unlink and relink your Google account with sufficient permissions under your account settings.'; | ||
| const errorMsgGeneric = 'There was an error while fetching Google Sheets data.'; | ||
| sheets.spreadsheets.values.get({ spreadsheetId: id, range: 'Class Data!A1:F' }, (err, response) => { | ||
| if (err) { | ||
| console.error('Google Sheets API Error:', err); | ||
| const msg = err.message === 'Insufficient Permission' ? errorMsgPermission : errorMsgGeneric; | ||
| req.flash('errors', { msg }); | ||
| return res.redirect('/api'); | ||
| } | ||
| res.render('api/google-sheets', { | ||
| title: 'Google Sheets API', | ||
| values: response.data.values, | ||
| }); | ||
| }); | ||
| res.redirect('/api/upload'); | ||
| }; |
There was a problem hiding this comment.
The logic for handling multerError (e.g., file size limits) and the explicit req.session.save() call have been removed. This means users will no longer receive feedback if an upload fails, and the success flash message might not persist across the redirect due to race conditions with the asynchronous session store.
| try { | ||
| const { data } = await drive.files.list({ | ||
| pageSize: 10, | ||
| fields: 'nextPageToken, files(id, name)', |
There was a problem hiding this comment.
The fields parameter in the drive.files.list call has been restricted to id, name. The previous implementation requested iconLink and webViewLink. If the api/google-drive view relies on these fields for rendering, it will now display incomplete or broken data.
| fields: 'nextPageToken, files(id, name)', | |
| fields: 'nextPageToken, files(id, name, iconLink, webViewLink)', |
| pageFirstSectionText = await getPageExtract(pageTitle); | ||
| pageFirstImage = await getPageImage(pageTitle); | ||
| const { data } = await sheets.spreadsheets.values.get({ | ||
| spreadsheetId: '129m_76shYfX67D7v_Yh63708S_Y99_y76shYfX67D7v_Yh63708S_Y99', |
There was a problem hiding this comment.
The hardcoded spreadsheetId appears to be an invalid placeholder. It should be restored to the previous functional example ID (1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgvE2upms) to ensure the example works for users.
| spreadsheetId: '129m_76shYfX67D7v_Yh63708S_Y99_y76shYfX67D7v_Yh63708S_Y99', | |
| spreadsheetId: '1BxiMVs0XRA5nFMdKvBdBZjgmUUqptlbs74OgvE2upms', |
2 participants
Checklist
npm run lint,npm test, andnpm run test:e2e-nokeypass locally.--no-verifywas not used when using git commitDescription
Screenshots of UI changes (browser) and logs/test results (console, terminal, shell, cmd)