Browse Source

feat(releases): Convert release commits to FC (#63860)

Scott Cooper 1 year ago
parent
commit
4e7ffaf39a

+ 126 - 0
static/app/views/releases/detail/commitsAndFiles/commits.spec.tsx

@@ -0,0 +1,126 @@
+import selectEvent from 'react-select-event';
+import {CommitFixture} from 'sentry-fixture/commit';
+import {ReleaseFixture} from 'sentry-fixture/release';
+import {ReleaseProjectFixture} from 'sentry-fixture/releaseProject';
+import {RepositoryFixture} from 'sentry-fixture/repository';
+
+import {initializeOrg} from 'sentry-test/initializeOrg';
+import {render, screen} from 'sentry-test/reactTestingLibrary';
+
+import RepositoryStore from 'sentry/stores/repositoryStore';
+import type {ReleaseProject} from 'sentry/types';
+import {ReleaseContext} from 'sentry/views/releases/detail';
+
+import Commits from './commits';
+
+describe('Commits', () => {
+  const release = ReleaseFixture();
+  const project = ReleaseProjectFixture() as Required<ReleaseProject>;
+  const {routerProps, routerContext, organization} = initializeOrg({
+    router: {params: {release: release.version}},
+  });
+  const repos = [RepositoryFixture({integrationId: '1'})];
+
+  function renderComponent() {
+    return render(
+      <ReleaseContext.Provider
+        value={{
+          release,
+          project,
+          deploys: [],
+          refetchData: () => {},
+          hasHealthData: false,
+          releaseBounds: {} as any,
+          releaseMeta: {} as any,
+        }}
+      >
+        <Commits releaseRepos={[]} projectSlug={project.slug} {...routerProps} />
+      </ReleaseContext.Provider>,
+      {context: routerContext}
+    );
+  }
+
+  beforeEach(() => {
+    MockApiClient.clearMockResponses();
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/repos/`,
+      body: repos,
+    });
+    RepositoryStore.init();
+  });
+
+  it('should render no repositories message', async () => {
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/repos/`,
+      body: [],
+    });
+    renderComponent();
+    expect(
+      await screen.findByText('Releases are better with commit data!')
+    ).toBeInTheDocument();
+  });
+
+  it('should render no commits', async () => {
+    MockApiClient.addMockResponse({
+      url: `/projects/${organization.slug}/${project.slug}/releases/${encodeURIComponent(
+        release.version
+      )}/repositories/`,
+      body: repos,
+    });
+    MockApiClient.addMockResponse({
+      url: `/organizations/org-slug/releases/${encodeURIComponent(
+        release.version
+      )}/commits/`,
+      body: [],
+    });
+    renderComponent();
+    expect(await screen.findByText(/There are no commits/)).toBeInTheDocument();
+  });
+
+  it('should render list of commits', async () => {
+    MockApiClient.addMockResponse({
+      url: `/projects/${organization.slug}/${project.slug}/releases/${encodeURIComponent(
+        release.version
+      )}/repositories/`,
+      body: repos,
+    });
+    MockApiClient.addMockResponse({
+      url: `/organizations/org-slug/releases/${encodeURIComponent(
+        release.version
+      )}/commits/`,
+      body: [CommitFixture()],
+    });
+    renderComponent();
+    expect(
+      await screen.findByText('(improve) Add Links to Spike-Protection Email (#2408)')
+    ).toBeInTheDocument();
+    expect(screen.getByText('f7f395d')).toBeInTheDocument();
+  });
+
+  it('should render repo picker', async () => {
+    MockApiClient.addMockResponse({
+      url: `/projects/${organization.slug}/${project.slug}/releases/${encodeURIComponent(
+        release.version
+      )}/repositories/`,
+      body: [
+        repos[0]!,
+        RepositoryFixture({
+          id: '5',
+          name: 'getsentry/sentry-frontend',
+          integrationId: '1',
+        }),
+      ],
+    });
+    MockApiClient.addMockResponse({
+      url: `/organizations/org-slug/releases/${encodeURIComponent(
+        release.version
+      )}/commits/`,
+      body: [CommitFixture()],
+    });
+    renderComponent();
+    expect(await screen.findByRole('button')).toHaveTextContent('example/repo-name');
+    expect(screen.queryByText('getsentry/sentry-frontend')).not.toBeInTheDocument();
+    selectEvent.openMenu(screen.getByRole('button'));
+    expect(screen.getByText('getsentry/sentry-frontend')).toBeInTheDocument();
+  });
+});

