Skip to content

Include query in sign in redirect#11579

Merged
techknowlogick merged 4 commits into
go-gitea:masterfrom
mrsdizzie:context-link
Jun 4, 2020
Merged

Include query in sign in redirect#11579
techknowlogick merged 4 commits into
go-gitea:masterfrom
mrsdizzie:context-link

Conversation

@mrsdizzie
Copy link
Copy Markdown
Member

@mrsdizzie mrsdizzie commented May 23, 2020

Include query string in ctx.Data["CurrentURL"] so people are redirected to the right page after singing in. Fixes case of:

Visit:

https://try.gitea.io/mrsdizzie/testcase/issues?q=&type=all&sort=&state=open&labels=7071&milestone=0&assignee=0

Sign in, then redirected to:

https://try.gitea.io/mrsdizzie/testcase/issues

Create a new "CurrentURL" instead of updating the existing "Link" because too many places in the code assume "Link" won't have a query

Include query string in ctx.Data["SignInLink"] so people are redirected to the right page after singing in. Fixes case of:

Visit:

 https://try.gitea.io/mrsdizzie/testcase/issues?q=&type=all&sort=&state=open&labels=7071&milestone=0&assignee=0

Sign in, then redirected to:

 https://try.gitea.io/mrsdizzie/testcase/issues

Create a new "SignInLink" instead of updating the existing "Link" because too many places in the code assume "Link" won't have a query
@jolheiser jolheiser added this to the 1.13.0 milestone May 24, 2020
@silverwind
Copy link
Copy Markdown
Member

silverwind commented May 27, 2020

Maybe call it RequestURL or similar? I don't like the association with the sign in process.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 27, 2020
Comment thread modules/context/context.go Outdated
Comment thread templates/base/head_navbar.tmpl Outdated
@mrsdizzie
Copy link
Copy Markdown
Member Author

@silverwind updated

@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 May 28, 2020
@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 Jun 4, 2020
@techknowlogick techknowlogick merged commit 2842f6c into go-gitea:master Jun 4, 2020
@silverwind silverwind deleted the context-link branch June 4, 2020 06:07
@silverwind silverwind restored the context-link branch June 4, 2020 06:07
ydelafollye pushed a commit to ydelafollye/gitea that referenced this pull request Jul 31, 2020
* Include query in sign in redirect

Include query string in ctx.Data["SignInLink"] so people are redirected to the right page after singing in. Fixes case of:

Visit:

 https://try.gitea.io/mrsdizzie/testcase/issues?q=&type=all&sort=&state=open&labels=7071&milestone=0&assignee=0

Sign in, then redirected to:

 https://try.gitea.io/mrsdizzie/testcase/issues

Create a new "SignInLink" instead of updating the existing "Link" because too many places in the code assume "Link" won't have a query

* Apply suggestions from code review

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
@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.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants