Allow options to disable user gpg keys configuration from the interface on app.ini #29486
Conversation
GiteaBot
added
the
lgtm/need 2
pull-request-size
bot
added
the
size/M
github-actions
bot
added
modifies/api
lunny
changed the title
GiteaBot
added
lgtm/need 1
| if setting.Admin.UserDisabledFeatures.Contains(setting.UserFeatureManageGPGKeys) { | ||
| ctx.NotFound("Not Found", fmt.Errorf("gpg keys setting is not allowed to be visited")) | ||
| return | ||
| } |
There was a problem hiding this comment.
Should we move this check into the middleware to keep the logic in the router on topic?
There was a problem hiding this comment.
I did that on #20549 but @wxiaoguang think it's better to move them into the router function body.
There was a problem hiding this comment.
Do you mean this comment? #29275 (comment)
At that time:
- Only one handler needs such check
- It bloats the "web.go", do we really want to fill more and more function into "web.go"?
For this time: it is still the only way: put checks into routers/web/user/setting/keys.go, it is impossible to "move into middleware" for it.
My reviews never means "it must do so", I am always open for better ideas.
So, for this problem: what is the consistent and maintainable approach in your mind?
There was a problem hiding this comment.
Something like this for the middleware:
func featureEnabled(feature /*...*/string) func(ctx *context.Context) {
if setting.Admin.UserDisabledFeatures.Contains(feature) {
ctx.Error(http.StatusNotFound)
return
}
}But that does not work well with the method with the type parameter. Repeating the same code multiple times is not really good either.
There was a problem hiding this comment.
And by the way, I think my concern for "testing" is still not addressed there #29275 (comment)
There was a problem hiding this comment.
But that does not work well with the method with the
typeparameter. Repeating the same code multiple times is not really good either.
There is no consistent way at the moment (if there is no refactoring). You can see that there is no way to use your middleware for the "web.go" now.
Update: I didn't mean blocker, and I don't have other ideas besides "refactoring to a clearer approach".
So feel free to use any style people prefer.
There was a problem hiding this comment.
Not really no way but we need to use one unpleasant way.
GiteaBot
added
lgtm/done
* giteaofficial/main: Actions Artifacts v4 backend (go-gitea#28965) Remove jQuery AJAX from common global functions (go-gitea#29528) Allow options to disable user gpg keys configuration from the interface on app.ini (go-gitea#29486) [skip ci] Updated translations via Crowdin Fix issue & comment history bugs (go-gitea#29525) Fix incorrect diff expander for deletion of last lines in a file (go-gitea#29501) Ignore `__debug_bin*` which is generated by vscode when debugging (go-gitea#29524)
Follow #29447
Fix #29454
Extract from #20549