Skip to content

fix: code quality and safety improvements - #753

Open
saurabhhhcodes wants to merge 1 commit into
GitMetricsLab:mainfrom
saurabhhhcodes:fix/github_tracker-83967
Open

fix: code quality and safety improvements#753
saurabhhhcodes wants to merge 1 commit into
GitMetricsLab:mainfrom
saurabhhhcodes:fix/github_tracker-83967

Conversation

@saurabhhhcodes

@saurabhhhcodes saurabhhhcodes commented Aug 2, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

  • Bug Fixes
    • Improved activity feed handling when GitHub requests fail, preventing unsuccessful responses from being processed as valid data.
    • Ensured loading completes correctly even when activity data cannot be retrieved.
    • Maintained equivalent validation for permitted connection origins.

@netlify

netlify Bot commented Aug 2, 2026

Copy link
Copy Markdown

Deploy Preview for github-spy ready!

Name Link
🔨 Latest commit b2f2ddc
🔍 Latest deploy log https://app.netlify.com/projects/github-spy/deploys/6a6f46b2fd735e00084488b1
😎 Deploy Preview https://deploy-preview-753--github-spy.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The server now uses Array.includes for CORS origin checks. ActivityFeed now rejects unsuccessful GitHub responses before parsing JSON and still stops loading on failure.

Changes

CORS validation

Layer / File(s) Summary
CORS origin membership check
backend/server.js
CORS validation now uses allowedOrigins.includes(origin) instead of an indexOf comparison.

Activity feed response handling

Layer / File(s) Summary
GitHub response validation
src/components/ActivityFeed.tsx
The event fetch flow throws for non-OK responses before JSON parsing. Failures are logged and stop loading.

Estimated code review effort: 2 (Simple) | ~5 minutes

Possibly related PRs

Suggested reviewers: mehul-m-prajapati

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request has no description and omits all required template sections, including the related issue, testing details, and change type. Add the required sections and describe the changes, testing performed, related issue, screenshots if applicable, and type of change.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the code quality and safety changes in the pull request.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@src/components/ActivityFeed.tsx`:
- Around line 36-37: Update the ActivityFeed request flow to track a distinct
error state when the response is not OK or parsing fails, and clear it on
successful fetches. Render the error state instead of “No activity found” or
stale events after failures, and add coverage for both initial-load and
interval-fetch failures.
🪄 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3deb0998-b9e4-4372-8e84-2c70c1662b84

📥 Commits

Reviewing files that changed from the base of the PR and between 53f820b and b2f2ddc.

📒 Files selected for processing (2)
  • backend/server.js
  • src/components/ActivityFeed.tsx

Comment on lines +36 to +37
if (!res.ok) throw new Error("Request failed");
const data = await res.json();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Show an error state for failed responses.

When res.ok is false, the catch block only stops loading. The UI then shows No activity found on the initial request, or silently displays stale events during a refresh. Add an error state and render it instead of treating a failed request as an empty successful response. Add tests for both initial and interval-fetch failures.

🤖 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 `@src/components/ActivityFeed.tsx` around lines 36 - 37, Update the
ActivityFeed request flow to track a distinct error state when the response is
not OK or parsing fails, and clear it on successful fetches. Render the error
state instead of “No activity found” or stale events after failures, and add
coverage for both initial-load and interval-fetch failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant