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.

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

Ensure "ref" parameter is not ignored in function RepoRefForAPI #24242

Closed
wants to merge 3 commits into from

Conversation

6543
Copy link
Member

@6543 6543 commented Apr 20, 2023

before

  • work
    • http://localhost:3000/api/v1/repos/test/aaa/raw/.woodpecker.yml
    • http://localhost:3000/api/v1/repos/test/aaa/raw/500c8e1f4c3d33c1d6990c5498db0f0a02136cc5/.woodpecker.yml
  • broken
    • http://localhost:3000/api/v1/repos/test/aaa/raw/.woodpecker.yml?ref=500c8e1f4c3d33c1d6990c5498db0f0a02136cc5

after all work

@6543 6543 added type/bug modifies/api This PR adds API routes or modifies them labels Apr 20, 2023
@6543
Copy link
Member Author

6543 commented Apr 20, 2023

there looks to be an underlying issue about our handler ... in this case that fix just did it and since we only need the API context here anyways it simplify the code :)

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 20, 2023
@6543 6543 changed the title Fix api raw ref issue Ensure "ref" parameter is not ignored in function RepoRefForAPI Apr 21, 2023
@wxiaoguang
Copy link
Contributor

there looks to be an underlying issue about our handler ...

What issue? Is it related to #24080 ? If yes, I will fix it.

@techknowlogick
Copy link
Member

Could you add some tests?

@wxiaoguang
Copy link
Contributor

I think I know the problem of this issue, it is related to #24080 , I am working on it.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 26, 2023

@6543 the new route code is correct. The old code (#19449) forgot to call next.ServeHTTP in a middleware

The fix is 77b426e , just one line fix, so it's also included in #24330

image

@6543
Copy link
Member Author

6543 commented Apr 27, 2023

@wxiaoguang the lastest changes fix it ... I will create a new pull that contains the test I used to determine it :)

@6543
Copy link
Member Author

6543 commented Apr 27, 2023

-> #24388

silverwind pushed a commit that referenced this pull request Apr 29, 2023
This pull request adds an integration test to validate the behavior of
raw content API's reference handling for all supported formats .

close  #24242

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@6543 6543 deleted the fix-api-raw-ref-issue branch April 29, 2023 03:29
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jul 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants