feat: add banners for seats + license expiration #2440
Conversation
✅ Deploy Preview for gazebo-staging ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2440 +/- ##
==========================================
+ Coverage 98.09% 98.11% +0.01%
==========================================
Files 767 770 +3
Lines 9786 9870 +84
Branches 2479 2510 +31
==========================================
+ Hits 9600 9684 +84
Misses 184 184
Partials 2 2
Continue to review full report in Codecov by Sentry.
|
Codecov Report
@@ Coverage Diff @@
## main #2440 +/- ##
==========================================
+ Coverage 98.09% 98.11% +0.01%
==========================================
Files 767 770 +3
Lines 9786 9870 +84
Branches 2474 2496 +22
==========================================
+ Hits 9600 9684 +84
Misses 184 184
Partials 2 2
Continue to review full report in Codecov by Sentry.
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2440 +/- ##
=======================================
+ Coverage 98.10 98.12 +0.02
=======================================
Files 767 770 +3
Lines 9786 9870 +84
Branches 2460 2466 +6
=======================================
+ Hits 9600 9684 +84
Misses 184 184
Partials 2 2
Continue to review full report in Codecov by Sentry.
|
| @@ -21,54 +21,56 @@ describe('useStaticNavLinks', () => { | |||
| const links = view.result.current | |||
|
|
|||
| describe.each` | |||
| link | outcome | |||
There was a problem hiding this comment.
Moved the space to the right cause the name I made for the new variable didn't fit, that's why all of these changed, not unexpected
| .object({ | ||
| config: z | ||
| .object({ | ||
| seatsUsed: z.number(), |
There was a problem hiding this comment.
Had to add these since the last time I made this hook
| isLicenseExpiringWithin30Days: boolean | ||
| } | ||
|
|
||
| const LicenseExpirationModal = ({ |
There was a problem hiding this comment.
1/2 files where the meat is for the PR
| isLicenseExpiringWithin30Days: PropTypes.bool.isRequired, | ||
| } | ||
|
|
||
| const SelfHostedLicenseExpiration = () => { |
There was a problem hiding this comment.
1/2 files where the meat is for the PR
There was a problem hiding this comment.
Couple of comments to take a peak at. As well I'm a little concerned because we currently deploy "self-hosted" as our dedicated cloud and I did not see any specific checks to remove things for instance linking out to generating a new license, as dedicated cloud users really should not have that option.
| @@ -19,28 +21,34 @@ export const SelfHostedLicenseSchema = z | |||
|
|
|||
| export interface UseTierArgs { | |||
| provider: string | |||
| opts?: QueryOptions | |||
There was a problem hiding this comment.
Can you limit these options to just what you require, i.e. enabled in this case. There are typing issues if you just pass the whole options for instance select is totally broken when just using the standard QueryOptions type because there is no type information for the return type.
There was a problem hiding this comment.
I do see a fix for this in the future tho ...
| const LicenseExpirationModal = ({ | ||
| isModalOpen, | ||
| setIsModalOpen, | ||
| isSeatsLimitReached, | ||
| isLicenseExpired, | ||
| isLicenseExpiringWithin30Days, | ||
| }: LicenseExpirationModalArgs) => { |
There was a problem hiding this comment.
I would suggest swapping this over to:
const LicenseExpirationModal: React.FC<LicenseExpirationModalArgs> = ({ /* ... */ }) => {
// ...
}That way you get the correct return type checks
| <div className="flex gap-2"> | ||
| {/* @ts-expect-error */} | ||
| <Button | ||
| to={{ pageName: 'generateSelfHostedLicense' }} | ||
| showExternalIcon={false} | ||
| > | ||
| Generate New License | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
So because we're technically running self hosted for dedicated cloud, I believe this will still show for those users as well? We don't have any checks afaik that differentiate between the two.
.../GlobalBanners/SelfHostedLicenseExpiration/LicenseExpirationModal/LicenseExpirationModal.tsx
Show resolved
Hide resolved
| }) | ||
| afterEach(() => jest.resetAllMocks()) | ||
|
|
||
| it('does not render the banner when it should not be displayed', async () => { |
There was a problem hiding this comment.
Can you update this title to be a little more specific as to why it should not be displayed? Just to give a little more context as to what this is testing.
…f-hosted-expiration-banner
| @@ -160,7 +160,7 @@ describe('SelfHostedLicenseExpiration', () => { | |||
| }) | |||
| afterEach(() => jest.resetAllMocks()) | |||
|
|
|||
| it('does not render the banner when it should not be displayed', async () => { | |||
| it('does not render the banner when there is not a seat limit or when the license has not expired/will expire within 30 days', async () => { | |||
There was a problem hiding this comment.
Just being super picky here, no changes required for this PR, but for the future if you find yourself with one of these really long strings, you can always take it apart and wrap the it block inside of a describe so in this case something like the following, as well I might break this up into two tests even if it's checking the same outcome:
describe('when there is not a seat limit', () => {
it('does not render the banner', () => { /* ... */)
}
describe('when the license has not expired/will expire within 30 days', () => {
it('does not render the banner', () => {})
})…f-hosted-expiration-banner
…f-hosted-expiration-banner
…m/codecov/gazebo into 881-self-hosted-expiration-banner
Description
This PR adds a banner for self-hosted customers showing their expiration + seat count limitations.
Closes codecov/engineering-team#881
Notable Changes
SelfHostedLicenseExpirationcomponent and a modal to show the items of interest. Adds this to theGlobalBannerscomponentusenavlinksfileScreenshots
screen-capture.9.webm
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.