-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Allow adding collaborators with (fullname) #3103
Conversation
Signed-off-by: Sasha Varlamov <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #3103 +/- ##
==========================================
- Coverage 34.08% 34.05% -0.03%
==========================================
Files 273 274 +1
Lines 40018 40019 +1
==========================================
- Hits 13639 13628 -11
- Misses 24443 24453 +10
- Partials 1936 1938 +2
Continue to review full report at Codecov.
|
LGTM |
routers/repo/setting.go
Outdated
@@ -367,6 +367,10 @@ func Collaboration(ctx *context.Context) { | |||
// CollaborationPost response for actions for a collaboration of a repository | |||
func CollaborationPost(ctx *context.Context) { | |||
name := strings.ToLower(ctx.Query("collaborator")) | |||
// name may be formatted as "username (fullname)" | |||
if strings.Contains(name, "(") && strings.HasSuffix(name, ")") { |
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.
nit: can simplify to
if index := strings.Index(name, " ("); index >= 0 {
name = name[:index]
}
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.
I just cut-n-pasted @lunny's fix. I agree with you on this. Where can I put a utility method and then refactor all of the usages? Sorry new to Gitea, so want to make sure I'm following your guys' best practices... Thanks
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.
I think adding a routers/utils/utils.go
file and putting it there makes the most sense. This proposed function seems specific to route handlers, so I'd prefer to not add it to modules/util
.
Signed-off-by: Sasha Varlamov <[email protected]>
So I moved it out into a new utils pkg within routers. I also refactored the two usages within |
LGTM. Thank you for adding tests! |
make L-G-T-M work |
Please send a back port to branch |
@lunny So I should file another PR against that rel branch? New to the Gitea standards here :) |
@svarlamov Yes, see #3106 as an example of a backport. |
@svarlamov git fetch release/v1.3
git co release/v1.3
git co -b lunny/fix_xxx
git cherry-pick xxxx
...... |
* Allow adding collaborators with (fullname) Signed-off-by: Sasha Varlamov <[email protected]> * Refactor username suffix to utils pkg Signed-off-by: Sasha Varlamov <[email protected]>
* Allow adding collaborators with (fullname) Signed-off-by: Sasha Varlamov <[email protected]> * Refactor username suffix to utils pkg Signed-off-by: Sasha Varlamov <[email protected]>
Signed-off-by: Sasha Varlamov [email protected]
Targeting: #3102
Perhaps we should fork this out into a utility func? I'm pretty new to Gitea, so I just in-lined it same as #2973 but since this keeps popping up, I suppose we might want a shared function for this... Where should I put it and then refactor the #2973 fix?