Enable/disable tabs and flag select based on team plan and private repo status #2377
Conversation
✅ Deploy Preview for gazebo-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov Report
@@ Coverage Diff @@
## main #2377 +/- ##
=======================================
Coverage 96.27% 96.27%
=======================================
Files 736 736
Lines 8956 8967 +11
Branches 2180 2231 +51
=======================================
+ Hits 8622 8633 +11
+ Misses 318 317 -1
- Partials 16 17 +1
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2377 +/- ##
=======================================
Coverage 96.27% 96.27%
=======================================
Files 736 736
Lines 8956 8967 +11
Branches 2180 2218 +38
=======================================
+ Hits 8622 8633 +11
+ Misses 318 317 -1
- Partials 16 17 +1
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Codecov Report
@@ Coverage Diff @@
## main #2377 +/- ##
=======================================
+ Coverage 96.27 96.28 +0.01
=======================================
Files 736 736
Lines 8956 8967 +11
Branches 2224 2236 +12
=======================================
+ Hits 8622 8633 +11
+ Misses 318 317 -1
- Partials 16 17 +1
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
✅ Deploy Preview for gazebo-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -18,13 +19,16 @@ function PullRequestPageTabs() { | |||
| indirectChangesCount, | |||
| directChangedFilesCount, | |||
| commitsCount, | |||
| isPrivateRepo, | |||
There was a problem hiding this comment.
How do you feel about using useRepoSettingsTeam and getting the private variable from there, instead of adding it to this hook? It feels a little weird that we're getting that from a hook called useTabsCounts and that the only reason we're fetching it from there is cause we already had access to the repository resolver so adding the key is inconvenient. On the flip side we had to change 6 files total to make this happen, while only the change affects this file on the component side. I mainly feel the hook starts to drift from being what it's meant to do, returning counts 😬
There was a problem hiding this comment.
It's definitely drifting, I was mostly trying to avoid adding another network request and this was all ready being used in the code, that being said it's definitely a short cut. To be honest I wasnt aware of the useRepoSettingsTeam hook, I think it makes sense to refactor to use that one instead.
| graphql.query('OwnerTier', (req, res, ctx) => | ||
| res( | ||
| ctx.status(200), | ||
| ctx.data({ owner: { plan: { tierName: plan.toLowerCase() } } }) |
There was a problem hiding this comment.
Nitpick: could we rename plan to tier ?
There was a problem hiding this comment.
I could at the query level, but it is called plan in the actual resolver so maybe we should also update it there and deprecate the plan resolver?
There was a problem hiding this comment.
Ah I should have been bit more specific, meant plan: { tierName: tier.toLowerCase()}}, cause tierName is a tier, not a plan itself
There was a problem hiding this comment.
Oh I see! Just to make sure I understand I need to replace the tiers with plans, so it needs to be
plan: { planName: plan.toLowerCase()}} ?
The existing useTier.ts is using tier, so is that no longer correct or are you asking for a new service for plans?
| @@ -36,7 +36,6 @@ const mockPullData = { | |||
| isCurrentUserPartOfOrg: true, | |||
| repository: { | |||
| __typename: 'Repository', | |||
| private: true, | |||
There was a problem hiding this comment.
Huh, not sure why this registered as a change but 🤷
terry-codecov
deleted the
624-client-pr-page-remove-multiple-tabs-for-team-tier
branch
Description
Updates the tabs and flag selector based on plan type and repo status
Addresses:
codecov/engineering-team#624
codecov/engineering-team#686
Code Example
Notable Changes
Screenshots
Team Plan Private repo
Non Team plan / public
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.