AD-FE-T1: Replace static homepage modules with API-driven content - #305
Open
SachinMhto wants to merge 1 commit into
Open
AD-FE-T1: Replace static homepage modules with API-driven content#305SachinMhto wants to merge 1 commit into
SachinMhto wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are a few correctness/contract issues around query param validation and error propagation that should be fixed to prevent unexpected responses and preserve structured ML-service errors end-to-end.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR replaces the last static homepage module (“Trending Insights”) with API-driven content by adding a new ML-service endpoint, proxying it through the Node backend, and updating the React Native section to fetch/render live results.
Changes:
- Added placeholder trending-categories data function and exposed it via a new Flask
GET /api/trending-categoriesroute. - Added a new Express proxy route
GET /api/ml/trending-categorieswired throughml.router.js→ml.controller.js. - Rewrote
TrendingInsightsSectionto fetch from the API and render loading/error/empty states instead of hardcoded cards.
File summaries
| File | Description |
|---|---|
| Frontend/components/home/TrendingInsightsSection.tsx | Fetches trending categories from the backend and renders dynamic cards with UI states. |
| Backend/src/routers/ml.router.js | Registers the new GET /trending-categories ML proxy route and Swagger docs. |
| Backend/src/controllers/ml.controller.js | Implements proxy handler to call Flask ML service for trending categories. |
| Backend/ml-service/ml_models/trending_categories.py | Adds placeholder trending categories model/data function. |
| Backend/ml-service/app.py | Adds Flask API route to serve trending categories via success_payload. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 5
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+58
to
+62
| const { limit } = req.query; | ||
|
|
||
| const params = {}; | ||
| if (limit) params.limit = limit; | ||
|
|
Comment on lines
+80
to
+97
| } catch (error) { | ||
| console.error("Error calling ML service:", error.message); | ||
|
|
||
| if (error.code === "ECONNREFUSED" || error.code === "ETIMEDOUT") { | ||
| return res.status(503).json({ | ||
| success: false, | ||
| message: "ML service is currently unavailable", | ||
| error: | ||
| "Service connection failed. Please ensure the Python ML service is running on port 5001.", | ||
| }); | ||
| } | ||
|
|
||
| return res.status(500).json({ | ||
| success: false, | ||
| message: "Failed to fetch trending categories", | ||
| error: error.message, | ||
| }); | ||
| } |
Comment on lines
+33
to
+35
| }, | ||
| ] | ||
| return trending[:limit] No newline at end of file |
Comment on lines
+147
to
+158
| <Pressable className="flex-row items-center gap-2"> | ||
| <Text | ||
| className={`text-sm ${theme.textColor} font-semibold`} | ||
| > | ||
| View {item.category} deals | ||
| </Text> | ||
| <FontAwesome6 | ||
| name="arrow-right" | ||
| size={14} | ||
| className={theme.textColor} | ||
| /> | ||
| </Pressable> |
Comment on lines
+189
to
+196
| @app.route('/api/trending-categories', methods=['GET']) | ||
| def get_trending_categories(): | ||
| try: | ||
| limit = int(request.args.get('limit', 3)) | ||
| trending = get_trending_categories_ml(limit=limit) | ||
| return success_payload(data=trending, count=len(trending)) | ||
| except Exception as e: | ||
| return error_payload('Failed to fetch trending categories', str(e)) |
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
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.
What was done:
Investigated all homepage sections in
app/(tabs)/index.tsxand foundWeeklySpecialsSectionwas already API-driven. IdentifiedTrendingInsightsSectionas the remaining static module (3 hardcoded category cards). No existing endpoint served category-level trending data, so built the full pipeline from scratch:Backend/ml-service/ml_models/trending_categories.py— new data function, following the same pattern asweekly_specials.pyGET /api/trending-categories— new Flask route inml-service/app.pyGET /api/ml/trending-categories— new Express proxy route (ml.router.js+ml.controller.js), matching the existingweekly-specialsproxy patternTrendingInsightsSection.tsxto fetch live data with proper loading/error/empty states, instead of hardcoded JSXTesting done:
curl http://localhost:3000/api/ml/trending-categories— confirmed Node → Python → response works end to endKnown limitation / follow-up:
The endpoint currently returns structured placeholder data, consistent with how
weekly-specialsis currently implemented in this codebase. Real category-level aggregation (avg price drop / avg savings grouped by category from actual discount data) is a natural follow-up ticket.Environment note for the team:
Local Python setup requires Python 3.10/3.11 specifically — newer versions (3.12+) break several pinned dependencies (pandas, google-auth) via missing prebuilt wheels.