-
-
Notifications
You must be signed in to change notification settings - Fork 6k
Manage User Badges in the UI #31262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Manage User Badges in the UI #31262
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 <[email protected]>
Co-authored-by: Diogo Vicente <[email protected]>
@HenriquerPimentel looks like some different CI steps are failing, also related to linting (some public go functions require comments to document their purpose, eg 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 |
options/locale/locale_en-US.ini
Outdated
@@ -603,12 +605,15 @@ unknown_error = Unknown error: | |||
captcha_incorrect = The CAPTCHA code is incorrect. | |||
password_not_match = The passwords do not match. | |||
lang_select_error = Select a language from the list. | |||
invalid_image_url_error = `Please provide a valid image URL.` | |||
invalid_slug_error = `Please provide a valid slug.` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Crowdin doesn't work with back-quote
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
routers/web/admin/badges.go
Outdated
switch { | ||
default: | ||
ctx.ServerError("UpdateBadge", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only switch-default
?
(many places)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
routers/web/admin/badges.go
Outdated
b.ImageURL = form.ImageURL | ||
b.Description = form.Description | ||
|
||
if err := user_model.UpdateBadge(ctx, ctx.Data["Badge"].(*user_model.Badge)); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why it uses dynamic type here ctx.Data["Badge"].(*user_model.Badge))
? There should be a static typed variable "badge"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
routers/web/admin/badges.go
Outdated
page := ctx.FormInt("page") | ||
if page <= 0 { | ||
page = 1 | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
min
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
09b4e68 It should use max
.
routers/web/admin/badges.go
Outdated
if user_model.IsErrBadgeNotExist(err) { | ||
ctx.Flash.Error(ctx.Tr("admin.badges.not_found")) | ||
} else { | ||
ctx.ServerError("AddUserBadge", err) | ||
} | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ctx.Flash.Error
+ return
is wrong
(many places)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
routers/web/admin/badges.go
Outdated
func RenderBadgeSearch(ctx *context.Context, opts *user_model.SearchBadgeOptions, tplName templates.TplName) { | ||
// Sitemap index for sitemap paths | ||
opts.Page = int(ctx.PathParamInt64("idx")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why "sitemap" in the "admin" routes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
services/forms/admin.go
Outdated
type AdminCreateBadgeForm struct { | ||
Slug string `binding:"Required;Slug"` | ||
Description string `binding:"Required"` | ||
ImageURL string `binding:"ValidImageUrl"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is ValidImageUrl
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented #29798
This feature implements:
Implemented Badge Management in administration panel
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