feat: org release limits #8682
feat: org release limits #8682
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
jordanl17
force-pushed
the
feat/corel-org-limits-simple
branch
from
4d0968a to
01401f1
Compare
|
No changes to documentation |
⚡️ Editor Performance ReportUpdated Fri, 21 Feb 2025 17:42:55 GMT
Detailed information🏠 Reference resultThe performance result of
🧪 Experiment resultThe performance result of this branch
📚 Glossary
|
|
Component Testing Report Updated Feb 21, 2025 5:47 PM (UTC) ❌ Failed Tests (4) -- expand for details
|
jordanl17
force-pushed
the
feat/corel-org-limits-simple
branch
from
09a9bcd to
36f9b37
Compare
…sion checks call (assume permissions)
…ssion checks call (assume permissions)
jordanl17
force-pushed
the
feat/corel-org-limits-simple
branch
from
af07dad to
6c1ecd9
Compare
| @@ -184,7 +184,8 @@ export function ReleasesList({ | |||
| text={t('release.action.create-new')} | |||
| data-testid="create-new-release-button" | |||
| tooltipProps={{ | |||
| content: !hasCreatePermission && t('release.action.permission.error'), | |||
| disabled: hasCreatePermission === true, | |||
There was a problem hiding this comment.
@RitaDias checking that this (and other uses elsewhere) is true explicitly so that the tooltip doesn't show whilst we are pending fetching the permissions
jordanl17
requested review from
RitaDias and
juice49
and removed request for
a team
| catchError((error: ClientError) => { | ||
| console.error(error) | ||
|
|
||
| if (typeof error.response.body !== 'string' && 'data' in error.response.body) { |
There was a problem hiding this comment.
Does this handle the scenario of the quota being reached? If so, would it be safer to additionally check the response status code?
I'm just wondering whether there are any other errors that could produce a response body containing a data property that would incorrectly be caught here.
There was a problem hiding this comment.
Yes I can do - on this particular endpoint, as it currently stands, data will always be of our contractually expected format. I'll need to check on all the error codes to allow as there are a handful depending on what the particular error is
There was a problem hiding this comment.
Cool. I don't think we necessarily need it, just wanted to check! 🙂
| @@ -355,9 +355,11 @@ export function createRequestAction( | |||
| }, | |||
| }) | |||
| } catch (e) { | |||
| if (isReleaseLimitError(e)) { | |||
| // if dryRunning then essentially this is a silent request | |||
| // so don't want to create disruptive upsell because of limit | |||
| const activeReleasesCount = activeReleases?.length || 0 | ||
|
|
||
| // dependencies must be objects not primitives, so nesting activeReleasesCount in an object | ||
| const count = useMemo(() => ({activeReleasesCount}), [activeReleasesCount]) |
There was a problem hiding this comment.
By creating an object in the useOrgActiveReleaseCount hook to use as a key, won't this code create and cache a new instance of the createOrgActiveReleaseCountStore store for every component that invokes it?
There was a problem hiding this comment.
Yeh that's true. This is a bug for sure! It doesn't get surfaced as a bug currently because useorgActiveReleaseCount is only used in ReleaseUpsellProvider. Perhaps useActiveReleases should be returning a memoized object instead?
There was a problem hiding this comment.
I think I would consider having the observable produced by createOrgActiveReleaseCountStore consume the observable produced by useActiveReleases and deriving its state based on that, rather than injecting a snapshot of it at creation-time.
It's not necessarily a problem for now. Just thinking out loud.
Description
With the API returning
With the API not existing
Releases+
Free project after trial:
Free project with trial:
Enterprise project:
What to review
useOrgActiveReleaseCountanduseReleaseLimits, and the way they emit their datadatawill still be returned an we assume limits not reached and releases+ not enabled, and internal errors on the server, eg when it goes away or is down.useOrgActiveReleaseCount - it caches for a TTL (60s) or until theactiveReleases.length` changesactiveReleases.lengthanddata.orgActiveReleaseCountTesting
Manually tested by above scenarios for now - unlikely we will fast-follow tests given this solution will go away in the close future
Notes for release
N/A