feat: Allow converting mirror repos to normal through the API #8932
No reviewers
Labels
No labels
arch
riscv64
backport/v1.19
backport/v1.20
backport/v1.21/forgejo
backport/v10.0/forgejo
backport/v11.0/forgejo
backport/v12.0/forgejo
backport/v13.0/forgejo
backport/v14.0/forgejo
backport/v7.0/forgejo
backport/v8.0/forgejo
backport/v9.0/forgejo
breaking
bug
bug
confirmed
bug
duplicate
bug
needs-more-info
bug
new-report
bug
reported-upstream
code/actions
code/api
code/auth
code/auth/faidp
code/auth/farp
code/email
code/federation
code/git
code/migrations
code/packages
code/wiki
database
MySQL
database
PostgreSQL
database
SQLite
dependency-upgrade
dependency
certmagic
dependency
chart.js
dependency
Chi
dependency
Chroma
dependency
citation.js
dependency
codespell
dependency
css-loader
dependency
devcontainers
dependency
dropzone
dependency
editorconfig-checker
dependency
elasticsearch
dependency
enmime
dependency
F3
dependency
ForgeFed
dependency
garage
dependency
Git
dependency
git-backporting
dependency
Gitea
dependency
gitignore
dependency
go-ap
dependency
go-enry
dependency
go-gitlab
dependency
Go-org
dependency
go-rpmutils
dependency
go-sql-driver mysql
dependency
go-swagger
dependency
go-version
dependency
go-webauthn
dependency
gocron
dependency
Golang
dependency
goldmark
dependency
goquery
dependency
Goth
dependency
grpc-go
dependency
happy-dom
dependency
Helm
dependency
image-spec
dependency
jsonschema
dependency
KaTeX
dependency
lint
dependency
MariaDB
dependency
Mermaid
dependency
minio-go
dependency
misspell
dependency
Monaco
dependency
PDFobject
dependency
playwright
dependency
postcss
dependency
postcss-plugins
dependency
pprof
dependency
prometheus client_golang
dependency
protobuf
dependency
relative-time-element
dependency
renovate
dependency
reply
dependency
ssh
dependency
swagger-ui
dependency
tailwind
dependency
temporal-polyfill
dependency
terminal-to-html
dependency
tests-only
dependency
text-expander-element
dependency
urfave
dependency
vfsgen
dependency
vite
dependency
Woodpecker CI
dependency
x tools
dependency
XORM
Discussion
duplicate
enhancement/feature
forgejo/accessibility
forgejo/branding
forgejo/ci
forgejo/commit-graph
forgejo/documentation
forgejo/furnace cleanup
forgejo/i18n
forgejo/interop
forgejo/moderation
forgejo/privacy
forgejo/release
forgejo/scaling
forgejo/security
forgejo/ui
Gain
High
Gain
Nice to have
Gain
Undefined
Gain
Very High
good first issue
i18n/backport-stable
impact
large
impact
medium
impact
small
impact
unknown
Incompatible license
issue
closed
issue
do-not-exist-yet
issue
open
manual test
Manually tested during feature freeze
OS
FreeBSD
OS
Linux
OS
macOS
OS
Windows
problem
QA
regression
release blocker
Release Cycle
Feature Freeze
release-blocker
v7.0
release-blocker
v7.0.1
release-blocker
v7.0.2
release-blocker
v7.0.3
release-blocker
v7.0.4
release-blocker
v8.0.0
release-blocker/v9.0.0
run-all-playwright-tests
run-end-to-end-tests
test
manual
test
needed
test
needs-help
test
not-needed
test
present
untested
User research - time-tracker
valuable code
worth a release-note
User research - Accessibility
User research - Blocked
User research - community
User research - Config (instance)
User research - Errors
User Research - Filters
User Research - Git workflow
User Research - In the Future
User Research - Labels
User Research - Moderation
User Research - Needs Input
User research - Notifications/Dashboard
User Research - Repo creation
User Research - Repo Units
User Research - security
User Research - Settings (in-app)
No milestone
No project
No assignees
6 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
forgejo/forgejo!8932
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "mactynow/forgejo:feat/add-convert-repository-api-for-mirrors"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Checklist
The contributor guide contains information that will be helpful to first time contributors. There also are a few conditions for merging Pull Requests in Forgejo repositories. You are also welcome to join the Forgejo development chatroom.
Tests
*_test.gofor unit tests.tests/integrationdirectory if it involves interactions with a live Forgejo server.web_src/js/*.test.jsif it can be unit tested.tests/e2e/*.test.e2e.jsif it requires interactions with a live Forgejo server (see also the developer guide for JavaScript testing).Documentation
Release notes
release-notes/<pull request number>.mdto be be used for the release notes instead of the title.Fixes #7733.
Release notes
b641c9a7990e20575d22Where can I update the API documentation for this change?
@mactynow wrote in #8932 (comment):
Not sure if this is applicable (i.e. this PR needs an update to the docs) but you would change this file via the forgejo/docs repository
Allow converting mirror repos to normal through the APIto feat: Allow converting mirror repos to normal through the API@mactynow If you only mean the swagger docs, I think
make generate-swaggerand committing the changed file should do the trick. I don't think this change requires extensive documentation.@fnetX that's exactly what I wanted to do. Thanks!
The linter is now giving me issues unrelated to my changes so I won't address them in this PR (unless you want me to?)
The linter check is related to your change at #8932/files
443598f3e23602787a97@earl-warren wrote in #8932 (comment):
My bad, that's fixed now, however I'm not sure what I should do about the failing e2e tests.
@mactynow wrote in #8932 (comment):
You can ignore them, they are known false negative.
@ -87,0 +90,4 @@func TestRepoConvertToNormalRepo(t *testing.T) {unittest.PrepareTestEnv(t)ctx, _ := contexttest.MockAPIContext(t, "user2/repo1") // TODO: is this a mirror? doesn't look like it, how to test?this test is effectively a false positive since the repo is not a mirror to begin with. You should look for a fixture that is already a mirror and then.
assert.True(t, repo.IsMirror)to be sure.https://codeberg.org/forgejo/forgejo/src/branch/forgejo/models/fixtures/repository.yml#L133-L165
Just a thought: Wouldn't it feel more intuitive if "convert to normal repo" was a specific route that you could POST to, e.g.
POST /owner/repo/convert(optionally with the option to convert from fork to normal, or from mirror to normal, but I prefer to just do it automatically).It feels a little odd to have a struct field for something that is basically a one-time and one-way operation.
What do you think?
@fnetX that would make sense. I thought that I'd leave it under
Edit()because it seemed that this is where all the operations from the UI related to making changes on the repo are grouped and this way it was consistent between the two. But with the code for the operation under services it's not complicated to understand what's going on. I'll make a separate route then.The UI has a "danger" section, though. And ... I think in general we should not think of the UI too much, because it's easier to move some buttons if they make more sense somewhere else. It's much more difficult with API routes.
2634e398979b51b3e5defeat: Allow converting mirror repos to normal through the APIto WIP: feat: Allow converting mirror repos to normal through the APII have a few questions:
user5for testing this doesn't work, I get a bunch of errors when doing the login steps. I suspect becauseis_activeisfalse? Regardless, I tried withuser20instead and don't get these errors, but becauseuser20/repo25is not in the fixtures as a an actual git repo, the tests are failing (on theCleanUpMigrateInfo()steps). Should I addrepo25as a fixture and if so, is there a process for that, or should I approach this differently?200if calling this api for a repo that is not a mirror, as no action is taken and nothing changes. Is that right or would you rather return something else?@mactynow wrote in #8932 (comment):
What is what you want to have? A mirror repository and a user who has access to convert it?
@mactynow wrote in #8932 (comment):
I think "422 Unprocessable Entity" would be fitting and is already used in other router code for such purposes.
@Gusted wrote in #8932 (comment):
Yes, but I think it also needs to have the actual repo (with the .git subdirectory) under
tests/gitea-repositories-meta. User3 doesn't seem to be authorized to log in.user3 is an organization.
I think you can use
user3/repo5and log in as user2 who is a owner of that organisation. This repository has fixtures.There's two linter issues, I think they are fixed if you call
make fmtand commit the result. Otherwisemake lint-go-fixmight fix it.@Gusted wrote in #8932 (comment):
Whoops, sorry, I missed that the push went through, since there were some issues earlier. Fixed now! Let me also remove the WIP tag.
WIP: feat: Allow converting mirror repos to normal through the APIto feat: Allow converting mirror repos to normal through the APIfe9de3684202fd10a1c0@ -675,0 +704,4 @@return}repo, err := repo_model.GetRepositoryByID(ctx, ctx.Repo.Repository.ID)Is there a reason to return the repository?
Not really, I just followed the pattern from the
Edit()function. I'm not sure what I should be returning or if I should be returning anything at all here actually. What do you think?There's not really anything interesting being returned for this repository data except
is_mirror = false, so I don't think the response would really be used. Hm let's leave it for now. Not too much of a problem then.@ -120,2 +120,4 @@}// ConvertMirrorToNormalRepo converts a mirror to a normal repofunc ConvertMirrorToNormalRepo(ctx context.Context, repo *repo_model.Repository) (err error) {Nicely done by moving it to
service/, good choice!The credit goes to you, that was your suggestion: #7733 (comment)
Oops 🙈
02fd10a1c04e60c61a0f4e60c61a0f0da361bb670da361bb67c20102aee4Where does that come from?
The following is a preview of the release notes for this pull request, as they will appear in the upcoming release. They are derived from the content of the `release-notes/8932.md` file, if it exists, or the title of the pull request. They were also added at the bottom of the description of this pull request for easier reference.This message and the release notes originate from a call to the release-notes-assistant.
Release notes