Conversation
✅ Deploy Preview for invoicen ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the document extraction service to use a new HTTPS production endpoint instead of the previous hard-coded HTTP IP-based URL. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Consider moving the API base URL into a configuration or environment variable instead of hardcoding it in the service so it can be more easily changed per environment (dev/stage/prod) without code changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider moving the API base URL into a configuration or environment variable instead of hardcoding it in the service so it can be more easily changed per environment (dev/stage/prod) without code changes.
## Individual Comments
### Comment 1
<location path="src/components/document/documentService.ts" line_range="3" />
<code_context>
import { ApiResponse } from "./types";
-const API_BASE_URL = "http://34.122.133.222:8000";
+const API_BASE_URL = "https://infernxt.com/py-extraction-endpoint/";
export async function extractDocumentData(file: File): Promise<ApiResponse> {
</code_context>
<issue_to_address>
**suggestion:** Consider moving the API base URL into configuration/environment instead of hardcoding it.
This makes it harder to switch between dev/staging/prod and forces code changes for any endpoint update. Prefer reading the base URL from environment-specific config (env variable or build-time config) so this module stays deployment-agnostic.
Suggested implementation:
```typescript
const API_BASE_URL =
process.env.REACT_APP_API_BASE_URL || "https://infernxt.com/py-extraction-endpoint/";
```
To fully implement this:
1. Define `REACT_APP_API_BASE_URL` in your environment files (e.g., `.env.development`, `.env.production`) with appropriate values for dev/staging/prod.
2. If this project does not use Create React App and instead uses another bundler (e.g., Vite), adjust the environment variable access pattern accordingly (for Vite, use `import.meta.env.VITE_API_BASE_URL` instead of `process.env.REACT_APP_API_BASE_URL`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| import { ApiResponse } from "./types"; | ||
|
|
||
| const API_BASE_URL = "http://34.122.133.222:8000"; | ||
| const API_BASE_URL = "https://infernxt.com/py-extraction-endpoint/"; |
There was a problem hiding this comment.
suggestion: Consider moving the API base URL into configuration/environment instead of hardcoding it.
This makes it harder to switch between dev/staging/prod and forces code changes for any endpoint update. Prefer reading the base URL from environment-specific config (env variable or build-time config) so this module stays deployment-agnostic.
Suggested implementation:
const API_BASE_URL =
process.env.REACT_APP_API_BASE_URL || "https://infernxt.com/py-extraction-endpoint/";To fully implement this:
- Define
REACT_APP_API_BASE_URLin your environment files (e.g.,.env.development,.env.production) with appropriate values for dev/staging/prod. - If this project does not use Create React App and instead uses another bundler (e.g., Vite), adjust the environment variable access pattern accordingly (for Vite, use
import.meta.env.VITE_API_BASE_URLinstead ofprocess.env.REACT_APP_API_BASE_URL).
Summary by Sourcery
Enhancements: