-
Notifications
You must be signed in to change notification settings - Fork 0
[feature] ApiClient + Posts Query #6
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
|
Caution Review failedThe pull request is closed. WalkthroughThis update introduces a new API client with caching and error handling, a Pinia store for API integration, and TypeScript interfaces for API responses. It refactors several Vue components to dynamically fetch and render post and profile data, implements dark mode via a composable, and adds utility functions for reading time and error handling. Configuration files are updated for new dependencies and path aliases. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppVue
participant ApiStore
participant ApiClient
participant API
User->>AppVue: Loads application
AppVue->>ApiStore: boot()
Note right of ApiStore: Logs message if in development
User->>HomePage: Navigates to home
HomePage->>ApiStore: getProfile()
ApiStore->>ApiClient: get('/profile')
ApiClient->>API: GET /profile (with headers, ETag)
API-->>ApiClient: 200 OK (profile data) or 304 Not Modified
ApiClient-->>ApiStore: Profile data (from API or cache)
ApiStore-->>HomePage: Profile data
User->>ArticlesListPartial: Loads articles list
ArticlesListPartial->>ApiStore: getPosts()
ApiStore->>ApiClient: get('/posts')
ApiClient->>API: GET /posts (with headers, ETag)
API-->>ApiClient: 200 OK (posts data) or 304 Not Modified
ApiClient-->>ApiStore: Posts data (from API or cache)
ApiStore-->>ArticlesListPartial: Posts data
User->>PostPage: Navigates to post detail
PostPage->>ApiStore: getPost(slug)
ApiStore->>ApiClient: get(`/posts/${slug}`)
ApiClient->>API: GET /posts/:slug (with headers, ETag)
API-->>ApiClient: 200 OK (post data) or 304 Not Modified
ApiClient-->>ApiStore: Post data (from API or cache)
ApiStore-->>PostPage: Post data
PostPage->>PostPage: Parse markdown, sanitize, highlight code, render
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✨ 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.
Summary of Changes
Hello @gocanto, 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 establishes the groundwork for robust API communication within the application. It introduces a dedicated HTTP client, defines necessary data structures for API interactions, and integrates an initial API call into the home page. Crucially, it also configures the development environment with a proxy to ensure seamless communication with the backend during local development.
Highlights
- New HTTP Client Implementation: A foundational
ApiClientclass has been introduced to centralize and standardize HTTP GET requests to the backend API. This client includes logic for setting common headers likeContent-Type,User-Agent, and custom API authentication headers (Key, Username, Signature). - API Data Structures and Example Usage: New TypeScript interfaces (
Account,ApiClientOptions,ApiResponse,UserProfile) have been defined to provide strong typing for API request parameters and response payloads. An examplefetchUserProfilefunction demonstrates how to use the newApiClientto fetch user data, including error handling and console logging. - Initial API Integration in Home Page: The
HomePage.vuecomponent now imports and calls thefetchUserProfilefunction ononMounted, showcasing the integration of the new API client. It also includes console logs to display the fetched user profile response and theVITE_API_URL. - Development Server Proxy Configuration: The Vite configuration (
vite.config.ts) has been updated to include a proxy rule. This rule redirects requests made to/apifrom the frontend tohttp://localhost:8080(presumably the backend server), which is essential for local development to bypass CORS issues.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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 or fill out our survey to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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
This pull request introduces a new HTTP client and integrates it into the home page. I've identified a critical bug in HomePage.vue where an async call is not being handled correctly, which would cause it to fail. I've also suggested several improvements to the new ApiClient for correctness and maintainability, including fixing a function that doesn't return a value, correcting a typo, and improving its flexibility. Finally, I've recommended a small enhancement to the Vite configuration to avoid hardcoding URLs.
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: 6
♻️ Duplicate comments (3)
src/pages/HomePage.vue (1)
58-70: Remove debug console.log statements before production.The async/await implementation is correct and addresses the previous review comment. However, the console.log statements on lines 59 and 65 should be removed before merging to keep the console clean in production.
- console.log('Attempting to fetch user profile...'); try { const userProfileResponse = await apiStore.getProfile(); if (userProfileResponse.data) { - console.log(`Welcome, ${userProfileResponse.data.name}!`); }src/stores/api/client.ts (2)
32-32: Fix typo: basedURL should be baseURL.This typo was previously identified and should be corrected throughout the file.
-private readonly basedURL: string; +private readonly baseURL: string;Also update usage on lines 41 and 85.
74-74: Make userAgent configurable via ApiClientOptions.The userAgent is hardcoded. As suggested in a previous review, it should be configurable.
First, add to the
ApiClientOptionsinterface:export interface ApiClientOptions { env: string; apiKey: string; apiUsername: string; apiSignature: string; + userAgent?: string; }Then update the client:
+private readonly userAgent: string; constructor(options: ApiClientOptions) { // ... other assignments + this.userAgent = options.userAgent ?? 'oullin/web-app'; }And in createHeaders:
-headers.append('User-Agent', 'oullin/web-app'); +headers.append('User-Agent', this.userAgent);
🧹 Nitpick comments (5)
src/public.ts (1)
19-44: LGTM! Well-implemented reading time calculation.The
getReadingTimefunction is well-structured with proper edge case handling, reasonable defaults, and clear formatting logic. The 225 WPM default is a good standard, and the minimum 1-minute guarantee prevents awkward "0 min read" displays.Consider adding a JSDoc comment to document the function's behavior and parameters:
+/** + * Calculates estimated reading time for given text + * @param text - The text content to analyze + * @param wpm - Words per minute reading speed (default: 225) + * @returns Formatted reading time string (e.g., "5 min read", "1 hour 30 min read") + */ export function getReadingTime(text: string, wpm: number = 225): string {src/stores/api/response/profile-response.ts (1)
1-7: LGTM! Well-structured ProfileResponse interface.The interface provides a clear contract for profile data with appropriately named string properties. Consider adding runtime validation for the email field when this response is processed.
src/stores/api/response/posts-response.ts (1)
11-13: Consider adding JSDoc comments for date fields.The date fields are correctly typed as strings for API responses, but it would be helpful to document the expected format.
+ /** ISO 8601 date string */ published_at: string; + /** ISO 8601 date string */ created_at: string; + /** ISO 8601 date string */ updated_at: string;src/stores/api/store.ts (1)
20-24: Consider using a proper logger instead of console.log.Direct console.log statements should be avoided in production code. Consider using a logging library or environment-based logging.
boot(): void { if (this.client.isDev()) { - console.log('API client booted ...'); + // Use a proper logger or remove this + if (import.meta.env.DEV) { + console.log('API client booted ...'); + } } },src/pages/PostPage.vue (1)
156-158: Consider using Vue Router for URL construction.Instead of manually constructing URLs with window.location, consider using Vue Router's utilities for more maintainable code.
+import { useRouter } from 'vue-router'; + +const router = useRouter(); const fullURLFor = (post: PostsResponse) => { - return `${window.location.origin}/posts/${post.slug}`; + const route = router.resolve({ name: 'PostDetail', params: { slug: post.slug } }); + return `${window.location.origin}${route.href}`; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
.env.example(1 hunks)eslint.config.js(1 hunks)index.html(1 hunks)package.json(1 hunks)src/App.vue(1 hunks)src/dark-mode.ts(1 hunks)src/pages/HomePage.vue(1 hunks)src/pages/PostPage.vue(6 hunks)src/partials/ArticleItemPartial.vue(3 hunks)src/partials/ArticlesListPartial.vue(1 hunks)src/partials/HeaderPartial.vue(2 hunks)src/partials/WidgetSkillsPartial.vue(0 hunks)src/public.ts(1 hunks)src/router.ts(1 hunks)src/stores/api/client.ts(1 hunks)src/stores/api/http-error.ts(1 hunks)src/stores/api/response/posts-response.ts(1 hunks)src/stores/api/response/profile-response.ts(1 hunks)src/stores/api/store.ts(1 hunks)tsconfig.json(1 hunks)vite.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/partials/WidgetSkillsPartial.vue
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/stores/api/store.ts (4)
src/stores/api/client.ts (3)
ApiClient(29-117)defaultCreds(5-10)ApiResponse(24-27)src/stores/api/response/profile-response.ts (1)
ProfileResponse(1-7)src/stores/api/http-error.ts (1)
parseError(17-27)src/stores/api/response/posts-response.ts (1)
PostsResponse(1-14)
src/stores/api/client.ts (1)
src/stores/api/http-error.ts (1)
HttpError(1-15)
🔇 Additional comments (26)
package.json (1)
15-15: highlight.js version ^11.11.1 is up-to-date and secureA check against the npm registry confirms that 11.11.1 is the latest release, and all known moderate-severity advisories (ReDOS and Prototype Pollution) affect only versions below 10.4.1 or 10.1.2 and were patched in those releases. No vulnerabilities apply to 11.11.1, so you can safely keep this dependency as is.
.env.example (1)
1-17: LGTM! Well-structured environment configuration.The environment variable configuration is well-organized with clear comments explaining each variable's purpose. The VITE_ prefixes are correctly used for client-side variables, and the structure supports the new API client implementation.
src/dark-mode.ts (1)
12-14: LGTM! Clean toggle implementation.The toggle function is simple and effective, following Vue composition API patterns well.
vite.config.ts (1)
30-30: LGTM! Consistent alias addition for API modules.The new '@api' alias follows the established pattern and properly supports the new API client modules. The path resolution is correctly implemented using the existing __dirname approach.
tsconfig.json (1)
27-28: LGTM! Path aliases correctly configured.The new "@api/*" alias provides cleaner imports for API-related modules. The configuration is technically sound and supports the new API client structure.
eslint.config.js (1)
75-75: Sanitization Verified for PostPage.vue
ThehtmlContentcomputed property appliesDOMPurify.sanitize(marked.parse(...))before rendering viav-html, ensuring XSS protection. No further changes required—approving the exemption.src/router.ts (1)
34-36: LGTM! Route configuration properly updated for dynamic posts.The addition of the
:slugparameter and named route 'PostDetail' correctly enables dynamic post loading. This aligns well with the new API client functionality.index.html (1)
10-12: LGTM! Simplified dark mode initialization logic.The streamlined approach only adds the 'dark' class when explicitly needed, which is cleaner and aligns well with reactive dark mode management in Vue components.
src/App.vue (1)
5-14: LGTM! Good architectural decision for API store initialization.The root-level API store initialization follows Vue 3 composition API best practices and ensures the API client is properly bootstrapped when the application starts.
src/pages/HomePage.vue (2)
47-47: Import reordering looks good.The import organization is improved for better readability.
53-54: Good integration with the new API architecture.The imports for the API store and error handling utilities are properly integrated.
src/partials/HeaderPartial.vue (2)
27-27: Good refactor to use composable for dark mode management.The change from
v-modelto@click="toggleDarkMode"is appropriate since theuseDarkModecomposable manages the state internally. This improves code organization and reusability.
55-57: Excellent use of composable pattern.The refactor to use the
useDarkModecomposable centralizes dark mode state management, making it more maintainable and reusable across components.src/partials/ArticleItemPartial.vue (5)
2-2: Good defensive programming with conditional rendering.The
v-if="item"prevents rendering errors when the item prop is null or undefined.
4-4: Property name updated to match API response structure.The change from
item.imagetoitem.cover_image_urlaligns with the new API response format.
6-8: Improved date formatting with shared utility.The date formatting now uses a centralized utility function and properly handles the
published_attimestamp, improving consistency across the application.
12-12: Better routing with named routes and parameters.The change to use named routes with parameters (
{ name: 'PostDetail', params: { slug: item.slug } }) is more maintainable than string-based routing.Also applies to: 21-25
37-42: Proper TypeScript integration with API types.The imports and prop definition now use the proper
PostsResponsetype from the API layer, improving type safety and maintainability.src/stores/api/http-error.ts (3)
1-15: Well-implemented custom error class.The
HttpErrorclass properly extends the nativeErrorclass with HTTP-specific properties. The prototype compatibility fix on line 13 is a good practice for older JavaScript environments.
29-36: Clean and useful debugging utility.The
debugErrorfunction provides appropriate logging for different error types, helping with development and troubleshooting.
17-27: Simplify the parseError function implementation.The function has unused parameters and redundant error message construction.
export function parseError(error: any): Promise<never> { let errorMessage = `An unexpected error occurred: ${error}`; if (error instanceof HttpError) { - errorMessage = `An error occurred in the API. status [${error.message}] / message [${error.body}]`; + errorMessage = `API Error: ${error.message}. Response: ${error.body}`; } - return new Promise((resolve, reject) => { - reject(new Error(errorMessage)); - }); + return Promise.reject(new Error(errorMessage)); }Likely an incorrect or invalid review comment.
src/stores/api/response/posts-response.ts (1)
1-14: LGTM! Well-structured TypeScript interfaces.The interfaces properly define the API response structure with clear naming conventions.
src/stores/api/client.ts (1)
83-116: Well-implemented caching mechanism with ETag support.The GET method properly implements HTTP caching with ETag headers and local storage. Error handling is appropriate with the custom HttpError class.
src/pages/PostPage.vue (3)
112-150: Excellent security implementation with DOMPurify.The markdown parsing with sanitization is properly implemented, preventing XSS attacks while maintaining functionality.
173-179: Smart implementation of dynamic syntax highlighting themes.The dynamic import of highlight.js themes based on dark mode is an excellent UX enhancement.
194-200: Type casting may be unnecessary.If the store's
getPostmethod is updated to returnApiResponse<PostsResponse>as suggested earlier, this will need adjustment.onMounted(async () => { try { - post.value = (await apiStore.getPost(slug.value)) as PostsResponse; + // If getPost returns ApiResponse<PostsResponse>: + // post.value = (await apiStore.getPost(slug.value)).data; + // Otherwise, the current code is correct + post.value = await apiStore.getPost(slug.value); } catch (error) { debugError(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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pages/PostPage.vue (1)
194-200: Good async data fetching with error handling.The onMounted hook properly fetches post data with error handling. The type assertion
as PostResponsemight be unnecessary if theapiStore.getPostmethod already returns the correct type, but it doesn't cause any issues.Consider removing the type assertion if the API store method already returns the correctly typed response:
- post.value = (await apiStore.getPost(slug.value)) as PostResponse; + post.value = await apiStore.getPost(slug.value);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
index.html(1 hunks)src/pages/PostPage.vue(6 hunks)src/partials/ArticleItemPartial.vue(3 hunks)src/partials/ArticlesListPartial.vue(1 hunks)src/stores/api/response/post-response.ts(1 hunks)src/stores/api/store.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/stores/api/response/post-response.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- index.html
- src/stores/api/store.ts
- src/partials/ArticlesListPartial.vue
- src/partials/ArticleItemPartial.vue
🔇 Additional comments (11)
src/pages/PostPage.vue (11)
2-2: Good defensive programming with conditional rendering.The
v-if="post"directive ensures the component only renders when post data is available, preventing potential undefined reference errors during async loading.
35-36: Dynamic metadata rendering looks good.The date formatting using
date().format(new Date(post.published_at))and reading time calculation withgetReadingTime(post.content)properly utilize the fetched post data.
43-73: Well-implemented social sharing functionality.The social sharing buttons properly generate dynamic URLs and include appropriate security attributes for external links (
target="_blank"andrel="noopener noreferrer"). The generic share button using the Web Share API is a nice modern touch.
84-90: Dynamic content rendering is well-implemented.The post title, excerpt, and cover image are properly bound to dynamic data. The use of
v-html="htmlContent"is appropriate here since the content is sanitized through DOMPurify (as seen in the computed property), and thepostContainerref enables syntax highlighting.
112-129: Comprehensive and well-organized imports.The script setup with TypeScript and the extensive imports cover all necessary functionality: markdown parsing, sanitization, routing, API integration, error handling, utilities, and syntax highlighting. Good organization and type safety approach.
132-137: Proper reactive state setup with TypeScript.The reactive references are well-typed and properly initialized. Route parameter extraction and API store usage follow Vue 3 composition API best practices.
139-142: Good markdown configuration.The marked library configuration with
breaks: trueandgfm: trueprovides appropriate GitHub Flavored Markdown support for blog content.
144-150: Excellent security implementation for HTML content.The computed property properly sanitizes markdown-parsed content using DOMPurify, which is essential for preventing XSS attacks when using
v-html. This is the correct and secure approach for rendering markdown content.
152-158: Well-implemented URL generation functions.The
xURLForandfullURLForfunctions properly construct social sharing URLs with appropriate parameters and use the canonical post URL structure.
160-171: Proper Web Share API implementation with error handling.The
sharePostfunction correctly uses the navigator.share API with comprehensive shareData and includes appropriate try-catch error handling to prevent crashes when sharing fails.
173-192: Comprehensive syntax highlighting with dynamic theming.The implementation properly handles dynamic theme switching with CSS imports and re-highlights code blocks when content changes. The use of
nextTick()ensures DOM updates are complete before highlighting, and the container-scoped queries prevent conflicts.
Summary by CodeRabbit
New Features
Improvements
Chores
Bug Fixes
Documentation