Skip to content

fix bug on issue view when not login - #1624

Merged
lunny merged 4 commits into
go-gitea:masterfrom
lunny:lunny/fix_issue_not_login
Apr 29, 2017
Merged

fix bug on issue view when not login#1624
lunny merged 4 commits into
go-gitea:masterfrom
lunny:lunny/fix_issue_not_login

Conversation

@lunny

@lunny lunny commented Apr 27, 2017

Copy link
Copy Markdown
Member

will fix #1539

@lunny lunny added the type/bug label Apr 27, 2017
@lunny lunny added this to the 1.2.0 milestone Apr 27, 2017
Comment thread routers/repo/issue.go Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I prefer that iw would stay at nil and in template https://github.com/go-gitea/gitea/blob/master/templates/repo/issue/view_content/sidebar.tmpl#L104 the form is only display if logged or/and IssueWatch is set.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

done.

@sapk

sapk commented Apr 27, 2017

Copy link
Copy Markdown
Member

LGTM

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 27, 2017
@bkcsoft

bkcsoft commented Apr 27, 2017

Copy link
Copy Markdown
Contributor

LGTM

@tboerger tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 27, 2017
@bkcsoft

bkcsoft commented Apr 27, 2017

Copy link
Copy Markdown
Contributor

@lunny Builds fail though 😢

@sapk

sapk commented Apr 27, 2017

Copy link
Copy Markdown
Member

@lunny You need to test the creation of a issue before and next try to view this issue.

Comment thread integrations/issue_test.go Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm confusing there are two fixtures. https://github.com/go-gitea/gitea/blob/master/models/fixtures and integrations/gitea-integration/fixtures,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In your test you seem to load "code.gitea.io/gitea/models".LoadFixtures() don't know for the other fixture.

@lunny

lunny commented Apr 27, 2017

Copy link
Copy Markdown
Member Author

waiting #1627

@sapk

sapk commented Apr 27, 2017

Copy link
Copy Markdown
Member

Good idea of removing duplicate fixture.

@lunny
lunny force-pushed the lunny/fix_issue_not_login branch from 78a21a9 to 6477e37 Compare April 28, 2017 14:11
@lunny

lunny commented Apr 29, 2017

Copy link
Copy Markdown
Member Author

let L-G-T-M work

@lunny
lunny merged commit e31c02d into go-gitea:master Apr 29, 2017
@lunny
lunny deleted the lunny/fix_issue_not_login branch April 29, 2017 05:52
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
project-mirrors-bot-tu Bot pushed a commit to project-mirrors/forgejo-as-gitea-fork that referenced this pull request Sep 1, 2025
closes go-gitea#1624

Co-authored-by: Maxim Slipenko <maks1ms@altlinux.org>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8798
Reviewed-by: Otto <otto@codeberg.org>
Co-authored-by: Maxim Slipenko <maks1ms@noreply.codeberg.org>
Co-committed-by: Maxim Slipenko <maks1ms@noreply.codeberg.org>
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.

Panic in routers/repo.ViewIssue() when not logged in

5 participants