Manage User Badges in the UI #31262

HenriquerPimentel · 2024-06-05T14:08:21Z

Implemented #29798

This feature implements:

List all badges
Create new badges
View badge
Edit badge
Add user to badge

Implemented Badge Management in administration panel

Updated badge model (models/user/badge.go)
Added new type of errors ErrBadgeAlreadyExist and ErrBadgeNotExist (models/user/error.go)
Implemented badge service methods (services/user/badge.go)
Implemented new type of form AdminCreateBadgeForm (services/forms/admin.go)
Implemented new binding rules to check if ImageURL is valid and if is Slug Pattern (modules/validation/binding.go) (modules/web/middleware.go) (modules/validation/helpers.go)
Implemented badge search (routers/web/explore/badge.go) (routers/web/admin/badges.go)
Implemented badges routing (routers/web/web.go)
Implemented templates for create/list/edit/view badges (templates/admin/badge/*) (templates/admin/navbar.tmpl)
9 Added new translation phrases (en-US): search.badge_kind, form.ImageURL, form.invalid_image_url_error, form.slug_been_taken, admin.badges, admin.badges.badges_manage_panel, admin.badges.details, admin.badges.new_badge, admin.badges.slug, admin.badges.description, admin.badges.image_url, admin.badges.slug.must_fill, admin.badges.new_success, admin.badges.update_success, admin.badges.deletion_success, admin.badges.edit_badge, admin.badges.update_badge, admin.badges.delete_badge, admin.badges.delete_badge_desc

Implemented User Badge Management Interface

Implemented rule to verify if a user badge already exists before insert (models/user/badge.go).
Implemented interface to manage user badge, within administration panel. (routers/web/web.go) (routers/web/explore/badge.go) (routers/web/admin/badges.go) (templates/admin/badge/users.tmpl).
Small bug fixes (view.tmpl, list.tmpl)
Added new translation phrases: admin.badges.users_with_badge, admin.badges.add_user, admin.badges.remove_user, admin.badges.delete_user_desc, admin.badges.not_found, admin.badges.user_add_success, admin.badges.user_remove_success, admin.badges.manage_users.

techknowlogick

Thanks for the PR! The CI has a few linting reports that came up that I've reported in line :)

templates/admin/badge/users.tmpl

HenriquerPimentel · 2024-06-17T21:31:01Z

Thanks for the PR! The CI has a few linting reports that came up that I've reported in line :)

Thank you so much!

Co-authored-by: Diogo Vicente <diogo.m.s.vicente@tecnico.ulisboa.pt>

routers/web/admin/badges.go

techknowlogick · 2024-06-19T15:34:13Z

@HenriquerPimentel looks like some different CI steps are failing, also related to linting (some public go functions require comments to document their purpose, eg AdminCreateBadge), although with AdminCreateBadge as it is a wrapper for another function without any transformative properties or checks that one could probably be kept as just CreateBadge without the secondary function.

As a sidenote: force pushing erases some review history as GitHub sees some files as new (even if they remain the same). If you could reduce your force pushes, it'd be helpful to me as a reviewer so I can keep track of what I've seen already
disclaimer: most of the time we try to discourage force pushing to PRs for this reason, although depending on the reviewer of a certain PR they can "call an audible" depending on the situation and might be ok with it.

models/user/badge.go

wxiaoguang · 2025-09-18T02:12:49Z

routers/web/admin/badges.go

+	if err != nil {
+		if user_model.IsErrUserNotExist(err) {
+			ctx.Flash.Error(ctx.Tr("form.user_not_exist"))
+			ctx.JSONRedirect(fmt.Sprintf("%s/-/admin/badges/%s/users", setting.AppSubURL, ctx.PathParam("badge_slug")))


Two bugs:

string variables used in a URL should be escaped (and other places)

this if block will continue executing, then you have 2 JSONRedirect calls in one response.

@go-gitea/maintainers

I have suggested many times to have a better framework to handle various errors and returns.

You can see that how many bugs caused by our stupid framework in this PR.

wxiaoguang · 2025-09-18T02:16:02Z

services/user/badge.go

+// UpdateBadgeDescription changes the description and/or image of a badge
+func UpdateBadge(ctx context.Context, b *user_model.Badge) error {
+	return db.WithTx(ctx, func(ctx context.Context) error {
+		return user_model.UpdateBadge(ctx, b)


Why single SQL needs transaction?

wxiaoguang · 2025-09-18T02:16:31Z

services/user/badge.go

+	return db.WithTx(ctx, func(ctx context.Context) error {
+		if err := user_model.DeleteBadge(ctx, b); err != nil {
+			return fmt.Errorf("DeleteBadge: %w", err)
+		}
+		return nil
+	})


There is already a transaction in DeleteBadge.

So why we need this wrap function in service package?

wxiaoguang · 2025-09-18T02:19:44Z

templates/admin/badge/edit.tmpl

+				<div class="non-local field {{if .Err_Slug}}error{{end}}" disabled=disabled>
+					<label for="slug">{{ctx.Locale.Tr "admin.badges.slug"}}</label>
+					<input disabled=disabled id="slug" name="slug" value="{{.Badge.Slug}}">
+				</div>


Some questions:

Why parent "div" has "disabled=disabled" and inner "input" also has "disabled=disabled"?

If there is no input (no change), why {{if .Err_Slug}} would occurs?

If the input is disabled, why the "label" has "for"?

What does "non-local" mean here?

wxiaoguang · 2025-09-18T02:20:19Z

templates/admin/badge/edit.tmpl

+					<label for="description">{{ctx.Locale.Tr "admin.badges.description"}}</label>
+					<textarea id="description" type="text" name="description" rows="2">{{.Badge.Description}}</textarea>


Our frontend framework can handle most a11y cases, so no need to repeat id and for (and other places)

wxiaoguang · 2025-09-18T02:20:55Z

templates/admin/badge/edit.tmpl

+		</div>
+	</div>
+
+<div class="ui g-modal-confirm delete modal" id="delete-badge-modal">


delete class is a no-op IIRC

wxiaoguang · 2025-09-18T02:24:59Z

templates/admin/badge/users.tmpl

+		{{end}}
+		{{template "base/paginate" .}}
+		<div class="ui bottom attached segment">
+			<form class="ui form" id="search-badge-user-form" action="{{.Link}}" method="post">


Why the search form is at the bottom?

IIRC for all (at least almost) cases Gitea layout is like this: search form at top, table at bottom.

ps: why it needs an id? I don't see it is used anywhere.

Hmm ... it is not a real "search" form, but it is used to add a new user, so maybe it's fine.

But still have the question why there are unused id attributes.

wxiaoguang · 2025-09-18T02:26:28Z

templates/admin/badge/view.tmpl

+				<div class="flex-list">
+					<div class="flex-item">
+					{{if .Image}}
+						<div class="flex-item-leading">
+						<img width="64" height="64" src="{{.Badge.ImageURL}}" alt="{{.Badge.Description}}" data-tooltip-content="{{.Badge.Description}}">
+					</div>
+					{{end}}
+					<div class="flex-item-main">
+							<div class="flex-item-title">
+								{{.Badge.Slug}}
+							</div>
+							<div class="flex-item-body">
+								{{.Badge.Description}}
+							</div>
+						</div>
+					</div>
+				</div>


Many "div" open/close tags misaligned, I think it's better to make the tags align correctly to make code readable.

GiteaBot added the lgtm/need 2 label Jun 5, 2024

pull-request-size bot added the size/XXL label Jun 5, 2024

github-actions bot added modifies/translation modifies/go modifies/templates labels Jun 5, 2024

HenriquerPimentel marked this pull request as draft June 5, 2024 14:10

HenriquerPimentel force-pushed the Badge branch from 9983632 to 91b085b Compare June 5, 2024 16:18

HenriquerPimentel force-pushed the Badge branch from 91b085b to 9bd77fd Compare June 14, 2024 11:59

HenriquerPimentel marked this pull request as ready for review June 14, 2024 12:00

HenriquerPimentel force-pushed the Badge branch from 9bd77fd to 5cc1147 Compare June 14, 2024 12:00

lunny added this to the 1.23.0 milestone Jun 14, 2024

techknowlogick reviewed Jun 14, 2024

View reviewed changes

templates/admin/badge/users.tmpl Show resolved Hide resolved

templates/admin/badge/users.tmpl Outdated Show resolved Hide resolved

HenriquerPimentel force-pushed the Badge branch from 5cc1147 to f378b9b Compare June 17, 2024 21:28

HenriquerPimentel force-pushed the Badge branch from 3f64e40 to 5cbed8a Compare June 18, 2024 23:42

HenriquerPimentel requested a review from techknowlogick June 18, 2024 23:42

HenriquerPimentel and others added 3 commits June 19, 2024 13:53

Implemented Badge Management in administration panel (go-gitea#29798)

8b86c31

Co-authored-by: Diogo Vicente <diogo.m.s.vicente@tecnico.ulisboa.pt>

Implemented User Badge Management Interface (go-gitea#29798)

f68e44d

Co-authored-by: Diogo Vicente <diogo.m.s.vicente@tecnico.ulisboa.pt>

Fix linting recommendations

d681313

HenriquerPimentel force-pushed the Badge branch from 5cbed8a to d681313 Compare June 19, 2024 12:53

techknowlogick reviewed Jun 19, 2024

View reviewed changes

routers/web/admin/badges.go Outdated Show resolved Hide resolved

techknowlogick added 2 commits September 7, 2024 18:09

Merge branch 'main' into Badge

10463ec

fix lints

52bc49d

pull-request-size bot added size/XL and removed size/XXL labels Sep 8, 2024

lunny reviewed Sep 26, 2024

View reviewed changes

models/user/badge.go Outdated Show resolved Hide resolved

lunny reviewed Sep 26, 2024

View reviewed changes

models/user/badge.go Outdated Show resolved Hide resolved

lunny reviewed Sep 26, 2024

View reviewed changes

models/user/badge.go Outdated Show resolved Hide resolved

Merge branch 'main' into Badge

401d257

fix test

f67cc76

github-actions bot added topic/code-linting modifies/api modifies/cli modifies/docs modifies/internal modifies/dependencies modifies/frontend labels Sep 14, 2025

improvements

09b4e68

github-actions bot removed topic/code-linting modifies/api modifies/cli modifies/docs modifies/internal modifies/dependencies modifies/frontend labels Sep 15, 2025

lunny added 4 commits September 17, 2025 12:21

fix switch

da6f411

Merge branch 'main' into HenriquerPimentel-Badge

a3a5428

update comment

97a72c3

Merge branch 'main' into HenriquerPimentel-Badge

17735bf

wxiaoguang reviewed Sep 18, 2025

View reviewed changes

lunny modified the milestones: 1.25.0, 1.26.0 Sep 18, 2025

		<label for="description">{{ctx.Locale.Tr "admin.badges.description"}}</label>
		<textarea id="description" type="text" name="description" rows="2">{{.Badge.Description}}</textarea>

Uh oh!

Manage User Badges in the UI #31262

Are you sure you want to change the base?

Manage User Badges in the UI #31262

Conversation

HenriquerPimentel commented Jun 5, 2024

Uh oh!

techknowlogick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

HenriquerPimentel commented Jun 17, 2024

Uh oh!

Uh oh!

techknowlogick commented Jun 19, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!