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

Add API endpoint for repo transfer #9947

Merged
merged 6 commits into from
Jan 31, 2020
Merged

Add API endpoint for repo transfer #9947

merged 6 commits into from
Jan 31, 2020

Conversation

6543
Copy link
Member

@6543 6543 commented Jan 23, 2020 •
edited

close #9858

  • docu (swagger)
  • function
  • test

GitHub Spec: link
original PR: https://gitea.artixlinux.org/artix/gitea/pulls/1

@codecov-io
Copy link

codecov-io commented Jan 23, 2020 •
edited

Codecov Report

Merging #9947 into master will increase coverage by 1.05%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9947      +/-   ##
==========================================
+ Coverage   42.38%   43.44%   +1.05%     
==========================================
  Files         620      566      -54     
  Lines       80961    78978    -1983     
==========================================
- Hits        34317    34313       -4     
+ Misses      42408    40429    -1979     
  Partials     4236     4236
Impacted Files Coverage Δ
models/migrations/v76.go 0% <0%> (ø) ⬆️
models/migrations/v70.go 0% <0%> (ø) ⬆️
models/migrations/migrations.go 4.16% <0%> (+2.86%) ⬆️
services/pull/temp_repo.go 31.62% <0%> (-2.57%) ⬇️
services/pull/patch.go 67.92% <0%> (-1.89%) ⬇️
modules/log/event.go 64.61% <0%> (-1.03%) ⬇️
models/gpg_key.go 54.81% <0%> (-0.56%) ⬇️
models/repo.go 51.53% <0%> (+0.06%) ⬆️
routers/repo/view.go 39.47% <0%> (+0.87%) ⬆️
services/pull/check.go 56.64% <0%> (+1.39%) ⬆️

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 cfa9dfd...b64626b. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 23, 2020
@6543 6543 force-pushed the api-repo-transfer branch 3 times, most recently from 2c56968 to e861f30 Compare January 25, 2020 18:33
@6543 6543 changed the title [WIP] API endpint for repo transfer API endpint for repo transfer Jan 25, 2020
@6543
Copy link
Member Author

6543 commented Jan 25, 2020

@camlafit here it is :)

@lunny lunny added modifies/api This PR adds API routes or modifies them type/feature Completely new functionality. Can only be merged if feature freeze is not active. labels Jan 26, 2020
lunny
lunny requested changes Jan 26, 2020
View reviewed changes
routers/api/v1/repo/repo.go Outdated Show resolved Hide resolved
@6543
Copy link
Member Author

6543 commented Jan 26, 2020

@lunny done

@6543 6543 requested a review from lunny January 26, 2020 04:37
@lunny lunny added this to the 1.12.0 milestone Jan 26, 2020
@6543
Copy link
Member Author

6543 commented Jan 26, 2020

@lunny removed the nonesense :D

@6543 6543 requested a review from lunny January 26, 2020 15:43
6543
6543 commented Jan 26, 2020
View reviewed changes
integrations/api_repo_test.go Outdated Show resolved Hide resolved
@zeripath zeripath changed the title API endpint for repo transfer API endpoint for repo transfer Jan 26, 2020
@6543
Copy link
Member Author

6543 commented Jan 26, 2020

@zeripath thanks :D

zeripath
zeripath approved these changes Jan 26, 2020
View reviewed changes
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

small nit - stop routers/api/v1/repo/repo.go from getting too large by splitting out the transfer function as repo_transfer.go

@GiteaBot GiteaBot 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 Jan 26, 2020
@6543
Copy link
Member Author

6543 commented Jan 26, 2020

@lunny look at last two commits - did code-refactor

@lunny
Copy link
Member

lunny commented Jan 27, 2020

I would like to rename repo/repo_migrate.go to repo/migrate.go.

@6543
Copy link
Member Author

6543 commented Jan 27, 2020

@lunny removed prefix because of package/folder name ...

@camlafit
Copy link
Contributor

Could be set teams_ids it's a great idea thanks :)

lunny
lunny requested changes Jan 28, 2020
View reviewed changes
services/repository/transfer.go Outdated
if err != nil {
return err
}
for _, team := range teams {
Copy link
Member

Choose a reason for hiding this comment

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

Could you check the two things?

  • if the new owner is an orgnization?
  • if the teams are belong to the new owner?

@6543 6543 changed the title API endpoint for repo transfer WIP: API endpoint for repo transfer Jan 28, 2020
@6543 6543 force-pushed the api-repo-transfer branch 2 times, most recently from 66cf951 to 84e5454 Compare January 28, 2020 23:56
@6543 6543 mentioned this pull request Jan 29, 2020
Merged
@6543
Copy link
Member Author

6543 commented Jan 29, 2020 •
edited

AddRepository handle OrgCheck and DubleAdd Prefention for us!

gitea/models/org_team.go

Lines 274 to 280 in 1f01f53

// AddRepository adds new repository to team of organization.
func (t *Team) AddRepository(repo *Repository) (err error) {
if repo.OwnerID != t.OrgID {
return errors.New("Repository does not belong to organization")
} else if t.HasRepository(repo.ID) {
return nil
}

it also check if the teams are belong to the new owner?

@6543
Copy link
Member Author

6543 commented Jan 29, 2020

And I already added a test witch make sure it works:

https://github.com/go-gitea/gitea/pull/9947/files#diff-ffd1c307569a21f7d0f7234d5e8f3724R407

@6543
Copy link
Member Author

6543 commented Jan 29, 2020

@lunny I think there is no problem, your described

@6543 6543 changed the title WIP: API endpoint for repo transfer API endpoint for repo transfer Jan 29, 2020
@6543 6543 requested a review from lunny January 29, 2020 00:24
@6543
Copy link
Member Author

6543 commented Jan 30, 2020

ed18d4688cb78317b315a8a3570603de8e289011 should make sure this functin os "atomic"

@GiteaBot GiteaBot 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 Jan 31, 2020
@6543
Copy link
Member Author

6543 commented Jan 31, 2020

ready to merge 🚀

@zeripath zeripath merged commit 13bc820 into go-gitea:master Jan 31, 2020
@6543 6543 deleted the api-repo-transfer branch January 31, 2020 15:50
@jolheiser jolheiser mentioned this pull request Feb 26, 2020
Closed
@hrvoj3e hrvoj3e mentioned this pull request Feb 27, 2020
Closed
7 tasks
@zeripath zeripath mentioned this pull request May 17, 2020
Merged
@zeripath zeripath changed the title API endpoint for repo transfer Add API endpoint for repo transfer May 17, 2020
@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

@lunny lunny lunny approved these changes

@zeripath zeripath zeripath approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/api This PR adds API routes or modifies them type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
1.12.0
Development

Successfully merging this pull request may close these issues.

[Feature] API endpint for repo transfer
6 participants
@6543 @codecov-io @lunny @camlafit @zeripath @GiteaBot