Skip to content

Add API routes to lock and unlock issues #34165

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
Merged
merged 10 commits into from
Apr 21, 2025
Merged

Add API routes to lock and unlock issues #34165

merged 10 commits into from
Apr 21, 2025

Conversation

YaFou
Copy link
Contributor

@YaFou YaFou commented Apr 9, 2025 •
edited by delvh
Loading

This pull request adds a GitHub-compatible API endpoint to lock and unlock an issue.

The following routes exist now:

  • PUT /api/v1/repos/{owner}/{repo}/issues/{id}/lock to lock an issue
  • DELETE /api/v1/repos/{owner}/{repo}/issues/{id}/lock to unlock an issue

Fixes #33677
Fixes #20012

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 9, 2025
@github-actions github-actions bot added modifies/api This PR adds API routes or modifies them modifies/go Pull requests that update Go code labels Apr 9, 2025
@pull-request-size pull-request-size bot added size/XL and removed size/L labels Apr 9, 2025
@delvh
Copy link
Member

delvh commented Apr 9, 2025

Do you know if there's a GitHub API for these routes?
If yes, we can (and should) use a similar path and response status

@pull-request-size pull-request-size bot added size/L and removed size/XL labels Apr 9, 2025
@delvh
Copy link
Member

delvh commented Apr 9, 2025

Ah, found it:
https://docs.github.com/en/rest/issues/issues?apiVersion=2022-11-28#lock-an-issue
https://docs.github.com/en/rest/issues/issues?apiVersion=2022-11-28#unlock-an-issue

So let's use exactly the same structure as those API routes.

delvh
delvh reviewed Apr 9, 2025
View reviewed changes
TheFox0x7
TheFox0x7 reviewed Apr 9, 2025
View reviewed changes
@TheFox0x7
Copy link
Contributor

Also closes: #20012

@delvh delvh changed the title Add an API endpoint to lock issues Add API routes to lock and unlock issues Apr 14, 2025
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 17, 2025 •
edited
Loading

Why not just accept any "reason" message?

TBH I do not think the legacy design is right: the "reasons" in the config could be candidates, but end users (repo writer/admin) should have the chance to use any reason they like.

The real problems of the legacy design (strict reason check) are:

  1. Not translatable, no i18n
  2. API callers don't know what are the "valid reasons" if an instance changes the default "reason candidates".

(The legacy design is not like GitHub, GitHub doesn't support changing the "reasons" by config file)

So to make things easier and clearer, my proposal is: drop the "valid reason check", use caller's reason as-is. Web UI could still be kept as-is at the moment since it is not in this PR's scope.

@wxiaoguang
Copy link
Contributor

@YaFou What do you think about my proposal #34165 (comment) ? It will resolve the concerns above and make code simpler and more flexible.

@lunny lunny added this to the 1.25.0 milestone Apr 19, 2025
@YaFou
Copy link
Contributor Author

YaFou commented Apr 20, 2025

@YaFou What do you think about my proposal #34165 (comment) ? It will resolve the concerns above and make code simpler and more flexible.

Sorry for the late response. I totally agree to your point of view. I removed all lock reason checks.

@wxiaoguang wxiaoguang modified the milestones: 1.25.0, 1.24.0 Apr 20, 2025
@wxiaoguang
Copy link
Contributor

Made some changes in f7959bc:

  1. Add comments about why the reasons could be used as-is
  2. Simplify the tests, now we don't need to worry about case-sensitive or not.

@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 Apr 20, 2025
@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 Apr 20, 2025
@wxiaoguang wxiaoguang enabled auto-merge (squash) April 21, 2025 00:18
@wxiaoguang wxiaoguang merged commit e947f30 into go-gitea:main Apr 21, 2025
26 checks passed
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 21, 2025
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
b623b85 
* giteaofficial/main:
  Add API routes to lock and unlock issues (go-gitea#34165)
  Make ROOT_URL support using request Host header (go-gitea#32564)
  Valid email address should only start with alphanumeric (go-gitea#28174)
  Fix notify watch failure when the content is too long (go-gitea#34233)
  Add "--fullname" arg to gitea admin user create (go-gitea#34241)
  Fix various UI problems (go-gitea#34243)
  markup: improve code block readability and isolate copy button (go-gitea#34009)
  Don't assume the default wiki branch is master in the wiki API (go-gitea#34244)
  [skip ci] Updated translations via Crowdin
  Optimize the calling code of queryElems (go-gitea#34235)
  Actions Runner rest api (go-gitea#33873)
  Fix some trivial problems (go-gitea#34237)
  Swift files can be passed either as file or as form value (go-gitea#34068)

# Conflicts:
#	templates/repo/wiki/revision.tmpl
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@techknowlogick techknowlogick techknowlogick left review comments

@TheFox0x7 TheFox0x7 TheFox0x7 left review comments

@delvh delvh delvh left review comments

@lunny lunny lunny approved these changes

@wxiaoguang wxiaoguang wxiaoguang 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 modifies/go Pull requests that update Go code modifies/translation
Projects
None yet
Milestone
1.24.0
Development

Successfully merging this pull request may close these issues.

Lock issue conversation via API API endpoint for locking conversation on issues
7 participants
@YaFou @delvh @TheFox0x7 @wxiaoguang @lunny @techknowlogick @GiteaBot