From 63145686f49e8fc3bb37b5a010b93d0136d89ff4 Mon Sep 17 00:00:00 2001 From: Enver Bisevac Date: Fri, 11 Aug 2023 13:29:25 +0200 Subject: [PATCH] fix commit adn files changed counters --- gitrpc/commit.go | 17 + gitrpc/internal/service/commit.go | 22 + internal/api/controller/repo/list_commits.go | 18 +- internal/api/handler/repo/diff.go | 23 + internal/api/handler/repo/list_commits.go | 12 +- internal/api/openapi/repo.go | 12 +- internal/router/api.go | 3 + types/git.go | 1 + web/src/components/Changes/Changes.tsx | 18 +- .../TabContentWrapper/TabContentWrapper.tsx | 27 + web/src/pages/Compare/Compare.tsx | 103 +- web/src/pages/Compare/CompareCommits.tsx | 58 + web/src/pages/PullRequest/PullRequest.tsx | 4 +- .../PullRequestCommits/PullRequestCommits.tsx | 14 +- web/src/services/code/index.tsx | 1 + web/src/services/code/swagger.yaml | 4180 +++++++++-------- 16 files changed, 2358 insertions(+), 2155 deletions(-) create mode 100644 web/src/components/TabContentWrapper/TabContentWrapper.tsx create mode 100644 web/src/pages/Compare/CompareCommits.tsx diff --git a/gitrpc/commit.go b/gitrpc/commit.go index 4397e65b8..5408a560d 100644 --- a/gitrpc/commit.go +++ b/gitrpc/commit.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "io" + "strconv" "time" "github.com/harness/gitness/gitrpc/rpc" @@ -97,6 +98,7 @@ type RenameDetails struct { type ListCommitsOutput struct { Commits []Commit RenameDetails []*RenameDetails + TotalCommits int } func (c *Client) ListCommits(ctx context.Context, params *ListCommitsParams) (*ListCommitsOutput, error) { @@ -122,6 +124,21 @@ func (c *Client) ListCommits(ctx context.Context, params *ListCommitsParams) (*L Commits: make([]Commit, 0, 16), } + // check for list commits header + header, err := stream.Header() + if err != nil { + return nil, processRPCErrorf(err, "failed to read list commits header from stream") + } + + values := header.Get("total-commits") + if len(values) > 0 && values[0] != "" { + total, err := strconv.ParseInt(values[0], 10, 32) + if err != nil { + return nil, processRPCErrorf(err, "failed to convert header total-commits") + } + output.TotalCommits = int(total) + } + for { var next *rpc.ListCommitsResponse next, err = stream.Recv() diff --git a/gitrpc/internal/service/commit.go b/gitrpc/internal/service/commit.go index e467dfafe..7d967b84b 100644 --- a/gitrpc/internal/service/commit.go +++ b/gitrpc/internal/service/commit.go @@ -6,12 +6,14 @@ package service import ( "context" + "strconv" "github.com/harness/gitness/gitrpc/internal/types" "github.com/harness/gitness/gitrpc/rpc" "github.com/rs/zerolog/log" "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" ) @@ -65,7 +67,27 @@ func (s RepositoryService) ListCommits(request *rpc.ListCommitsRequest, return processGitErrorf(err, "failed to get list of commits") } + // try to get total commits between gitref and After refs + totalCommits := 0 + if request.Page == 1 && len(gitCommits) < int(request.Limit) { + totalCommits = len(gitCommits) + } else if request.After != "" && request.GitRef != request.After { + div, err := s.adapter.GetCommitDivergences(ctx, repoPath, []types.CommitDivergenceRequest{ + {From: request.GitRef, To: request.After}, + }, 0) + if err != nil { + return processGitErrorf(err, "failed to get total commits") + } + if len(div) > 0 { + totalCommits = int(div[0].Ahead) + } + } + log.Ctx(ctx).Trace().Msgf("git adapter returned %d commits", len(gitCommits)) + header := metadata.New(map[string]string{"total-commits": strconv.Itoa(totalCommits)}) + if err := stream.SendHeader(header); err != nil { + return ErrInternalf("unable to send 'total-commits' header", err) + } for i := range gitCommits { var commit *rpc.Commit diff --git a/internal/api/controller/repo/list_commits.go b/internal/api/controller/repo/list_commits.go index 6ba6d3eb1..d4994034c 100644 --- a/internal/api/controller/repo/list_commits.go +++ b/internal/api/controller/repo/list_commits.go @@ -20,14 +20,14 @@ import ( * ListCommits lists the commits of a repo. */ func (c *Controller) ListCommits(ctx context.Context, session *auth.Session, - repoRef string, gitRef string, filter *types.CommitFilter) ([]types.Commit, []types.RenameDetails, error) { + repoRef string, gitRef string, filter *types.CommitFilter) (types.ListCommitResponse, error) { repo, err := c.repoStore.FindByRef(ctx, repoRef) if err != nil { - return nil, nil, err + return types.ListCommitResponse{}, err } if err = apiauth.CheckRepo(ctx, c.authorizer, session, repo, enum.PermissionRepoView, false); err != nil { - return nil, nil, err + return types.ListCommitResponse{}, err } // set gitRef to default branch in case an empty reference was provided @@ -47,7 +47,7 @@ func (c *Controller) ListCommits(ctx context.Context, session *auth.Session, Committer: filter.Committer, }) if err != nil { - return nil, nil, err + return types.ListCommitResponse{}, err } commits := make([]types.Commit, len(rpcOut.Commits)) @@ -55,7 +55,7 @@ func (c *Controller) ListCommits(ctx context.Context, session *auth.Session, var commit *types.Commit commit, err = controller.MapCommit(&rpcOut.Commits[i]) if err != nil { - return nil, nil, fmt.Errorf("failed to map commit: %w", err) + return types.ListCommitResponse{}, fmt.Errorf("failed to map commit: %w", err) } commits[i] = *commit } @@ -64,9 +64,13 @@ func (c *Controller) ListCommits(ctx context.Context, session *auth.Session, for i := range rpcOut.RenameDetails { renameDetails := controller.MapRenameDetails(rpcOut.RenameDetails[i]) if renameDetails == nil { - return nil, nil, fmt.Errorf("rename details was nil") + return types.ListCommitResponse{}, fmt.Errorf("rename details was nil") } renameDetailList[i] = *renameDetails } - return commits, renameDetailList, nil + return types.ListCommitResponse{ + Commits: commits, + RenameDetails: renameDetailList, + TotalCommits: rpcOut.TotalCommits, + }, nil } diff --git a/internal/api/handler/repo/diff.go b/internal/api/handler/repo/diff.go index d8c6ac11b..23581781e 100644 --- a/internal/api/handler/repo/diff.go +++ b/internal/api/handler/repo/diff.go @@ -43,3 +43,26 @@ func HandleDiff(repoCtrl *repo.Controller) http.HandlerFunc { render.JSONArrayDynamic(ctx, w, stream) } } + +// HandleDiffStats how diff statistics of two commits, branches or tags. +func HandleDiffStats(repoCtrl *repo.Controller) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + session, _ := request.AuthSessionFrom(ctx) + repoRef, err := request.GetRepoRefFromPath(r) + if err != nil { + render.TranslatedUserError(w, err) + return + } + + path := request.GetOptionalRemainderFromPath(r) + + output, err := repoCtrl.DiffStats(ctx, session, repoRef, path) + if err != nil { + render.TranslatedUserError(w, err) + return + } + + render.JSON(w, http.StatusOK, output) + } +} diff --git a/internal/api/handler/repo/list_commits.go b/internal/api/handler/repo/list_commits.go index 3b0e5dd16..f970efaa5 100644 --- a/internal/api/handler/repo/list_commits.go +++ b/internal/api/handler/repo/list_commits.go @@ -10,7 +10,6 @@ import ( "github.com/harness/gitness/internal/api/controller/repo" "github.com/harness/gitness/internal/api/render" "github.com/harness/gitness/internal/api/request" - "github.com/harness/gitness/types" ) /* @@ -34,20 +33,15 @@ func HandleListCommits(repoCtrl *repo.Controller) http.HandlerFunc { return } - commits, renameDetails, err := repoCtrl.ListCommits(ctx, session, repoRef, gitRef, filter) + list, err := repoCtrl.ListCommits(ctx, session, repoRef, gitRef, filter) if err != nil { render.TranslatedUserError(w, err) return } - commitsResponse := types.ListCommitResponse{ - Commits: commits, - RenameDetails: renameDetails, - } - // TODO: get last page indicator explicitly - current check is wrong in case len % limit == 0 - isLastPage := len(commits) < filter.Limit + isLastPage := len(list.Commits) < filter.Limit render.PaginationNoTotal(r, w, filter.Page, filter.Limit, isLastPage) - render.JSON(w, http.StatusOK, commitsResponse) + render.JSON(w, http.StatusOK, list) } } diff --git a/internal/api/openapi/repo.go b/internal/api/openapi/repo.go index 2d9478a3a..8fb613eb9 100644 --- a/internal/api/openapi/repo.go +++ b/internal/api/openapi/repo.go @@ -661,13 +661,23 @@ func repoOperations(reflector *openapi3.Reflector) { opDiff.WithTags("repository") opDiff.WithMapOfAnything(map[string]interface{}{"operationId": "rawDiff"}) _ = reflector.SetRequest(&opDiff, new(getRawDiffRequest), http.MethodGet) - _ = reflector.SetJSONResponse(&opDiff, new(types.DiffStats), http.StatusOK) _ = reflector.SetStringResponse(&opDiff, http.StatusOK, "text/plain") + _ = reflector.SetJSONResponse(&opDiff, []gitrpc.FileDiff{}, http.StatusOK) _ = reflector.SetJSONResponse(&opDiff, new(usererror.Error), http.StatusInternalServerError) _ = reflector.SetJSONResponse(&opDiff, new(usererror.Error), http.StatusUnauthorized) _ = reflector.SetJSONResponse(&opDiff, new(usererror.Error), http.StatusForbidden) _ = reflector.Spec.AddOperation(http.MethodGet, "/repos/{repo_ref}/diff/{range}", opDiff) + opDiffStats := openapi3.Operation{} + opDiffStats.WithTags("repository") + opDiffStats.WithMapOfAnything(map[string]interface{}{"operationId": "diffStats"}) + _ = reflector.SetRequest(&opDiffStats, new(getRawDiffRequest), http.MethodGet) + _ = reflector.SetJSONResponse(&opDiffStats, new(types.DiffStats), http.StatusOK) + _ = reflector.SetJSONResponse(&opDiffStats, new(usererror.Error), http.StatusInternalServerError) + _ = reflector.SetJSONResponse(&opDiffStats, new(usererror.Error), http.StatusUnauthorized) + _ = reflector.SetJSONResponse(&opDiffStats, new(usererror.Error), http.StatusForbidden) + _ = reflector.Spec.AddOperation(http.MethodGet, "/repos/{repo_ref}/diff-stats/{range}", opDiffStats) + opMergeCheck := openapi3.Operation{} opMergeCheck.WithTags("repository") opMergeCheck.WithMapOfAnything(map[string]interface{}{"operationId": "mergeCheck"}) diff --git a/internal/router/api.go b/internal/router/api.go index 0e6f130d8..bb452f1a7 100644 --- a/internal/router/api.go +++ b/internal/router/api.go @@ -271,6 +271,9 @@ func setupRepos(r chi.Router, r.Route("/diff", func(r chi.Router) { r.Get("/*", handlerrepo.HandleDiff(repoCtrl)) }) + r.Route("/diff-stats", func(r chi.Router) { + r.Get("/*", handlerrepo.HandleDiffStats(repoCtrl)) + }) r.Route("/merge-check", func(r chi.Router) { r.Post("/*", handlerrepo.HandleMergeCheck(repoCtrl)) }) diff --git a/types/git.go b/types/git.go index c9588858b..3b5fd2bb7 100644 --- a/types/git.go +++ b/types/git.go @@ -74,4 +74,5 @@ type RenameDetails struct { type ListCommitResponse struct { Commits []Commit `json:"commits"` RenameDetails []RenameDetails `json:"rename_details"` + TotalCommits int `json:"total_commits,omitempty"` } diff --git a/web/src/components/Changes/Changes.tsx b/web/src/components/Changes/Changes.tsx index 1f6d08ff7..2a7c14bb8 100644 --- a/web/src/components/Changes/Changes.tsx +++ b/web/src/components/Changes/Changes.tsx @@ -76,11 +76,13 @@ export const Changes: React.FC = ({ const [commitRange, setCommitRange] = useState(defaultCommitRange || []) const { routes } = useAppContext() - const { data: prCommits } = useGet({ - path: `/api/v1/repos/${repoMetadata?.path}/+/pullreq/${pullRequestMetadata?.number}/commits`, + const { data: prCommits } = useGet<{ + commits: TypesCommit[] + }>({ + path: `/api/v1/repos/${repoMetadata?.path}/+/commits`, queryParams: { - git_ref: pullRequestMetadata?.source_branch, - after: pullRequestMetadata?.target_branch + git_ref: sourceRef, + after: targetRef }, lazy: !pullRequestMetadata?.number }) @@ -118,8 +120,6 @@ export const Changes: React.FC = ({ path: `/api/v1/repos/${repoMetadata?.path}/+/diff/${ commitRangePath ? commitRangePath - : pullRequestMetadata - ? `${pullRequestMetadata.merge_base_sha}...${pullRequestMetadata.source_sha}` : `${targetRef}...${sourceRef}` }`, requestOptions: { @@ -233,7 +233,7 @@ export const Changes: React.FC = ({ @@ -309,8 +309,8 @@ export const Changes: React.FC = ({ repoMetadata={repoMetadata} pullRequestMetadata={pullRequestMetadata} onCommentUpdate={onCommentUpdate} - targetRef={pullRequestMetadata ? pullRequestMetadata.merge_base_sha : targetRef} - sourceRef={pullRequestMetadata ? pullRequestMetadata.source_sha : sourceRef} + targetRef={targetRef} + sourceRef={sourceRef} /> ))} diff --git a/web/src/components/TabContentWrapper/TabContentWrapper.tsx b/web/src/components/TabContentWrapper/TabContentWrapper.tsx new file mode 100644 index 000000000..5f966c7cd --- /dev/null +++ b/web/src/components/TabContentWrapper/TabContentWrapper.tsx @@ -0,0 +1,27 @@ +import React from 'react' +import { Container, PageError } from '@harness/uicore' +import { getErrorMessage } from 'utils/Utils' +import { LoadingSpinner } from 'components/LoadingSpinner/LoadingSpinner' + +interface TabContentWrapperProps { + className?: string + loading?: boolean + error?: Unknown + onRetry: () => void +} + +export const TabContentWrapper: React.FC = ({ + className, + loading, + error, + onRetry, + children +}) => { + return ( + + + {error && } + {!error && children} + + ) +} \ No newline at end of file diff --git a/web/src/pages/Compare/Compare.tsx b/web/src/pages/Compare/Compare.tsx index 0f0379057..daa38c9bb 100644 --- a/web/src/pages/Compare/Compare.tsx +++ b/web/src/pages/Compare/Compare.tsx @@ -1,5 +1,5 @@ import { noop } from 'lodash-es' -import React, { useCallback, useEffect, useMemo, useState } from 'react' +import React, { useCallback, useState } from 'react' import { Container, PageBody, @@ -20,19 +20,17 @@ import { useAppContext } from 'AppContext' import { useGetRepositoryMetadata } from 'hooks/useGetRepositoryMetadata' import { useStrings } from 'framework/strings' import { RepositoryPageHeader } from 'components/RepositoryPageHeader/RepositoryPageHeader' -import { voidFn, getErrorMessage, LIST_FETCHING_LIMIT, showToaster } from 'utils/Utils' +import { getErrorMessage, showToaster } from 'utils/Utils' import { Images } from 'images' import { CodeIcon, isRefATag, makeDiffRefs } from 'utils/GitUtils' -import { CommitsView } from 'components/CommitsView/CommitsView' import { Changes } from 'components/Changes/Changes' -import type { OpenapiCreatePullReqRequest, TypesCommit, TypesPullReq } from 'services/code' +import type { OpenapiCreatePullReqRequest, TypesDiffStats, TypesPullReq, TypesRepository } from 'services/code' import { LoadingSpinner } from 'components/LoadingSpinner/LoadingSpinner' -import { usePageIndex } from 'hooks/usePageIndex' -import { ResourceListingPagination } from 'components/ResourceListingPagination/ResourceListingPagination' import { TabTitleWithCount, tabContainerCSS } from 'components/TabTitleWithCount/TabTitleWithCount' -import type { DiffFileEntry } from 'utils/types' import { MarkdownEditorWithPreview } from 'components/MarkdownEditorWithPreview/MarkdownEditorWithPreview' +import { TabContentWrapper } from 'components/TabContentWrapper/TabContentWrapper' import { CompareContentHeader, PRCreationType } from './CompareContentHeader/CompareContentHeader' +import { CompareCommits } from './CompareCommits' import css from './Compare.module.scss' export default function Compare() { @@ -42,29 +40,16 @@ export default function Compare() { const { repoMetadata, error, loading, diffRefs } = useGetRepositoryMetadata() const [sourceGitRef, setSourceGitRef] = useState(diffRefs.sourceGitRef) const [targetGitRef, setTargetGitRef] = useState(diffRefs.targetGitRef) - const [page, setPage] = usePageIndex() - const limit = LIST_FETCHING_LIMIT - const { - data: commits, - error: commitsError, - refetch, - response - } = useGet<{ - commits: TypesCommit[] - }>({ - path: `/api/v1/repos/${repoMetadata?.path}/+/commits`, - queryParams: { - limit, - page, - git_ref: sourceGitRef, - after: targetGitRef - }, - lazy: !repoMetadata - }) const [title, setTitle] = useState('') const [description, setDescription] = useState('') - const [diffs, setDiffs] = useState([]) const { showError } = useToaster() + const { + data, + error: errorStats + } = useGet({ + path: `/api/v1/repos/${repoMetadata?.path}/+/diff-stats/${targetGitRef}...${sourceGitRef}`, + lazy: !repoMetadata + }) const { mutate: createPullRequest, loading: creatingInProgress } = useMutate({ verb: 'POST', path: `/api/v1/repos/${repoMetadata?.path}/+/pullreq` @@ -144,30 +129,6 @@ export default function Compare() { repoMetadata ] ) - const ChangesTab = useMemo(() => { - if (repoMetadata) { - return ( - - - - ) - } - }, [repoMetadata, sourceGitRef, targetGitRef, getString]) - - useEffect(() => { - if (commits?.commits?.length) { - setTitle(commits.commits[0].title as string) - } - }, [commits?.commits]) return ( @@ -176,7 +137,7 @@ export default function Compare() { title={getString('comparingChanges')} dataTooltipId="comparingChanges" /> - + {repoMetadata && ( @@ -219,7 +180,6 @@ export default function Compare() { id="prComparing" defaultSelectedTabId="general" large={false} - onChange={() => setPage(1)} tabList={[ { id: 'general', @@ -259,8 +219,6 @@ export default function Compare() { - {/** Fake rendering Changes Tab to get changes count - no API has it now */} - {ChangesTab} ) }, @@ -270,21 +228,17 @@ export default function Compare() { ), - panel: ( - - - - - ) + panel: + {}} // TODO: when to refresh + /> }, { id: 'diff', @@ -292,11 +246,22 @@ export default function Compare() { ), - panel: ChangesTab + panel: + {}} className={css.changesContainer}> + + } ]} /> diff --git a/web/src/pages/Compare/CompareCommits.tsx b/web/src/pages/Compare/CompareCommits.tsx new file mode 100644 index 000000000..71bcd4641 --- /dev/null +++ b/web/src/pages/Compare/CompareCommits.tsx @@ -0,0 +1,58 @@ +import React from 'react' +import { useGet } from 'restful-react' +import type { TypesCommit } from 'services/code' +import type { GitInfoProps } from 'utils/GitUtils' +import { voidFn, LIST_FETCHING_LIMIT } from 'utils/Utils' +import { usePageIndex } from 'hooks/usePageIndex' +import { useStrings } from 'framework/strings' +import { ResourceListingPagination } from 'components/ResourceListingPagination/ResourceListingPagination' +import { CommitsView } from 'components/CommitsView/CommitsView' +import { TabContentWrapper } from 'components/TabContentWrapper/TabContentWrapper' + +interface CommitProps extends Pick { + sourceSha? :string + targetSha? :string + handleRefresh: () => void +} + +export const CompareCommits: React.FC = ({ + repoMetadata, + sourceSha, + targetSha, + handleRefresh +}) => { + const limit = LIST_FETCHING_LIMIT + const [page, setPage] = usePageIndex() + const { getString } = useStrings() + const { + data, + error, + loading, + refetch, + response + } = useGet<{ + commits: TypesCommit[] + }>({ + path: `/api/v1/repos/${repoMetadata?.path}/+/commits`, + queryParams: { + limit, + page, + git_ref: sourceSha, + after: targetSha + }, + lazy: !repoMetadata + }) + + return ( + + + + + ) +} diff --git a/web/src/pages/PullRequest/PullRequest.tsx b/web/src/pages/PullRequest/PullRequest.tsx index e66394edd..240d60243 100644 --- a/web/src/pages/PullRequest/PullRequest.tsx +++ b/web/src/pages/PullRequest/PullRequest.tsx @@ -226,8 +226,8 @@ export default function PullRequest() { repoMetadata={repoMetadata as TypesRepository} pullRequestMetadata={prData as TypesPullReq} defaultCommitRange={compact(commitSHA?.split(/~1\.\.\.|\.\.\./g))} - targetRef={prData?.target_branch} - sourceRef={prData?.source_branch} + targetRef={prData?.merge_base_sha} + sourceRef={prData?.source_sha} emptyTitle={getString('noChanges')} emptyMessage={getString('noChangesPR')} onCommentUpdate={voidFn(refetchPullRequest)} diff --git a/web/src/pages/PullRequest/PullRequestCommits/PullRequestCommits.tsx b/web/src/pages/PullRequest/PullRequestCommits/PullRequestCommits.tsx index 413898a1e..0aecf1321 100644 --- a/web/src/pages/PullRequest/PullRequestCommits/PullRequestCommits.tsx +++ b/web/src/pages/PullRequest/PullRequestCommits/PullRequestCommits.tsx @@ -24,18 +24,20 @@ export const PullRequestCommits: React.FC = ({ const [page, setPage] = usePageIndex() const { getString } = useStrings() const { - data: commits, + data, error, loading, refetch, response - } = useGet({ - path: `/api/v1/repos/${repoMetadata?.path}/+/pullreq/${pullRequestMetadata.number}/commits`, + } = useGet<{ + commits: TypesCommit[] + }>({ + path: `/api/v1/repos/${repoMetadata?.path}/+/commits`, queryParams: { limit, page, - git_ref: pullRequestMetadata.source_branch, - after: pullRequestMetadata.target_branch + git_ref: pullRequestMetadata.source_sha, + after: pullRequestMetadata.merge_base_sha }, lazy: !repoMetadata }) @@ -43,7 +45,7 @@ export const PullRequestCommits: React.FC = ({ return (