MOSIP-45242-GitHub tracker add support for both inji and mosip project#42
Open
jayesh12234 wants to merge 5 commits into
Open
MOSIP-45242-GitHub tracker add support for both inji and mosip project#42jayesh12234 wants to merge 5 commits into
jayesh12234 wants to merge 5 commits into
Conversation
Signed-off-by: Jayesh Kharode <jayesh.kharode@technoforte.co.in>
Signed-off-by: Jayesh Kharode <jayesh.kharode@technoforte.co.in>
Signed-off-by: Jayesh Kharode <jayesh.kharode@technoforte.co.in>
Signed-off-by: Jayesh Kharode <jayesh.kharode@technoforte.co.in>
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
github-activity-tracker/backend/services/userDetailsService.js (1)
29-39: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winScope the initial user lookup to the requested org.
dailyQueryis org-filtered, but the profile lookup still resolvesloginglobally. A request like/orgs/inji/users/<mosip-only-user>can return that user with zero activity instead of “not found” for the org.🐛 Proposed fix
async function getUserDetails(login, period, orgId) { if (isExcludedGitHubLogin(login)) { throw new Error('User not found'); } + const orgOwner = String(orgId).toLowerCase(); + /* 1. Get user details */ const userQuery = ` - SELECT id, login, avatar_url, NULL as name, NULL as email - FROM github_users - WHERE login = $1 + SELECT gu.id, gu.login, gu.avatar_url, NULL as name, NULL as email + FROM github_users gu + WHERE gu.login = $1 + AND EXISTS ( + SELECT 1 + FROM activity_events e + JOIN repos r ON r.github_repo_id = e.repo_id + WHERE e.user_id = gu.id + AND LOWER(r.owner) = $2 + ) `; - const userRes = await pool.query(userQuery, [login]); + const userRes = await pool.query(userQuery, [login, orgOwner]);- const orgOwner = String(orgId).toLowerCase(); - const dailyRes = await pool.query(dailyQuery, [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@github-activity-tracker/backend/services/userDetailsService.js` around lines 29 - 39, The initial user lookup in getUserDetails currently searches github_users only by login, so it can return a user outside the requested org. Update the userQuery in getUserDetails to scope the lookup by orgId as well, using the same org membership data/source used elsewhere in this service, so org-only requests return “User not found” when the login does not belong to that org.github-activity-tracker/backend/routes/repoSyncRoute.js (1)
25-41: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDon't report zero after partial multi-org writes.
If
syncRepos(orgName)fails on the second or later org,github-activity-tracker/backend/services/syncRepos.js:13-99may already have written repos for earlier orgs (and even part of the failing org), but this catch block still returnsrepos_processed: 0. That breaks the route contract for callers and makes retries blind. Please return partial per-org results, or at least preserve the completed count instead of zeroing the whole response.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@github-activity-tracker/backend/routes/repoSyncRoute.js` around lines 25 - 41, The error handling in repoSyncRoute’s sync loop is resetting repos_processed to 0 even after syncRepos(orgName) has already completed some work for earlier orgs. Update the route’s try/catch so it preserves the accumulated reposProcessed count from the for-of loop, or otherwise returns partial per-org progress instead of zeroing the response in the catch block. Use the existing repoSyncRoute handler and syncRepos(orgName) callsite to keep the completed count consistent with partial multi-org writes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@github-activity-tracker/backend/config/syncConfig.js`:
- Around line 10-11: The org list in syncConfig.js is being de-duplicated before
case normalization, so values like “mosip” and “MOSIP” still survive as separate
entries. Update the org parsing logic to normalize each trimmed value to
lowercase before it is passed into the Set, and keep the existing filtering of
empty values so repoSyncRoute only syncs each org once.
In `@github-activity-tracker/backend/services/orgSummaryService.js`:
- Around line 59-60: The time-range filter in orgSummaryService.js currently
uses an inclusive BETWEEN in the query-building logic, which double-counts
events exactly at the boundary between periods. Update the predicate in the code
that appends to whereClauses after params.push(start, end) so it uses a
half-open window with e.created_at >= start and e.created_at < end, preserving
correct separation between the previous and current summary windows.
---
Outside diff comments:
In `@github-activity-tracker/backend/routes/repoSyncRoute.js`:
- Around line 25-41: The error handling in repoSyncRoute’s sync loop is
resetting repos_processed to 0 even after syncRepos(orgName) has already
completed some work for earlier orgs. Update the route’s try/catch so it
preserves the accumulated reposProcessed count from the for-of loop, or
otherwise returns partial per-org progress instead of zeroing the response in
the catch block. Use the existing repoSyncRoute handler and syncRepos(orgName)
callsite to keep the completed count consistent with partial multi-org writes.
In `@github-activity-tracker/backend/services/userDetailsService.js`:
- Around line 29-39: The initial user lookup in getUserDetails currently
searches github_users only by login, so it can return a user outside the
requested org. Update the userQuery in getUserDetails to scope the lookup by
orgId as well, using the same org membership data/source used elsewhere in this
service, so org-only requests return “User not found” when the login does not
belong to that org.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72dd3c0e-545f-4526-8edf-f8712cd6716c
📒 Files selected for processing (13)
github-activity-tracker/backend/.env.examplegithub-activity-tracker/backend/app.jsgithub-activity-tracker/backend/config/syncConfig.jsgithub-activity-tracker/backend/routes/orgActivityRoute.jsgithub-activity-tracker/backend/routes/orgSummaryRoute.jsgithub-activity-tracker/backend/routes/repoSyncRoute.jsgithub-activity-tracker/backend/routes/userDetailsRoute.jsgithub-activity-tracker/backend/services/leaderBoardService.jsgithub-activity-tracker/backend/services/orgActivityService.jsgithub-activity-tracker/backend/services/orgSummaryService.jsgithub-activity-tracker/backend/services/orgUsersService.jsgithub-activity-tracker/backend/services/userDetailsService.jsgithub-activity-tracker/backend/utils/orgQuery.js
Signed-off-by: Jayesh Kharode <jayesh.kharode@technoforte.co.in>
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
2 participants
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.
Summary
GITHUB_ORGSenv var to sync multiple GitHub organizations in one callrepos.ownerso/orgs/mosip/*and/orgs/inji/*return org-specific dataChanges
repoSyncRouteloops overGITHUB_ORGSwhen no org is passed in request bodyrepos.ownerutils/orgQuery.js— shared SQL join/filter helpersConfig
GITHUB_ORGS=mosip,inji
Test plan
Summary by CodeRabbit
New Features
Bug Fixes