Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix create team, update team missing units #5188

Merged
merged 8 commits into from
Nov 10, 2018
Merged

Fix create team, update team missing units #5188

merged 8 commits into from
Nov 10, 2018

Conversation

lunny
Copy link
Member

@lunny lunny commented Oct 26, 2018

Should fix #5187. Now when you use the API, you have to give the units on the form if the team is not an admin team. The units is an array of string, it could be repo.code,repo.issues,repo.ext_issues,repo.wiki,repo.pulls,repo.releases,repo.ext_wiki now.

@lunny lunny added the type/bug label Oct 26, 2018
@lunny lunny added this to the 1.7.0 milestone Oct 26, 2018
kolaente
kolaente suggested changes Oct 26, 2018
View reviewed changes
Copy link
Member

@kolaente kolaente left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update swagger.

models/unit.go Outdated
// FindUnitTypes give the unit key name and return unit
func FindUnitTypes(nameKeys ...string) (res []UnitType) {
for t, u := range Units {
if strings.EqualFold(u.NameKey, u.NameKey) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is always true. You are comparing u.NameKey with u.NameKey...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 26, 2018
@kolaente
Copy link
Member

Should we backport this?

@lunny
Copy link
Member Author

lunny commented Oct 27, 2018

@kolaente I think yes, since v1.6 stable is not released.

@kolaente
Copy link
Member

CI still fails, looks like swagger is not right.

techknowlogick
techknowlogick reviewed Oct 30, 2018
View reviewed changes
templates/swagger/v1_json.tmpl Outdated
@@ -21,7 +21,7 @@
},
"version": "1.1.1"
},
"basePath": "{{AppSubUrl}}/api/v1",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lunny the change on this line needs to be reverted for CI to be successful.

@techknowlogick
Copy link
Member

Alright this now passes swagger check.

@kolaente would you be able to review?

@techknowlogick
Copy link
Member

Blocked by vendor check.

@bkcsoft bkcsoft added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 2, 2018
@lunny lunny mentioned this pull request Nov 3, 2018
Merged
@lunny
Copy link
Member Author

lunny commented Nov 3, 2018

blocked by go-gitea/go-sdk#127

@codecov-io
Copy link

codecov-io commented Nov 9, 2018 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@d487a76). Click here to learn what that means.
The diff coverage is 6.12%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5188   +/-   ##
=========================================
  Coverage          ?   37.37%           
=========================================
  Files             ?      312           
  Lines             ?    46379           
  Branches          ?        0           
=========================================
  Hits              ?    17334           
  Misses            ?    26566           
  Partials          ?     2479
Impacted Files Coverage Δ
routers/api/v1/org/team.go 4.61% <0%> (ø)
models/unit.go 0% <0%> (ø)
routers/api/v1/convert/convert.go 70.86% <100%> (ø)
models/org_team.go 50.6% <11.11%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d487a76...a4a1ca5. Read the comment docs.

@lunny
Copy link
Member Author

lunny commented Nov 9, 2018

@techknowlogick CI PASS

@bkcsoft bkcsoft added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Nov 9, 2018
@lafriks
Copy link
Member

lafriks commented Nov 9, 2018

This wont be breaking change?

@lunny
Copy link
Member Author

lunny commented Nov 9, 2018

@lafriks I think no. Before this PR, the non-owner team you created via API could not visit any repositories since units parameters is missing. After this, you could add units parameters when creating team via API.

@techknowlogick techknowlogick merged commit b3000ae into go-gitea:master Nov 10, 2018
@lunny lunny deleted the lunny/fix_api_create_team branch November 11, 2018 01:57
lunny added a commit to lunny/gitea that referenced this pull request Nov 11, 2018
@lunny
Fix create team, update team missing units (go-gitea#5188)
9f85be2
@lunny lunny mentioned this pull request Nov 11, 2018
Merged
@lunny lunny added the backport/done All backports for this PR have been created label Nov 11, 2018
techknowlogick pushed a commit that referenced this pull request Nov 11, 2018
@lunny @techknowlogick
Fix create team, update team missing units (#5188) (#5313)
8670dec
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@techknowlogick techknowlogick techknowlogick left review comments

@kolaente kolaente kolaente approved these changes

@appleboy appleboy appleboy approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Milestone
1.7.0
Development

Successfully merging this pull request may close these issues.

When creating a team with the api, that team does not have access to anything
7 participants
@lunny @kolaente @techknowlogick @codecov-io @lafriks @appleboy @bkcsoft