Skip to content

Fix fetch action redirect#37437

Merged
wxiaoguang merged 4 commits intogo-gitea:mainfrom
wxiaoguang:fix-fetch-action-redirect
Apr 26, 2026
Merged

Fix fetch action redirect#37437
wxiaoguang merged 4 commits intogo-gitea:mainfrom
wxiaoguang:fix-fetch-action-redirect

Conversation

@wxiaoguang
Copy link
Copy Markdown
Contributor

And add tests for its behavior

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 26, 2026
@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 26, 2026

Not sure if this changes anything but keep note that redirect behaviour can be controlled in fetch via https://developer.mozilla.org/en-US/docs/Web/API/Request/redirect.

const resp = fetch(url, {redirect: 'manual'});
if (resp.type === 'opaqueredirect') {
  console.log("Redirect intercepted manually.");
}

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Not sure if this changes anything but keep note that redirect behaviour can be controlled in fetch via https://developer.mozilla.org/en-US/docs/Web/API/Request/redirect.

Unrelated. I think I have answered such question in the previous PRs, and left enough comments.

@silverwind
Copy link
Copy Markdown
Member

Backport needed?

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Backport needed?

No, it's caused by the htmx removal.

@wxiaoguang wxiaoguang added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Apr 26, 2026
- await the async calls
- mockClear between cases instead of resetAllMocks (preserves the
  mockImplementation), restore spies at end
- return void from mockImplementation rather than null

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Test cleanup done in 06e0fb5.

@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 26, 2026
@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Apr 26, 2026

Why not just call vi.clearAllMocks();?

@silverwind
Copy link
Copy Markdown
Member

silverwind commented Apr 26, 2026

Why not just call vi.clearAllMocks();?

vi.resetAllMocks() also wipes the mock implementation, so after the first reset the spies fall through to the real assign/reload. mockClear() keeps the () => {} no-op and just clears call history; mockRestore() at the end puts the originals back.


Posted by Claude Opus 4.7 on behalf of @silverwind

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Why not just call vi.clearAllMocks();?

vi.resetAllMocks() also wipes the mock implementation, so after the first reset the spies fall through to the real assign/reload. mockClear() keeps the () => {} no-op and just clears call history; mockRestore() at the end puts the originals back.

Are you SURE?

image

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

Why not just call vi.clearAllMocks();?

vi.resetAllMocks() also wipes the mock implementation, so after the first reset the spies fall through to the real assign/reload. mockClear() keeps the () => {} no-op and just clears call history; mockRestore() at the end puts the originals back.

Posted by Claude Opus 4.7 on behalf of @silverwind

Can you spend more time on thinking about the problem?

If "fall through to the real ", how can the tests pass and succeed?

@silverwind
Copy link
Copy Markdown
Member

vi.clearAllMocks is actually functionally equivalent here, keep it, it's shorter.

@wxiaoguang
Copy link
Copy Markdown
Contributor Author

wxiaoguang commented Apr 26, 2026

vi.clearAllMocks is actually functionally equivalent here, keep it, it's shorter.

resetAllMocks is also right, teach your AI

@silverwind
Copy link
Copy Markdown
Member

It saw its error, I will add the memory

I jumped from "wipes the implementation" to "test isolation is broken", without actually testing that.

