fix: resolve tech-debt batch 2 — dropdown validation, editor cleanup, rename

- fix(templates): add required validation to issue dropdown fields
- refactor(ts): remove redundant `handled` field from MarkdownHandleIndentionResult
- refactor(go): rename HasOrgOrUserVisible to IsOwnerVisibleToDoer for clarity

Refs #311, #334

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

models/organization/org.go

@@ -417,8 +417,8 @@ func GetUsersWhoCanCreateOrgRepo(ctx context.Context, orgID int64) (map[int64]*u
 		And("team_user.org_id = ?", orgID).Find(&users)
 }
 
-// HasOrgOrUserVisible tells if the given user can see the given org or user
-func HasOrgOrUserVisible(ctx context.Context, orgOrUser, user *user_model.User) bool {
+// IsOwnerVisibleToDoer tells if the given user can see the given org or user
+func IsOwnerVisibleToDoer(ctx context.Context, orgOrUser, user *user_model.User) bool {
 	// If user is nil, it's an anonymous user/request.
 	// The Ghost user is handled like an anonymous user.
 	if user == nil || user.IsGhost() {
@@ -446,7 +446,7 @@ func HasOrgsVisible(ctx context.Context, orgs []*Organization, user *user_model.
 	}
 
 	for _, org := range orgs {
-		if HasOrgOrUserVisible(ctx, org.AsUser(), user) {
+		if IsOwnerVisibleToDoer(ctx, org.AsUser(), user) {
 			return true
 		}
 	}

models/organization/org_test.go

@@ -378,18 +378,18 @@ func TestHasOrgVisibleTypePublic(t *testing.T) {
 	assert.NoError(t, organization.CreateOrganization(t.Context(), org, owner))
 	org = unittest.AssertExistsAndLoadBean(t,
 		&organization.Organization{Name: org.Name, Type: user_model.UserTypeOrganization})
-	test1 := organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), owner)
-	test2 := organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), org3)
-	test3 := organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), nil)
+	test1 := organization.IsOwnerVisibleToDoer(t.Context(), org.AsUser(), owner)
+	test2 := organization.IsOwnerVisibleToDoer(t.Context(), org.AsUser(), org3)
+	test3 := organization.IsOwnerVisibleToDoer(t.Context(), org.AsUser(), nil)
 	assert.True(t, test1) // owner of org
 	assert.True(t, test2) // user not a part of org
 	assert.True(t, test3) // logged out user
 
 	restrictedUser := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 29, IsRestricted: true})
 	require.True(t, restrictedUser.IsRestricted)
-	assert.True(t, organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), restrictedUser))
+	assert.True(t, organization.IsOwnerVisibleToDoer(t.Context(), org.AsUser(), restrictedUser))
 	defer test.MockVariableValue(&setting.Service.RequireSignInViewStrict, true)()
-	assert.False(t, organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), restrictedUser))
+	assert.False(t, organization.IsOwnerVisibleToDoer(t.Context(), org.AsUser(), restrictedUser))
 }
 
 func TestHasOrgVisibleTypeLimited(t *testing.T) {
@@ -407,9 +407,9 @@ func TestHasOrgVisibleTypeLimited(t *testing.T) {
 	assert.NoError(t, organization.CreateOrganization(t.Context(), org, owner))
 	org = unittest.AssertExistsAndLoadBean(t,
 		&organization.Organization{Name: org.Name, Type: user_model.UserTypeOrganization})
-	test1 := organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), owner)
-	test2 := organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), org3)
-	test3 := organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), nil)
+	test1 := organization.IsOwnerVisibleToDoer(t.Context(), org.AsUser(), owner)
+	test2 := organization.IsOwnerVisibleToDoer(t.Context(), org.AsUser(), org3)
+	test3 := organization.IsOwnerVisibleToDoer(t.Context(), org.AsUser(), nil)
 	assert.True(t, test1)  // owner of org
 	assert.True(t, test2)  // user not a part of org
 	assert.False(t, test3) // logged out user
@@ -430,9 +430,9 @@ func TestHasOrgVisibleTypePrivate(t *testing.T) {
 	assert.NoError(t, organization.CreateOrganization(t.Context(), org, owner))
 	org = unittest.AssertExistsAndLoadBean(t,
 		&organization.Organization{Name: org.Name, Type: user_model.UserTypeOrganization})
-	test1 := organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), owner)
-	test2 := organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), org3)
-	test3 := organization.HasOrgOrUserVisible(t.Context(), org.AsUser(), nil)
+	test1 := organization.IsOwnerVisibleToDoer(t.Context(), org.AsUser(), owner)
+	test2 := organization.IsOwnerVisibleToDoer(t.Context(), org.AsUser(), org3)
+	test3 := organization.IsOwnerVisibleToDoer(t.Context(), org.AsUser(), nil)
 	assert.True(t, test1)  // owner of org
 	assert.False(t, test2) // user not a part of org
 	assert.False(t, test3) // logged out user

models/perm/access/repo_permission.go

@@ -427,8 +427,7 @@ func GetIndividualUserRepoPermission(ctx context.Context, repo *repo_model.Repos
 
 	// Prevent strangers from checking out public repo of private organization/users
 	// Allow user if they are a collaborator of a repo within a private user or a private organization but not a member of the organization itself
-	// TODO: rename it to "IsOwnerVisibleToDoer"
-	if !organization.HasOrgOrUserVisible(ctx, repo.Owner, user) && !isCollaborator {
+	if !organization.IsOwnerVisibleToDoer(ctx, repo.Owner, user) && !isCollaborator {
 		perm.AccessMode = perm_model.AccessModeNone
 		return perm, nil
 	}

routers/api/v1/org/org.go

@@ -145,8 +145,8 @@ func GetUserOrgsPermissions(ctx *context.APIContext) {
 
 	op := api.OrganizationPermissions{}
 
-	if !organization.HasOrgOrUserVisible(ctx, o, ctx.Doer) {
-		ctx.APIErrorNotFound("HasOrgOrUserVisible", nil)
+	if !organization.IsOwnerVisibleToDoer(ctx, o, ctx.Doer) {
+		ctx.APIErrorNotFound("IsOwnerVisibleToDoer", nil)
 		return
 	}
 
@@ -311,8 +311,8 @@ func Get(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	if !organization.HasOrgOrUserVisible(ctx, ctx.Org.Organization.AsUser(), ctx.Doer) {
-		ctx.APIErrorNotFound("HasOrgOrUserVisible", nil)
+	if !organization.IsOwnerVisibleToDoer(ctx, ctx.Org.Organization.AsUser(), ctx.Doer) {
+		ctx.APIErrorNotFound("IsOwnerVisibleToDoer", nil)
 		return
 	}
 

routers/api/v1/repo/fork.go

@@ -93,7 +93,7 @@ func prepareDoerCreateRepoInOrg(ctx *context.APIContext, orgName string) *organi
 		return nil
 	}
 
-	if !organization.HasOrgOrUserVisible(ctx, org.AsUser(), ctx.Doer) {
+	if !organization.IsOwnerVisibleToDoer(ctx, org.AsUser(), ctx.Doer) {
 		ctx.APIErrorNotFound()
 		return nil
 	}

services/context/package.go

@@ -149,7 +149,7 @@ func determineAccessMode(ctx *Base, pkgOwner, doer *user_model.User) (perm.Acces
 				}
 			}
 		}
-		if accessMode == perm.AccessModeNone && organization.HasOrgOrUserVisible(ctx, pkgOwner, doer) {
+		if accessMode == perm.AccessModeNone && organization.IsOwnerVisibleToDoer(ctx, pkgOwner, doer) {
 			// 2. If user is unauthorized or no org member, check if org is visible
 			accessMode = perm.AccessModeRead
 		}

templates/repo/issue/fields/dropdown.tmpl

@@ -1,8 +1,7 @@
 <div class="field {{if not .item.VisibleOnForm}}tw-hidden{{end}}">
 	{{template "repo/issue/fields/header" .}}
-	{{/* FIXME: required validation */}}
 	<div class="ui fluid selection dropdown {{if .item.Attributes.multiple}}multiple clearable{{end}}">
-		<input type="hidden" name="form-field-{{.item.ID}}" value="{{.item.Attributes.default}}">
+		<input type="hidden" name="form-field-{{.item.ID}}" value="{{.item.Attributes.default}}" {{if .item.Validations.required}}required{{end}}>
 		{{svg "octicon-triangle-down" 14 "dropdown icon"}}
 		{{if not .item.Validations.required}}
 		{{svg "octicon-x" 14 "remove icon"}}

web_src/js/features/comp/EditorMarkdown.test.ts

@@ -29,12 +29,11 @@ test('markdownHandleIndention', () => {
     input = input.replaceAll('|', '');
     const ret = markdownHandleIndention({value: input, selStart: inputPos, selEnd: inputPos});
     if (expected === null) {
-      expect(ret).toEqual({handled: false});
+      expect(ret).toEqual({});
     } else {
       const expectedPos = expected.indexOf('|');
       expected = expected.replaceAll('|', '');
       expect(ret).toEqual({
-        handled: true,
         valueSelection: {value: expected, selStart: expectedPos, selEnd: expectedPos},
       });
     }

web_src/js/features/comp/EditorMarkdown.ts

@@ -73,7 +73,6 @@ function handleIndentSelection(textarea: HTMLTextAreaElement, e: KeyboardEvent)
 }
 
 type MarkdownHandleIndentionResult = {
-  handled: boolean;
   valueSelection?: TextareaValueSelection;
 };
 
@@ -135,7 +134,7 @@ function recalculateLengthBeforeLine(linesBuf: TextLinesBuffer) {
 }
 
 export function markdownHandleIndention(tvs: TextareaValueSelection): MarkdownHandleIndentionResult {
-  const unhandled: MarkdownHandleIndentionResult = {handled: false};
+  const unhandled: MarkdownHandleIndentionResult = {};
   if (tvs.selEnd !== tvs.selStart) return unhandled; // do not process when there is a selection
 
   const linesBuf = textareaSplitLines(tvs.value, tvs.selStart);
@@ -182,13 +181,13 @@ export function markdownHandleIndention(tvs: TextareaValueSelection): MarkdownHa
 
   markdownReformatListNumbers(linesBuf, indention);
   const newPos = linesBuf.lengthBeforePosLine + linesBuf.inlinePos;
-  return {handled: true, valueSelection: {value: linesBuf.lines.join('\n'), selStart: newPos, selEnd: newPos}};
+  return {valueSelection: {value: linesBuf.lines.join('\n'), selStart: newPos, selEnd: newPos}};
 }
 
 function handleNewline(textarea: HTMLTextAreaElement, e: KeyboardEvent) {
   if (e.isComposing) return;
   const ret = markdownHandleIndention({value: textarea.value, selStart: textarea.selectionStart, selEnd: textarea.selectionEnd});
-  if (!ret.handled || !ret.valueSelection) return; // FIXME: the "handled" seems redundant, only valueSelection is enough (null for unhandled)
+  if (!ret.valueSelection) return;
   e.preventDefault();
   textarea.value = ret.valueSelection.value;
   textarea.setSelectionRange(ret.valueSelection.selStart, ret.valueSelection.selEnd);