Conversation
|
Addressed the open review threads with the following changes:
Validation run:
|
56aeab8 to
5cbe732
Compare
|
fixed |
wxiaoguang
left a comment
There was a problem hiding this comment.
Need more improvements
| func TestDeleteBadgeUserRedirectEscapesSlug(t *testing.T) { | ||
| unittest.PrepareTestEnv(t) | ||
| ctx, resp := contexttest.MockContext(t, "POST /-/admin/badges/slug/badge/users/delete") | ||
| ctx.SetPathParam("badge_slug", "badge/slug with space") |
There was a problem hiding this comment.
Why it can happen
This test seems not useful and unnecessary
| <label for="slug">{{ctx.Locale.Tr "admin.badges.slug"}}</label> | ||
| <input autofocus required id="slug" name="slug"> |
There was a problem hiding this comment.
No need to use for / id . Framework will add them
| // ErrInvalidGroupTeamMap is returned when a group team mapping is invalid | ||
| ErrInvalidGroupTeamMap = "InvalidGroupTeamMap" | ||
| // ErrInvalidSlug is returned when a slug is invalid | ||
| ErrInvalidSlug = "InvalidSlug" |
There was a problem hiding this comment.
ErrInvalidBadgeSlug = "InvalidBadgeSlug"
| func addSlugPatternRule() { | ||
| binding.AddRule(&binding.Rule{ | ||
| IsMatch: func(rule string) bool { | ||
| return rule == "Slug" |
| if sortType == "" { | ||
| sortType = BadgeSearchDefaultAdminSort | ||
| ctx.SetFormString("sort", sortType) | ||
| } |
There was a problem hiding this comment.
RenderBadgeSearch already does correct fall-back.
These if / SetFormString actually helps nothing
| <div class="menu"> | ||
| <button class="item" name="sort" value="oldest">{{ctx.Locale.Tr "repo.issues.filter_sort.oldest"}}</button> | ||
| <button class="item" name="sort" value="newest">{{ctx.Locale.Tr "repo.issues.filter_sort.latest"}}</button> | ||
| <button class="item" name="sort" value="alphabetically">{{ctx.Locale.Tr "repo.issues.label.filter_sort.alphabetically"}}</button> | ||
| <button class="item" name="sort" value="reversealphabetically">{{ctx.Locale.Tr "repo.issues.label.filter_sort.reverse_alphabetically"}}</button> | ||
| </div> |
There was a problem hiding this comment.
Is there a bug? No item will be selected even if end user chooses one. I haven't tested.
| validSlugPattern: regexp.MustCompile(`^[\da-zA-Z][-.\w]*$`), | ||
| invalidSlugPattern: regexp.MustCompile(`[-._]{2,}|[-._]$`), |
There was a problem hiding this comment.
What are the proper patterns for "badge slug"?
Should it support names like "Awesome!" or "Emoji 💯" ? ( I think no, it only uses image)
If it is not designed to support these patterns, then the test ( https://github.com/go-gitea/gitea/pull/36752/files#r2867004215 ) seems not useful.


trying to reenable #31262