-
-
Notifications
You must be signed in to change notification settings - Fork 900
fix: remove console logs exposing user data and critical website flows #3845
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
WalkthroughThis pull request removes unnecessary console logging from two parts of the codebase. In the Changes
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
✅ Deploy Preview for asyncapi-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@sagarkori143 checks are failing |
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/NewsletterSubscribe.tsx
(1 hunks)
🧰 Additional context used
🪛 ESLint
components/NewsletterSubscribe.tsx
[error] 88-88: Expected blank line before this statement.
(padding-line-between-statements)
🪛 GitHub Actions: PR testing - if Node project
components/NewsletterSubscribe.tsx
[error] 88-88: Expected blank line before this statement. padding-line-between-statements
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: 1
♻️ Duplicate comments (1)
components/NewsletterSubscribe.tsx (1)
82-90
: Good security practice: Removing console logs of user data.I see you've removed the console log that was exposing response data (which likely contained user details) after the fetch call, which aligns with the PR's objective to remove unnecessary console logs that expose user data. This is a good security practice.
🧰 Tools
🪛 ESLint
[error] 88-88: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 88-88: Delete
······
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
[error] 88-88: Trailing spaces not allowed. no-trailing-spaces
[error] 88-88: Delete
······
prettier/prettier
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/NewsletterSubscribe.tsx
(1 hunks)
🧰 Additional context used
🪛 ESLint
components/NewsletterSubscribe.tsx
[error] 88-88: Trailing spaces not allowed.
(no-trailing-spaces)
[error] 88-88: Delete ······
(prettier/prettier)
🪛 GitHub Actions: PR testing - if Node project
components/NewsletterSubscribe.tsx
[error] 88-88: Trailing spaces not allowed. no-trailing-spaces
[error] 88-88: Delete ······
prettier/prettier
⏰ Context from checks skipped due to timeout of 180000ms (2)
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3845 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 778 778
Branches 144 144
=========================================
Hits 778 778 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
All issues have been resolved. PTAL! |
/rtm |
asyncapi#3845) Co-authored-by: Sagar Kori <[email protected]> Co-authored-by: Chan <[email protected]> Co-authored-by: Sambhav Gupta <[email protected]> Co-authored-by: Akshat Nema <[email protected]>
This PR removes unnecessary console logs that were exposing user data and critical website flows in the browser console. Keeping these logs could lead to potential security risks and unintended data exposure.
Summary by CodeRabbit