From df22f8da5f051548843403f0b413e90d08c9ff28 Mon Sep 17 00:00:00 2001
From: Solomon Victorino <git@solomonvictorino.com>
Date: Tue, 16 Jul 2024 14:42:35 +0000
Subject: [PATCH] fix: preserve object format dropdown options on /repo/create
error (#4360)
To reproduce:
- make the repo creation form return with an error, like a duplicate name
- click on the Object format dropdown
- the options are missing as the listbox is empty
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4360
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>
Co-authored-by: Solomon Victorino <git@solomonvictorino.com>
Co-committed-by: Solomon Victorino <git@solomonvictorino.com>
---
routers/web/repo/repo.go | 2 +
templates/repo/create.tmpl | 2 +-
tests/integration/html_helper.go | 32 +++++++++++
tests/integration/repo_generate_test.go | 74 ++++++++++++++++++-------
4 files changed, 89 insertions(+), 21 deletions(-)
diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go
index 4d17be3758..711a1e1e12 100644
--- a/routers/web/repo/repo.go
+++ b/routers/web/repo/repo.go
@@ -231,6 +231,8 @@ func CreatePost(ctx *context.Context) {
ctx.Data["CanCreateRepo"] = ctx.Doer.CanCreateRepo()
ctx.Data["MaxCreationLimit"] = ctx.Doer.MaxCreationLimit()
+ ctx.Data["SupportedObjectFormats"] = git.SupportedObjectFormats
+ ctx.Data["DefaultObjectFormat"] = git.Sha1ObjectFormat
ctxUser := checkContextUser(ctx, form.UID)
if ctx.Written() {
diff --git a/templates/repo/create.tmpl b/templates/repo/create.tmpl
index 3afd978ee9..d3a11e7ed5 100644
--- a/templates/repo/create.tmpl
+++ b/templates/repo/create.tmpl
@@ -69,7 +69,7 @@
<div class="inline field">
<label>{{ctx.Locale.Tr "repo.template"}}</label>
<div id="repo_template_search" class="ui search selection dropdown">
- <input type="hidden" id="repo_template" name="repo_template" value="{{.repo_template}}">
+ <input type="hidden" id="repo_template" name="repo_template" value="{{if ne .repo_template 0}}{{.repo_template}}{{end}}">
<div class="default text">{{.repo_template_name}}</div>
<div class="menu">
</div>
diff --git a/tests/integration/html_helper.go b/tests/integration/html_helper.go
index 933bb51cf8..6c7c8e7467 100644
--- a/tests/integration/html_helper.go
+++ b/tests/integration/html_helper.go
@@ -5,6 +5,7 @@ package integration
import (
"bytes"
+ "fmt"
"testing"
"github.com/PuerkitoBio/goquery"
@@ -36,6 +37,37 @@ func (doc *HTMLDoc) GetInputValueByName(name string) string {
return text
}
+func (doc *HTMLDoc) AssertDropdown(t testing.TB, name string) *goquery.Selection {
+ t.Helper()
+
+ dropdownGroup := doc.Find(fmt.Sprintf(".dropdown:has(input[name='%s'])", name))
+ assert.Equal(t, dropdownGroup.Length(), 1, fmt.Sprintf("%s dropdown does not exist", name))
+ return dropdownGroup
+}
+
+// Assert that a dropdown has at least one non-empty option
+func (doc *HTMLDoc) AssertDropdownHasOptions(t testing.TB, dropdownName string) {
+ t.Helper()
+
+ options := doc.AssertDropdown(t, dropdownName).Find(".menu [data-value]:not([data-value=''])")
+ assert.Greater(t, options.Length(), 0, fmt.Sprintf("%s dropdown has no options", dropdownName))
+}
+
+func (doc *HTMLDoc) AssertDropdownHasSelectedOption(t testing.TB, dropdownName, expectedValue string) {
+ t.Helper()
+
+ dropdownGroup := doc.AssertDropdown(t, dropdownName)
+
+ selectedValue, _ := dropdownGroup.Find(fmt.Sprintf("input[name='%s']", dropdownName)).Attr("value")
+ assert.Equal(t, expectedValue, selectedValue, fmt.Sprintf("%s dropdown doesn't have expected value selected", dropdownName))
+
+ dropdownValues := dropdownGroup.Find(".menu [data-value]").Map(func(i int, s *goquery.Selection) string {
+ value, _ := s.Attr("data-value")
+ return value
+ })
+ assert.Contains(t, dropdownValues, expectedValue, fmt.Sprintf("%s dropdown doesn't have an option with expected value", dropdownName))
+}
+
// Find gets the descendants of each element in the current set of
// matched elements, filtered by a selector. It returns a new Selection
// object containing these matched elements.
diff --git a/tests/integration/repo_generate_test.go b/tests/integration/repo_generate_test.go
index 961255cedf..2cd2002b51 100644
--- a/tests/integration/repo_generate_test.go
+++ b/tests/integration/repo_generate_test.go
@@ -6,7 +6,7 @@ package integration
import (
"fmt"
"net/http"
- "net/http/httptest"
+ "strconv"
"strings"
"testing"
@@ -18,11 +18,23 @@ import (
"github.com/stretchr/testify/assert"
)
-func testRepoGenerate(t *testing.T, session *TestSession, templateID, templateOwnerName, templateRepoName, generateOwnerName, generateRepoName string) *httptest.ResponseRecorder {
- generateOwner := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: generateOwnerName})
+func assertRepoCreateForm(t *testing.T, htmlDoc *HTMLDoc, owner *user_model.User, templateID string) {
+ _, exists := htmlDoc.doc.Find("form.ui.form[action^='/repo/create']").Attr("action")
+ assert.True(t, exists, "Expected the repo creation form")
+ htmlDoc.AssertDropdownHasSelectedOption(t, "uid", strconv.FormatInt(owner.ID, 10))
+
+ // the template menu is loaded client-side, so don't assert the option exists
+ assert.Equal(t, templateID, htmlDoc.GetInputValueByName("repo_template"), "Unexpected repo_template selection")
+
+ for _, name := range []string{"issue_labels", "gitignores", "license", "readme", "object_format_name"} {
+ htmlDoc.AssertDropdownHasOptions(t, name)
+ }
+}
+
+func testRepoGenerate(t *testing.T, session *TestSession, templateID, templateOwnerName, templateRepoName string, user, generateOwner *user_model.User, generateRepoName string) {
// Step0: check the existence of the generated repo
- req := NewRequestf(t, "GET", "/%s/%s", generateOwnerName, generateRepoName)
+ req := NewRequestf(t, "GET", "/%s/%s", generateOwner.Name, generateRepoName)
session.MakeRequest(t, req, http.StatusNotFound)
// Step1: go to the main page of template repo
@@ -36,12 +48,9 @@ func testRepoGenerate(t *testing.T, session *TestSession, templateID, templateOw
req = NewRequest(t, "GET", link)
resp = session.MakeRequest(t, req, http.StatusOK)
- // Step3: fill the form of the create
+ // Step3: test and submit form
htmlDoc = NewHTMLParser(t, resp.Body)
- link, exists = htmlDoc.doc.Find("form.ui.form[action^=\"/repo/create\"]").Attr("action")
- assert.True(t, exists, "The template has changed")
- _, exists = htmlDoc.doc.Find(fmt.Sprintf(".owner.dropdown .item[data-value=\"%d\"]", generateOwner.ID)).Attr("data-value")
- assert.True(t, exists, fmt.Sprintf("Generate owner '%s' is not present in select box", generateOwnerName))
+ assertRepoCreateForm(t, htmlDoc, user, templateID)
req = NewRequestWithValues(t, "POST", link, map[string]string{
"_csrf": htmlDoc.GetCSRF(),
"uid": fmt.Sprintf("%d", generateOwner.ID),
@@ -52,41 +61,66 @@ func testRepoGenerate(t *testing.T, session *TestSession, templateID, templateOw
session.MakeRequest(t, req, http.StatusSeeOther)
// Step4: check the existence of the generated repo
- req = NewRequestf(t, "GET", "/%s/%s", generateOwnerName, generateRepoName)
+ req = NewRequestf(t, "GET", "/%s/%s", generateOwner.Name, generateRepoName)
session.MakeRequest(t, req, http.StatusOK)
// Step5: check substituted values in Readme
- req = NewRequestf(t, "GET", "/%s/%s/raw/branch/master/README.md", generateOwnerName, generateRepoName)
+ req = NewRequestf(t, "GET", "/%s/%s/raw/branch/master/README.md", generateOwner.Name, generateRepoName)
resp = session.MakeRequest(t, req, http.StatusOK)
body := fmt.Sprintf(`# %s Readme
Owner: %s
Link: /%s/%s
Clone URL: %s%s/%s.git`,
generateRepoName,
- strings.ToUpper(generateOwnerName),
- generateOwnerName,
+ strings.ToUpper(generateOwner.Name),
+ generateOwner.Name,
generateRepoName,
setting.AppURL,
- generateOwnerName,
+ generateOwner.Name,
generateRepoName)
assert.Equal(t, body, resp.Body.String())
// Step6: check substituted values in substituted file path ${REPO_NAME}
- req = NewRequestf(t, "GET", "/%s/%s/raw/branch/master/%s.log", generateOwnerName, generateRepoName, generateRepoName)
+ req = NewRequestf(t, "GET", "/%s/%s/raw/branch/master/%s.log", generateOwner.Name, generateRepoName, generateRepoName)
resp = session.MakeRequest(t, req, http.StatusOK)
assert.Equal(t, generateRepoName, resp.Body.String())
+}
- return resp
+// test form elements before and after POST error response
+func TestRepoCreateForm(t *testing.T) {
+ defer tests.PrepareTestEnv(t)()
+ userName := "user1"
+ session := loginUser(t, userName)
+ user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: userName})
+
+ req := NewRequest(t, "GET", "/repo/create")
+ resp := session.MakeRequest(t, req, http.StatusOK)
+ htmlDoc := NewHTMLParser(t, resp.Body)
+ assertRepoCreateForm(t, htmlDoc, user, "")
+
+ req = NewRequestWithValues(t, "POST", "/repo/create", map[string]string{
+ "_csrf": htmlDoc.GetCSRF(),
+ })
+ resp = session.MakeRequest(t, req, http.StatusOK)
+ htmlDoc = NewHTMLParser(t, resp.Body)
+ assertRepoCreateForm(t, htmlDoc, user, "")
}
func TestRepoGenerate(t *testing.T) {
defer tests.PrepareTestEnv(t)()
- session := loginUser(t, "user1")
- testRepoGenerate(t, session, "44", "user27", "template1", "user1", "generated1")
+ userName := "user1"
+ session := loginUser(t, userName)
+ user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: userName})
+
+ testRepoGenerate(t, session, "44", "user27", "template1", user, user, "generated1")
}
func TestRepoGenerateToOrg(t *testing.T) {
defer tests.PrepareTestEnv(t)()
- session := loginUser(t, "user2")
- testRepoGenerate(t, session, "44", "user27", "template1", "user2", "generated2")
+ userName := "user2"
+ session := loginUser(t, userName)
+ user := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: userName})
+ org := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "org3"})
+
+ testRepoGenerate(t, session, "44", "user27", "template1", user, org, "generated2")
}