+ 76 - 120
static/app/views/releases/detail/commitsAndFiles/commits.tsx

@@ -4,16 +4,21 @@ import type {Location} from 'history';
 
 import {CommitRow} from 'sentry/components/commitRow';
 import * as Layout from 'sentry/components/layouts/thirds';
+import LoadingError from 'sentry/components/loadingError';
 import LoadingIndicator from 'sentry/components/loadingIndicator';
 import Pagination from 'sentry/components/pagination';
 import Panel from 'sentry/components/panels/panel';
 import PanelBody from 'sentry/components/panels/panelBody';
 import PanelHeader from 'sentry/components/panels/panelHeader';
+import SentryDocumentTitle from 'sentry/components/sentryDocumentTitle';
 import {t} from 'sentry/locale';
-import type {Commit, Organization, Project, Repository} from 'sentry/types';
+import type {Commit, Project, Repository} from 'sentry/types';
 import {formatVersion} from 'sentry/utils/formatters';
+import {useApiQuery} from 'sentry/utils/queryClient';
 import routeTitleGen from 'sentry/utils/routeTitle';
-import DeprecatedAsyncView from 'sentry/views/deprecatedAsyncView';
+import {useLocation} from 'sentry/utils/useLocation';
+import useOrganization from 'sentry/utils/useOrganization';
+import {useParams} from 'sentry/utils/useParams';
 
 import {getCommitsByRepository, getQuery, getReposToRender} from '../utils';
 
@@ -21,136 +26,87 @@ import EmptyState from './emptyState';
 import RepositorySwitcher from './repositorySwitcher';
 import withReleaseRepos from './withReleaseRepos';
 
