-
Notifications
You must be signed in to change notification settings - Fork 24
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
fix: Only show GH app config banner if admin #3691
Conversation
{/* we only want one of these two banners to show at a time */} | ||
{!hasGhApp && !showDemoAlert && <GithubConfigBanner />} | ||
{/* we only want one of this or DemoAlert banners to show at a time */} | ||
{isAdmin && !hasGhApp && !showDemoAlert ? <GithubConfigBanner /> : null} |
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.
This is the main change.
Bundle ReportChanges will increase total bundle size by 443 bytes (0.0%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
Codecov ReportAttention: Patch coverage is ✅ All tests successful. No failed tests found.
@@ Coverage Diff @@
## main #3691 +/- ##
==========================================
- Coverage 98.79% 98.78% -0.01%
==========================================
Files 825 825
Lines 14808 14806 -2
Branches 4203 4193 -10
==========================================
- Hits 14629 14626 -3
- Misses 170 171 +1
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
@@ Coverage Diff @@
## main #3691 +/- ##
==========================================
- Coverage 98.79% 98.78% -0.01%
==========================================
Files 825 825
Lines 14808 14806 -2
Branches 4203 4193 -10
==========================================
- Hits 14629 14626 -3
- Misses 170 171 +1
Partials 9 9
Continue to review full report in Codecov by Sentry.
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
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.
🥔
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.
Seems to be working! v nice
Description
Closes codecov/engineering-team#3287
Background info copied from ^ for context:
In the new non-PAT appless experience, we made a shift of rendering the GH Config Banner from
<HeaderBanners>
to<ListRepo>
which moved it outside of scope of theSilentNetworkErrorWrapper
. Theaccount-details
API (used for getting data about if org has GH app installed) call 404s inside of that ErrorBoundary wrapper so anythingisGHApp
related would just error and be caught by the boundary. Now that we've moved<GithubConfigBanner>
out, it needs more boolean checks to make sure we only show it for admins on their org that does not have the GH app already installed. We also don't want to show this at the same time as the "Welcome" banner when someone just onboards.Notable Changes
Check that the user viewing the org page is an admin to get the GH Config Banner. Rest of the changes are just minor refactors and TS conversion made while I was there.
Screenshots
Link to Sample Entry
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.