diff --git a/appview/db/repos.go b/appview/db/repos.go index c7e58187..1fe23d89 100644 --- a/appview/db/repos.go +++ b/appview/db/repos.go @@ -636,60 +636,14 @@ func GetForksByDid(e Execer, did string) ([]models.Repo, error) { return repos, nil } -func GetForkByDid(e Execer, did string, rkey string) (*models.Repo, error) { - var repo models.Repo - var createdAt string - var nullableDescription sql.NullString - var nullableWebsite sql.NullString - var nullableTopicStr sql.NullString - var nullableSource sql.NullString - var nullableRepoDid sql.NullString - - row := e.QueryRow( - `select id, did, name, knot, rkey, description, website, topics, created, source, repo_did - from repos - where did = ? and rkey = ? and source is not null and source != ''`, - did, rkey, - ) - - err := row.Scan(&repo.Id, &repo.Did, &repo.Name, &repo.Knot, &repo.Rkey, &nullableDescription, &nullableWebsite, &nullableTopicStr, &createdAt, &nullableSource, &nullableRepoDid) - if err != nil { - return nil, err - } - - if nullableDescription.Valid { - repo.Description = nullableDescription.String - } - - if nullableWebsite.Valid { - repo.Website = nullableWebsite.String - } - - if nullableTopicStr.Valid { - repo.Topics = strings.Fields(nullableTopicStr.String) - } - - if nullableSource.Valid { - repo.Source = nullableSource.String - } - if nullableRepoDid.Valid { - repo.RepoDid = nullableRepoDid.String - } - - createdAtTime, err := time.Parse(time.RFC3339, createdAt) - if err != nil { - repo.Created = time.Now() - } else { - repo.Created = createdAtTime - } - - return &repo, nil -} - func GetRepoByDid(e Execer, repoDid string) (*models.Repo, error) { return GetRepo(e, orm.FilterEq("repo_did", repoDid)) } +func GetForkByRepoDid(e Execer, repoDid string) (*models.Repo, error) { + return GetRepo(e, orm.FilterEq("repo_did", repoDid), orm.FilterNotEq("source", "")) +} + // TODO: just queue every legacy records regardless of target repo has a DID or not. // doable after we have `repo_did` column in db for each tables. func EnqueuePdsRewritesForRepo(tx *sql.Tx, repoDid, repoAtUri string) error { diff --git a/appview/pages/templates/repo/pulls/fragments/pullCompareForks.html b/appview/pages/templates/repo/pulls/fragments/pullCompareForks.html index df98f4b1..205b5386 100644 --- a/appview/pages/templates/repo/pulls/fragments/pullCompareForks.html +++ b/appview/pages/templates/repo/pulls/fragments/pullCompareForks.html @@ -15,8 +15,7 @@ > {{ range .Forks }} - {{ $ident := printf "%s/%s" .Did .Name }} - {{ end }} diff --git a/appview/pulls/compose.go b/appview/pulls/compose.go index c1362f63..518a2fed 100644 --- a/appview/pulls/compose.go +++ b/appview/pulls/compose.go @@ -2,6 +2,7 @@ package pulls import ( "context" + "database/sql" "encoding/json" "errors" "fmt" @@ -19,7 +20,6 @@ import ( "tangled.org/core/appview/pages/markup" "tangled.org/core/appview/pages/repoinfo" "tangled.org/core/appview/xrpcclient" - "tangled.org/core/orm" "tangled.org/core/patchutil" "tangled.org/core/types" @@ -222,6 +222,9 @@ func (s *Pulls) composeParams(r *http.Request, repo *models.Repo) (pages.RepoNew l.Warn("failed to list user forks", "err", err, "user", user.Did) } } + forks = slices.DeleteFunc(forks, func(f models.Repo) bool { + return f.RepoDid == "" + }) repoInfo := s.repoResolver.GetRepoInfo(r, user) source, ok := pages.ParseSource(r.FormValue("source")) @@ -238,7 +241,7 @@ func (s *Pulls) composeParams(r *http.Request, repo *models.Repo) (pages.RepoNew patch := r.FormValue("patch") if source == pages.SourceFork && fork == "" && len(forks) == 1 { - fork = fmt.Sprintf("%s/%s", forks[0].Did, forks[0].Name) + fork = forks[0].RepoDid } var forkBranches []types.Branch @@ -345,12 +348,14 @@ func (s *Pulls) listBranches(ctx context.Context, repo *models.Repo) ([]types.Br return result.Branches, nil } -func (s *Pulls) listForkBranches(ctx context.Context, forkIdent string) ([]types.Branch, error) { - parts := strings.SplitN(forkIdent, "/", 2) - if len(parts) != 2 { - return nil, fmt.Errorf("invalid fork identifier: %s", forkIdent) +func (s *Pulls) listForkBranches(ctx context.Context, forkRepoDid string) ([]types.Branch, error) { + if forkRepoDid == "" { + return nil, fmt.Errorf("fork not found") + } + forkRepo, err := db.GetForkByRepoDid(s.db, forkRepoDid) + if errors.Is(err, sql.ErrNoRows) { + return nil, fmt.Errorf("fork not found") } - forkRepo, err := db.GetRepo(s.db, orm.FilterEq("did", parts[0]), orm.FilterEq("name", parts[1])) if err != nil { return nil, err } @@ -551,12 +556,14 @@ func (s *Pulls) fetchBranchComparison(ctx context.Context, repo *models.Repo, ta return &comparison, nil } -func (s *Pulls) fetchForkComparison(r *http.Request, forkIdent, targetBranch, sourceBranch string) (*types.RepoFormatPatchResponse, error) { - parts := strings.SplitN(forkIdent, "/", 2) - if len(parts) != 2 { - return nil, fmt.Errorf("invalid fork identifier: %s", forkIdent) +func (s *Pulls) fetchForkComparison(r *http.Request, forkRepoDid, targetBranch, sourceBranch string) (*types.RepoFormatPatchResponse, error) { + if forkRepoDid == "" { + return nil, fmt.Errorf("fork not found") + } + fork, err := db.GetForkByRepoDid(s.db, forkRepoDid) + if errors.Is(err, sql.ErrNoRows) { + return nil, fmt.Errorf("fork not found") } - fork, err := db.GetForkByDid(s.db, parts[0], parts[1]) if err != nil { return nil, err } diff --git a/appview/pulls/compose_helpers_test.go b/appview/pulls/compose_helpers_test.go index 17c85211..cbf479e2 100644 --- a/appview/pulls/compose_helpers_test.go +++ b/appview/pulls/compose_helpers_test.go @@ -202,11 +202,11 @@ func TestComposeCanonicalURL(t *testing.T) { pages.RepoNewPullParams{ RepoInfo: repo, Source: pages.SourceFork, - Fork: "did:plc:other/repo", + Fork: "did:plc:limpet", SourceBranch: "feature", TargetBranch: "main", }, - "/did:plc:abc/demo/pulls/new?fork=did%3Aplc%3Aother%2Frepo&source=fork&sourceBranch=feature&targetBranch=main", + "/did:plc:abc/demo/pulls/new?fork=did%3Aplc%3Alimpet&source=fork&sourceBranch=feature&targetBranch=main", }, { "branch with selection drops source param", @@ -449,8 +449,8 @@ func TestPrefetchComparisonMissingInputs(t *testing.T) { {"branch missing target", pages.SourceBranch, "", "", "feature"}, {"branch missing source", pages.SourceBranch, "", "main", ""}, {"fork missing fork", pages.SourceFork, "", "main", "feature"}, - {"fork missing target", pages.SourceFork, "did/repo", "", "feature"}, - {"fork missing source", pages.SourceFork, "did/repo", "main", ""}, + {"fork missing target", pages.SourceFork, "did:plc:limpet", "", "feature"}, + {"fork missing source", pages.SourceFork, "did:plc:limpet", "main", ""}, {"unknown source", pages.Source("bogus"), "", "", ""}, } for _, c := range cases { diff --git a/appview/pulls/create.go b/appview/pulls/create.go index 1b10890d..da54669e 100644 --- a/appview/pulls/create.go +++ b/appview/pulls/create.go @@ -94,18 +94,19 @@ func (s *Pulls) handlePatchBasedPull(w http.ResponseWriter, r *http.Request, rep s.createPullRequest(w, r, repo, userDid, title, body, targetBranch, patch, "", "", nil, isStacked, stackTitles, stackBodies) } -func (s *Pulls) handleForkBasedPull(w http.ResponseWriter, r *http.Request, repo *models.Repo, userDid syntax.DID, forkRepo string, title, body, targetBranch, sourceBranch string, isStacked bool, stackTitles, stackBodies map[string]string) { - l := s.logger.With("handler", "handleForkBasedPull", "user", userDid, "fork_repo", forkRepo, "target_branch", targetBranch, "source_branch", sourceBranch, "is_stacked", isStacked) +func (s *Pulls) handleForkBasedPull(w http.ResponseWriter, r *http.Request, repo *models.Repo, userDid syntax.DID, forkRepoDid string, title, body, targetBranch, sourceBranch string, isStacked bool, stackTitles, stackBodies map[string]string) { + l := s.logger.With("handler", "handleForkBasedPull", "user", userDid, "fork_repo_did", forkRepoDid, "target_branch", targetBranch, "source_branch", sourceBranch, "is_stacked", isStacked) - repoString := strings.SplitN(forkRepo, "/", 2) - forkOwnerDid := repoString[0] - forkRkey := strings.ToLower(repoString[1]) - fork, err := db.GetForkByDid(s.db, forkOwnerDid, forkRkey) + if forkRepoDid == "" { + s.pages.Notice(w, "pull", "No such fork.") + return + } + fork, err := db.GetForkByRepoDid(s.db, forkRepoDid) if errors.Is(err, sql.ErrNoRows) { s.pages.Notice(w, "pull", "No such fork.") return } else if err != nil { - l.Error("failed to fetch fork", "err", err, "fork_owner_did", forkOwnerDid, "fork_rkey", forkRkey) + l.Error("failed to fetch fork", "err", err, "fork_repo_did", forkRepoDid) s.pages.Notice(w, "pull", "Failed to fetch fork.") return }