-type Props = RouteComponentProps<{release: string}, {}> & {
+interface CommitsProps extends RouteComponentProps<{release: string}, {}> {
   location: Location;
-  orgSlug: Organization['slug'];
   projectSlug: Project['slug'];
-  release: string;
   releaseRepos: Repository[];
   activeReleaseRepo?: Repository;
-} & DeprecatedAsyncView['props'];
-
-type State = {
-  commits: Commit[];
-} & DeprecatedAsyncView['state'];
-
-class Commits extends DeprecatedAsyncView<Props, State> {
-  getTitle() {
-    const {params, orgSlug, projectSlug} = this.props;
-
-    return routeTitleGen(
-      t('Commits - Release %s', formatVersion(params.release)),
-      orgSlug,
-      false,
-      projectSlug
-    );
-  }
-
-  getDefaultState(): State {
-    return {
-      ...super.getDefaultState(),
-      commits: [],
-    };
-  }
-
-  componentDidUpdate(prevProps: Props, prevState: State) {
-    if (prevProps.activeReleaseRepo?.name !== this.props.activeReleaseRepo?.name) {
-      this.remountComponent();
-      return;
-    }
-    super.componentDidUpdate(prevProps, prevState);
-  }
-
-  getEndpoints(): ReturnType<DeprecatedAsyncView['getEndpoints']> {
-    const {
-      projectSlug,
-      activeReleaseRepo: activeRepository,
-      location,
-      orgSlug,
-      release,
-    } = this.props;
-
-    const query = getQuery({location, activeRepository});
-
-    return [
-      [
-        'commits',
-        `/projects/${orgSlug}/${projectSlug}/releases/${encodeURIComponent(
-          release
-        )}/commits/`,
-        {query},
-      ],
-    ];
-  }
-
-  renderLoading() {
-    return this.renderBody();
-  }
-
-  renderContent() {
-    const {commits, commitsPageLinks, loading} = this.state;
-    const {activeReleaseRepo} = this.props;
-
-    if (loading) {
-      return <LoadingIndicator />;
-    }
+}
 
-    if (!commits.length) {
-      return (
-        <EmptyState>
-          {!activeReleaseRepo
-            ? t('There are no commits associated with this release.')
-            : t(
-                'There are no commits associated with this release in the %s repository.',
-                activeReleaseRepo.name
-              )}
-        </EmptyState>
-      );
+function Commits({activeReleaseRepo, releaseRepos, projectSlug}: CommitsProps) {
+  const location = useLocation();
+  const params = useParams<{release: string}>();
+  const organization = useOrganization();
+
+  const query = getQuery({location, activeRepository: activeReleaseRepo});
+  const {
+    data: commitList = [],
+    isLoading: isLoadingCommitList,
+    error: commitListError,
+    refetch,
+    getResponseHeader,
+  } = useApiQuery<Commit[]>(
+    [
+      `/organizations/${organization.slug}/releases/${encodeURIComponent(
+        params.release
+      )}/commits/`,
+      {query},
+    ],
+    {
+      staleTime: Infinity,
     }
-
-    const commitsByRepository = getCommitsByRepository(commits);
-    const reposToRender = getReposToRender(Object.keys(commitsByRepository));
-
-    return (
-      <Fragment>
-        {reposToRender.map(repoName => (
-          <Panel key={repoName}>
-            <PanelHeader>{repoName}</PanelHeader>
-            <PanelBody>
-              {commitsByRepository[repoName]?.map(commit => (
-                <CommitRow key={commit.id} commit={commit} />
-              ))}
-            </PanelBody>
-          </Panel>
-        ))}
-        <Pagination pageLinks={commitsPageLinks} />
-      </Fragment>
-    );
-  }
-
-  renderBody() {
-    const {activeReleaseRepo, releaseRepos} = this.props;
-
-    return (
-      <Fragment>
+  );
+
+  const commitsByRepository = getCommitsByRepository(commitList);
+  const reposToRender = getReposToRender(Object.keys(commitsByRepository));
+
+  return (
+    <Layout.Body>
+      <Layout.Main fullWidth>
+        <SentryDocumentTitle
+          title={routeTitleGen(
+            t('Commits - Release %s', formatVersion(params.release)),
+            organization.slug,
+            false,
+            projectSlug
+          )}
+        />
         {releaseRepos.length > 1 && (
           <RepositorySwitcher
             repositories={releaseRepos}
             activeRepository={activeReleaseRepo}
           />
         )}
-        {this.renderContent()}
-      </Fragment>
-    );
-  }
-
-  renderComponent() {
-    return (
-      <Layout.Body>
-        <Layout.Main fullWidth>{super.renderComponent()}</Layout.Main>
-      </Layout.Body>
-    );
-  }
+        {commitListError && <LoadingError onRetry={refetch} />}
+        {isLoadingCommitList ? (
+          <LoadingIndicator />
+        ) : commitList.length ? (
+          <Fragment>
+            {reposToRender.map(repoName => (
+              <Panel key={repoName}>
+                <PanelHeader>{repoName}</PanelHeader>
+                <PanelBody>
+                  {commitsByRepository[repoName]?.map(commit => (
+                    <CommitRow key={commit.id} commit={commit} />
+                  ))}
+                </PanelBody>
+              </Panel>
+            ))}
+            <Pagination pageLinks={getResponseHeader?.('Link')} />
+          </Fragment>
+        ) : (
+          <EmptyState>
+            {activeReleaseRepo
+              ? t(
+                  'There are no commits associated with this release in the %s repository.',
+                  activeReleaseRepo.name
+                )
+              : t('There are no commits associated with this release.')}
+          </EmptyState>
+        )}
+      </Layout.Main>
+    </Layout.Body>
+  );
 }
 
 export default withReleaseRepos(Commits);

+ 2 - 4
static/app/views/releases/detail/utils.tsx

@@ -22,9 +22,7 @@ import {decodeList} from 'sentry/utils/queryString';
 import {getReleaseBounds, getReleaseParams, isMobileRelease} from '../utils';
 import {commonTermsDescription, SessionTerm} from '../utils/sessionTerm';
 
-export type CommitsByRepository = {
-  [key: string]: Commit[];
-};
+type CommitsByRepository = Record<string, Commit[]>;
 
 /**
  * Convert list of individual file changes into a per-file summary grouped by repository
@@ -58,7 +56,7 @@ export function getFilesByRepository(fileList: CommitFile[]) {
  * Convert list of individual commits into a summary grouped by repository
  */
 export function getCommitsByRepository(commitList: Commit[]): CommitsByRepository {
-  return commitList.reduce((commitsByRepository, commit) => {
+  return commitList.reduce<CommitsByRepository>((commitsByRepository, commit) => {
     const repositoryName = commit.repository?.name ?? t('unknown');
 
     if (!commitsByRepository.hasOwnProperty(repositoryName)) {