-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(preprod): Add second row with build number, version info and date to comparison screen (EME-520) #104117
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import {IconClose, IconCommit, IconFocus, IconLock, IconTelescope} from 'sentry/ | |
| import {t} from 'sentry/locale'; | ||
| import ProjectsStore from 'sentry/stores/projectsStore'; | ||
| import {trackAnalytics} from 'sentry/utils/analytics'; | ||
| import {getFormat, getFormattedDate} from 'sentry/utils/dates'; | ||
| import useOrganization from 'sentry/utils/useOrganization'; | ||
| import {useParams} from 'sentry/utils/useParams'; | ||
| import type {BuildDetailsApiResponse} from 'sentry/views/preprod/types/buildDetailsTypes'; | ||
|
|
@@ -35,12 +36,34 @@ function BuildButton({ | |
| const sha = buildDetails.vcs_info?.head_sha?.substring(0, 7); | ||
| const branchName = buildDetails.vcs_info?.head_ref; | ||
| const buildId = buildDetails.id; | ||
| const version = buildDetails.app_info?.version; | ||
| const buildNumber = buildDetails.app_info?.build_number; | ||
| const dateBuilt = buildDetails.app_info?.date_built; | ||
| const dateAdded = buildDetails.app_info?.date_added; | ||
|
|
||
| const buildUrl = `/organizations/${organization.slug}/preprod/${projectId}/${buildId}/`; | ||
| const platform = buildDetails.app_info?.platform ?? null; | ||
|
|
||
| const dateToShow = dateBuilt || dateAdded; | ||
| const formattedDate = getFormattedDate( | ||
| dateToShow, | ||
| getFormat({timeZone: true, year: true}), | ||
| { | ||
| local: true, | ||
| } | ||
| ); | ||
|
|
||
| // Build metadata parts for the second line | ||
| const metadataParts = [formattedDate]; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the date isn't optional, everything else is.
Comment on lines
+47
to
+57
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we always have at least the date_added so this shouldn't happen. |
||
| if (version) { | ||
| metadataParts.unshift(`Version ${version}`); | ||
| } | ||
| if (buildNumber) { | ||
| metadataParts.unshift(`Build ${buildNumber}`); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Metadata order reversed from documented intentThe |
||
|
|
||
| return ( | ||
| <LinkButton | ||
| <StyledLinkButton | ||
| to={buildUrl} | ||
| onClick={() => | ||
| trackAnalytics('preprod.builds.compare.go_to_build_details', { | ||
|
|
@@ -53,15 +76,17 @@ function BuildButton({ | |
| }) | ||
| } | ||
| > | ||
| <Flex align="center" gap="sm"> | ||
| {icon} | ||
| <Text size="sm" variant="accent" bold> | ||
| {label} | ||
| </Text> | ||
| <Flex align="center" gap="md"> | ||
| <Flex direction="column" gap="xs"> | ||
| <Flex align="center" gap="sm"> | ||
| {icon} | ||
| <Text size="sm" variant="accent" bold> | ||
| {`#${buildId}`} | ||
| {label} | ||
| </Text> | ||
| {!buildNumber && ( | ||
| <Text size="sm" variant="accent" bold> | ||
| {`#${buildId}`} | ||
| </Text> | ||
| )} | ||
| {sha && ( | ||
| <Flex align="center" gap="xs"> | ||
| <IconCommit size="xs" /> | ||
|
|
@@ -70,33 +95,43 @@ function BuildButton({ | |
| </Text> | ||
| </Flex> | ||
| )} | ||
| {branchName && ( | ||
| <BuildBranch> | ||
| <Text size="sm" variant="muted"> | ||
| {branchName} | ||
| </Text> | ||
| </BuildBranch> | ||
| )} | ||
| {onRemove && ( | ||
| <Button | ||
| onClick={e => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| onRemove(); | ||
| }} | ||
| size="zero" | ||
| priority="transparent" | ||
| borderless | ||
| aria-label={t('Clear base build')} | ||
| icon={<IconClose size="xs" color="purple400" />} | ||
| /> | ||
| )} | ||
| </Flex> | ||
| <Flex align="center" gap="sm"> | ||
| <Text size="sm" variant="muted"> | ||
| {metadataParts.join(' • ')} | ||
| </Text> | ||
| </Flex> | ||
| {branchName && ( | ||
| <BuildBranch> | ||
| <Text size="sm" variant="muted"> | ||
| {branchName} | ||
| </Text> | ||
| </BuildBranch> | ||
| )} | ||
| {onRemove && ( | ||
| <Button | ||
| onClick={e => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| onRemove(); | ||
| }} | ||
| size="zero" | ||
| priority="transparent" | ||
| borderless | ||
| aria-label={t('Clear base build')} | ||
| icon={<IconClose size="xs" color="purple400" />} | ||
| /> | ||
| )} | ||
| </Flex> | ||
| </LinkButton> | ||
| </StyledLinkButton> | ||
| ); | ||
| } | ||
|
|
||
| const StyledLinkButton = styled(LinkButton)` | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to create this otherwise the |
||
| height: auto; | ||
| min-height: auto; | ||
| `; | ||
|
|
||
| const ComparisonContainer = styled(Flex)` | ||
| flex-wrap: wrap; | ||
| align-items: center; | ||
|
|
||
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: Missing date displays current date instead of fallback
When both
dateBuiltanddateAddedare undefined or null,dateToShowbecomes undefined. Passing undefined togetFormattedDate(which wraps moment.js) causes it to return the current date/time instead of handling the missing data case. This results in displaying today's date in the metadata line, which would mislead users about when the build was actually created.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.
I believe we always have at least the
date_addedso this shouldn't happen.