-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Tumblr integrations #713
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
WalkthroughThis change introduces a dynamic blog feature by adding a new API route that fetches posts from the Tumblr API and exposes them at Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/backend.yaml(2 hunks)api-server/routes/api.js(1 hunks)api-server/routes/logues.js(1 hunks)web-frontend/src/pages/Blog.vue(1 hunks)
🔇 Additional comments (2)
api-server/routes/api.js (1)
8-8: LGTM! Router integration follows existing patterns.The new posts router is properly imported and mounted following the established pattern in this file.
Also applies to: 16-16
.github/workflows/backend.yaml (1)
14-14: LGTM! Environment variable updates are consistent.The replacement of
RAPID_API_KEYwithTUMBLR_CONSUMER_KEYis properly implemented in both the environment declaration and the Cloud Run deployment step.Also applies to: 70-70
| const CONSUMER_KEY = process.env.TUMBLR_CONSUMER_KEY; | ||
| const url = 'https://api.tumblr.com/v2/blog/hwakabh.tumblr.com/posts?api_key=' + CONSUMER_KEY; | ||
| const respPosts = await axios.get(url) | ||
| .then(response => { | ||
| return response.data.response.posts | ||
| }) | ||
| .catch(error => { | ||
| console.log(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.
Improve error handling and API request structure.
Several issues with the current implementation:
- Missing error handling: If the Tumblr API fails,
respPostswill beundefined, causing a crash on line 21. - API key security: Concatenating the API key in the URL string is less secure than using query parameters.
- No error response: Errors are only logged but not returned to the client.
Apply this diff to improve error handling and security:
const CONSUMER_KEY = process.env.TUMBLR_CONSUMER_KEY;
- const url = 'https://api.tumblr.com/v2/blog/hwakabh.tumblr.com/posts?api_key=' + CONSUMER_KEY;
- const respPosts = await axios.get(url)
- .then(response => {
- return response.data.response.posts
- })
- .catch(error => {
- console.log(error);
- })
+
+ if (!CONSUMER_KEY) {
+ return res.status(500).json({ error: 'Tumblr API key not configured' });
+ }
+
+ try {
+ const response = await axios.get('https://api.tumblr.com/v2/blog/hwakabh.tumblr.com/posts', {
+ params: { api_key: CONSUMER_KEY }
+ });
+
+ const respPosts = response.data.response.posts || [];Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In api-server/routes/logues.js around lines 10 to 18, improve error handling by
using try-catch instead of mixing await with then/catch, so you can handle
errors properly and avoid respPosts being undefined. Use axios with a params
object to pass the API key securely instead of concatenating it in the URL
string. In the catch block, log the error and send an appropriate error response
to the client to avoid silent failures.
| const respPosts = await axios.get(url) | ||
| .then(response => { | ||
| return response.data.response.posts | ||
| }) | ||
| .catch(error => { | ||
| console.log(error); | ||
| }) | ||
|
|
||
| const posts = []; | ||
| respPosts.forEach(p => { | ||
| posts.push({ | ||
| date: p.date, | ||
| body: p.body | ||
| }) | ||
| }); | ||
|
|
||
| console.log(posts); | ||
|
|
||
| res.header('Content-Type', 'application/json; charset=utf-8'); | ||
| res.json({ | ||
| "path": req.originalUrl, | ||
| "content": posts | ||
| }); | ||
|
|
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.
🛠️ Refactor suggestion
Add proper error handling for the entire route.
The route needs comprehensive error handling to prevent crashes and provide meaningful responses to clients.
Wrap the entire route logic in a try-catch block:
router.get('/', async (req, res, next) => {
// #swagger.tags = ['Logues']
// #swagger.summary = 'returns dynamic content for the blog page'
// #swagger.description = '/api/v1/logues'
+ try {
const CONSUMER_KEY = process.env.TUMBLR_CONSUMER_KEY;
if (!CONSUMER_KEY) {
return res.status(500).json({ error: 'Tumblr API key not configured' });
}
const response = await axios.get('https://api.tumblr.com/v2/blog/hwakabh.tumblr.com/posts', {
params: { api_key: CONSUMER_KEY }
});
const respPosts = response.data.response.posts || [];
const posts = [];
if (respPosts && Array.isArray(respPosts)) {
respPosts.forEach(p => {
posts.push({
date: p.date,
body: p.body
})
});
}
res.header('Content-Type', 'application/json; charset=utf-8');
res.json({
"path": req.originalUrl,
"content": posts
});
+ } catch (error) {
+ console.error('Error fetching Tumblr posts:', error);
+ res.status(500).json({
+ error: 'Failed to fetch blog posts',
+ path: req.originalUrl
+ });
+ }
});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In api-server/routes/logues.js between lines 12 and 35, the route lacks
comprehensive error handling which can cause crashes and unclear client
responses. Wrap the entire route logic inside a try-catch block to catch any
errors during execution. In the catch block, log the error and send a meaningful
error response to the client with an appropriate HTTP status code, such as 500,
to indicate a server error.
| const posts = []; | ||
| respPosts.forEach(p => { | ||
| posts.push({ | ||
| date: p.date, | ||
| body: p.body | ||
| }) | ||
| }); | ||
|
|
||
| console.log(posts); |
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.
🛠️ Refactor suggestion
Remove debug logging and add validation.
The current implementation has production code issues:
- Debug logging:
console.log(posts)should be removed from production code. - Missing validation: No check if
respPostsexists before processing.
Apply this diff:
const posts = [];
- respPosts.forEach(p => {
- posts.push({
- date: p.date,
- body: p.body
- })
- });
-
- console.log(posts);
+ if (respPosts && Array.isArray(respPosts)) {
+ respPosts.forEach(p => {
+ posts.push({
+ date: p.date,
+ body: p.body
+ })
+ });
+ }📝 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.
| const posts = []; | |
| respPosts.forEach(p => { | |
| posts.push({ | |
| date: p.date, | |
| body: p.body | |
| }) | |
| }); | |
| console.log(posts); | |
| const posts = []; | |
| if (respPosts && Array.isArray(respPosts)) { | |
| respPosts.forEach(p => { | |
| posts.push({ | |
| date: p.date, | |
| body: p.body | |
| }) | |
| }); | |
| } |
🤖 Prompt for AI Agents
In api-server/routes/logues.js between lines 20 and 28, remove the debug
statement console.log(posts) to clean up production code and add a validation
check to ensure respPosts exists and is an array before iterating over it. This
prevents runtime errors if respPosts is undefined or null. Use a conditional
statement to verify respPosts is valid before the forEach loop.
| axios.get('/api/v1/posts') | ||
| .then((resp) => { | ||
| postData.value = resp.data.content; | ||
| }) | ||
| .catch((err) => { | ||
| console.log(err); | ||
| }) |
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.
🛠️ Refactor suggestion
Improve error handling and add loading state.
The current implementation lacks user feedback for loading and error states.
Apply this diff to improve user experience:
+const postData = ref([]);
+const loading = ref(true);
+const error = ref(null);
-const postData = ref([]);
-axios.get('/api/v1/posts')
- .then((resp) => {
- postData.value = resp.data.content;
- })
- .catch((err) => {
- console.log(err);
- })
+
+const fetchPosts = async () => {
+ try {
+ loading.value = true;
+ const resp = await axios.get('/api/v1/posts');
+ postData.value = resp.data.content || [];
+ } catch (err) {
+ console.error('Failed to fetch posts:', err);
+ error.value = 'Failed to load blog posts. Please try again later.';
+ } finally {
+ loading.value = false;
+ }
+};
+
+fetchPosts();Then update the template to handle these states:
<template>
<section class="wrapper fullscreen">
<div class="inner">
<h1 class="major">Blogs</h1>
+ <div v-if="loading">Loading posts...</div>
+ <div v-else-if="error" class="error">{{ error }}</div>
+ <div v-else-if="postData.length === 0">No posts available.</div>
<div v-for="(p, idx) in postData"
:key="idx"
class="post-detail"
>🤖 Prompt for AI Agents
In web-frontend/src/pages/Blog.vue around lines 6 to 12, the current axios call
lacks loading and error state handling, which reduces user feedback. Introduce
reactive variables to track loading and error states, set loading to true before
the request, update postData on success, set error state on failure, and reset
loading after completion. Then update the template to display appropriate
messages or indicators based on these states.
| <div v-html="p.date"></div> | ||
| <div v-html="p.body"></div> |
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.
CRITICAL: XSS vulnerability with v-html on external content.
Using v-html to render content from external APIs (Tumblr) creates a serious XSS vulnerability. Malicious scripts in blog posts could execute in users' browsers.
Never use v-html with untrusted external content. Consider these safer alternatives:
Option 1: Text-only rendering (safest)
- <div v-html="p.date"></div>
- <div v-html="p.body"></div>
+ <div>{{ p.date }}</div>
+ <div>{{ p.body }}</div>Option 2: Server-side sanitization (recommended)
Install and use DOMPurify on the backend to sanitize HTML before sending to frontend:
// In logues.js
const DOMPurify = require('isomorphic-dompurify');
posts.push({
date: p.date,
body: DOMPurify.sanitize(p.body)
});Option 3: Client-side sanitization (fallback)
+import DOMPurify from 'dompurify';
- <div v-html="p.body"></div>
+ <div v-html="DOMPurify.sanitize(p.body)"></div>🤖 Prompt for AI Agents
In web-frontend/src/pages/Blog.vue around lines 24 to 25, using v-html to render
external content from Tumblr creates a critical XSS vulnerability. To fix this,
avoid using v-html directly with untrusted content. Instead, sanitize the HTML
on the server side before sending it to the frontend by integrating a library
like DOMPurify to clean the HTML in the backend code that fetches the posts.
Alternatively, if server-side sanitization is not possible, sanitize the content
on the client side before rendering. If neither sanitization is feasible, render
the content as plain text without HTML interpretation.
|
Evidences in local environment: 25-08-03 23:17:08 git/hwakabh.github.io [feat/blog-integrations] % export TUMBLR_CONSUMER_KEY='xxx'
25-08-03 23:17:26 git/hwakabh.github.io [feat/blog-integrations] % make clean && make all
# ...
>>> [api-server] Starting up API server process
>>> [web-frontend] Starting up API server process
25-08-03 23:17:37 git/hwakabh.github.io [feat/blog-integrations] %
> api-server@0.0.0 dev
> nodemon ./bin/www
> web-frontend@0.0.0 dev
> vite --clearScreen false
[nodemon] 3.1.10
[nodemon] to restart at any time, enter `rs`
[nodemon] watching path(s): *.*
[nodemon] watching extensions: js,mjs,cjs,json
[nodemon] starting `node ./bin/www`
VITE v7.0.6 ready in 114 ms
➜ Local: http://localhost:5173/
➜ Network: use --host to expose
[
{
date: '2025-08-03 13:22:35 GMT',
body: '<h2>Each title seems to be good with h2</h2><p>Another post here, without any title block.</p><p>another paragraph here.</p>'
},
{
date: '2025-08-03 13:17:08 GMT',
body: '<h2>Another title are here</h2><p>Another contents here.</p><p>With another posts.</p>'
}
] |
Issue/PR link
N/A
What does this PR do?
Describe what changes you make in your branch:
v-html(Optional) Additional Contexts
Describe additional information for reviewers (i.e. What does not included)
N/A
Summary by CodeRabbit
New Features
Documentation
Chores