@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 26, 2026
@wxiaoguang wxiaoguang enabled auto-merge (squash) April 26, 2026 16:32
@wxiaoguang wxiaoguang merged commit 2f42c8c into go-gitea:main Apr 26, 2026
26 checks passed
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 26, 2026
silverwind added a commit to McMichalK/gitea that referenced this pull request Apr 26, 2026
* origin/main: (176 commits)
  Refactor pull request view (3) (go-gitea#37439)
  Update 1.26.1 changelog in main (go-gitea#37442)
  Make GetPossibleUserByID can handle deleted user (go-gitea#37430)
  Fix fetch action redirect (go-gitea#37437)
  Refactor integration test DecodeJSON calls to use generic return value (go-gitea#37432)
  Integrate renovate bot for all dependency updates (go-gitea#37050)
  Refactor pull request view (2) (go-gitea#37428)
  Use MarkLongPolling instead of hard-coded route path (go-gitea#37427)
  Optimize CI caches (go-gitea#37387)
  Update AGENTS.md (go-gitea#37420)
  Update Nix flake (go-gitea#37425)
  [skip ci] Updated translations via Crowdin
  remove excessive quote from terraform instructions (go-gitea#37424)
  Improve testing init, clean up webhook tests (go-gitea#37412)
  Fix color regressions, add `priority` color (go-gitea#37417)
  [skip ci] Updated translations via Crowdin
  Stabilize e2e logout propagation test (go-gitea#37403)
  refactor: serve site manifest via `/assets/site-manifest.json` endpoint (go-gitea#37405)
  feat(security): set X-Content-Type-Options: nosniff by default (go-gitea#37354)
  Refactor pull request view (1) (go-gitea#37380)
  ...

# Conflicts:
#	templates/repo/diff/box.tmpl
@wxiaoguang wxiaoguang deleted the fix-fetch-action-redirect branch April 26, 2026 19:58
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 27, 2026
* main: (33 commits)
  refactor: use named `Permission` field in `Repository` struct instead of anonymous embedding (go-gitea#37441)
  Refactor pull request view (3) (go-gitea#37439)
  Update 1.26.1 changelog in main (go-gitea#37442)
  Make GetPossibleUserByID can handle deleted user (go-gitea#37430)
  Fix fetch action redirect (go-gitea#37437)
  Refactor integration test DecodeJSON calls to use generic return value (go-gitea#37432)
  Integrate renovate bot for all dependency updates (go-gitea#37050)
  Refactor pull request view (2) (go-gitea#37428)
  Use MarkLongPolling instead of hard-coded route path (go-gitea#37427)
  Optimize CI caches (go-gitea#37387)
  Update AGENTS.md (go-gitea#37420)
  Update Nix flake (go-gitea#37425)
  [skip ci] Updated translations via Crowdin
  remove excessive quote from terraform instructions (go-gitea#37424)
  Improve testing init, clean up webhook tests (go-gitea#37412)
  Fix color regressions, add `priority` color (go-gitea#37417)
  [skip ci] Updated translations via Crowdin
  Stabilize e2e logout propagation test (go-gitea#37403)
  refactor: serve site manifest via `/assets/site-manifest.json` endpoint (go-gitea#37405)
  feat(security): set X-Content-Type-Options: nosniff by default (go-gitea#37354)
  ...
silverwind added a commit to hanism01/gitea that referenced this pull request Apr 27, 2026
…-review-feedback

* origin/main: (144 commits)
  Add API endpoint to reply to pull request review comments (go-gitea#36683)
  Add CurrentURL template variable back (go-gitea#37444)
  refactor: use named `Permission` field in `Repository` struct instead of anonymous embedding (go-gitea#37441)
  Refactor pull request view (3) (go-gitea#37439)
  Update 1.26.1 changelog in main (go-gitea#37442)
  Make GetPossibleUserByID can handle deleted user (go-gitea#37430)
  Fix fetch action redirect (go-gitea#37437)
  Refactor integration test DecodeJSON calls to use generic return value (go-gitea#37432)
  Integrate renovate bot for all dependency updates (go-gitea#37050)
  Refactor pull request view (2) (go-gitea#37428)
  Use MarkLongPolling instead of hard-coded route path (go-gitea#37427)
  Optimize CI caches (go-gitea#37387)
  Update AGENTS.md (go-gitea#37420)
  Update Nix flake (go-gitea#37425)
  [skip ci] Updated translations via Crowdin
  remove excessive quote from terraform instructions (go-gitea#37424)
  Improve testing init, clean up webhook tests (go-gitea#37412)
  Fix color regressions, add `priority` color (go-gitea#37417)
  [skip ci] Updated translations via Crowdin
  Stabilize e2e logout propagation test (go-gitea#37403)
  ...

# Conflicts:
#	models/project/column.go
#	routers/web/repo/issue_page_meta.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants