-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
ref(releases): Rebuild ReleasesPromo #98061
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
ref(releases): Rebuild ReleasesPromo #98061
Conversation
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.
The code LGTM but the behaviour seems weird:
- I can see the "Token From" dropdown on the preview deploy, but I don't see anything like that in production on my user. Is the permission check bugged?
- When I change the token in the dropdown, it doesn't update the code snippet
I'd like to merge this as is, so we can unblock the removal of The right behavior is probably to make an API query to get the list of tokens for the selected integration, and if there's no tokens we can give a warning to the user that they don't have permission to create a token for the selected app. |
66a956b
to
ac3b3b9
Compare
- Replaces the deprecated dropdownAutoComplete with a CompactSelect - Removes the complex logic to have a <click here for token> with a much simpler interface that uses a familiar drop-down to select the integration to use a token from.
ac3b3b9
to
dad0741
Compare
triggerLabel={selectedApp ? undefined : t('Select Integration')} | ||
triggerProps={{prefix: selectedApp ? t('Token From') : undefined}} | ||
onChange={option => { | ||
const app = apps.find(i => i.slug === option.value)!; |
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.
Bug: Unsafe Non-Null Assertion in onChange
Handler
The non-null assertion operator (!
) on apps.find()
in the CompactSelect
's onChange
handler can lead to a runtime error. If the selected option's value doesn't match an existing app (e.g., due to race conditions or data inconsistencies), apps.find()
returns undefined
, causing a crash.
- Replaces the deprecated dropdownAutoComplete with a CompactSelect - Removes the complex logic to have a <click here for token> with a much simpler interface that uses a familiar drop-down to select the integration to use a token from. See example here https://sentry-kajlvytky.sentry.dev/explore/releases/?project=4503972821204992
- Replaces the deprecated dropdownAutoComplete with a CompactSelect - Removes the complex logic to have a <click here for token> with a much simpler interface that uses a familiar drop-down to select the integration to use a token from. See example here https://sentry-kajlvytky.sentry.dev/explore/releases/?project=4503972821204992
Replaces the deprecated dropdownAutoComplete with a CompactSelect
Removes the complex logic to have a with a much
simpler interface that uses a familiar drop-down to select the
integration to use a token from.
See example here https://sentry-kajlvytky.sentry.dev/explore/releases/?project=4503972821204992