Fix ref links in issue overviews for tags #8742

vijfhoek · 2019-10-30T02:22:31Z

I changed Issue.Ref to store the actual ref instead of (what Gitea assumes to be) the branch name, and modified all templates to deal with this. This ultimately fixes #8085.

To do this, I created a few helper functions to generate the URL (and updated SlackLinkToRef to them, since that's where I got them from).

This is my first PR for Gitea, so I hope I did not make any dumb mistakes.

Tags used to not generate correct URLs (src/branch/tags/1.0.0 instead of src/tags/1.0.0). Also cleans up some code around it with the created helper functions.

codecov-io · 2019-10-30T02:37:09Z

Codecov Report

❗ No coverage uploaded for pull request base (master@d338e82). Click here to learn what that means.
The diff coverage is 67.56%.

@@            Coverage Diff            @@
##             master    #8742   +/-   ##
=========================================
  Coverage          ?   41.13%           
=========================================
  Files             ?      549           
  Lines             ?    71450           
  Branches          ?        0           
=========================================
  Hits              ?    29393           
  Misses            ?    38345           
  Partials          ?     3712

Impacted Files	Coverage Δ
models/migrations/migrations.go	`1.3% <ø> (ø)`
modules/webhook/slack.go	`0.94% <0%> (ø)`
models/migrations/v109.go	`0% <0%> (ø)`
routers/repo/issue.go	`34.56% <100%> (ø)`
modules/git/utils.go	`79.41% <100%> (ø)`
routers/user/home.go	`53.46% <100%> (ø)`
services/issue/issue.go	`31.76% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d338e82...d719cd6. Read the comment docs.

models/issue.go

modules/git/utils_test.go

models/migrations/v105.go

…ed-link-tags

vijfhoek · 2020-05-10T21:58:19Z

Could someone restart the test-mysql8 test? It seems to have failed randomly

lafriks · 2020-05-10T22:05:28Z

Restarted

6543

prefent Truncated incorrect DOUBLE value: 'refs/heads/' if migration is running twice (this should not happen but if you downgrade without restore the database it will

…ed-link-tags

vijfhoek · 2020-05-11T10:30:02Z

The XSS test failed again...

lafriks · 2020-05-11T12:04:24Z

@lunny need your approval

…ed-link-tags

vijfhoek · 2020-05-14T22:15:43Z

Any updates?

dismissing review

services/issue/issue_test.go

modules/git/utils_test.go

techknowlogick · 2020-05-14T22:55:31Z

ping LG-TM

kolaente · 2020-05-15T10:43:54Z

Looks like this breaks mysql installations:

2020/05/15 10:37:25 ...ations/migrations.go:298:Migrate() [I] Migration[139]: prepend refs/heads/ to issue refs
2020/05/15 10:37:25 routers/init.go:74:initDBEngine() [E] ORM engine initialization attempt #2/10 failed. Error: migrate: do migrate: Error 1292: Truncated incorrect DOUBLE value: 'refs/heads/'
2020/05/15 10:37:25 routers/init.go:75:initDBEngine() [I] Backing off for 3 seconds
2020/05/15 10:37:28 routers/init.go:68:initDBEngine() [I] ORM engine initialization attempt #3/10...
2020/05/15 10:37:28 ...rm/session_schema.go:25:Ping() [I] PING DATABASE mysql

kolaente · 2020-05-15T10:47:05Z

I was able to fix it in my installation by running

UPDATE `issue` SET `ref` = CONCAT('refs/heads/', `ref`) WHERE `ref` IS NOT NULL AND `ref` <> '' AND `ref` NOT LIKE 'refs/%';

I'll send a pr.-

kolaente · 2020-05-15T10:53:35Z

PR is up: #11419

The migration introduced in #8742 breaks mysql installations. This pr fixes that by correctly using CONCAT. Signed-off-by: kolaente <[email protected]>

* Properly generate ref URLs Tags used to not generate correct URLs (src/branch/tags/1.0.0 instead of src/tags/1.0.0). Also cleans up some code around it with the created helper functions. * Fix formatting and create migration * Add copyright head to utils_test * Use a raw query for the ref migration * Remove semicolon * Quote column and table names in migration SQL * Change || to CONCAT, since MSSQL does not support || * Make migration engine aware * Add missing import * Move ref EndName and URL to the issue service * Fix tests * Add test for commit refs * Update issue.go * Use the right command for building JavaScript bundles * Prepare for merge * Check for refs/* before prepending in migration * Update services/issue/issue_test.go * Update modules/git/utils_test.go Co-authored-by: techknowlogick <[email protected]> Co-authored-by: techknowlogick <[email protected]>

The migration introduced in go-gitea#8742 breaks mysql installations. This pr fixes that by correctly using CONCAT. Signed-off-by: kolaente <[email protected]>

vijfhoek added 2 commits October 30, 2019 01:10

Properly generate ref URLs

e882d55

Tags used to not generate correct URLs (src/branch/tags/1.0.0 instead of src/tags/1.0.0). Also cleans up some code around it with the created helper functions.

Fix formatting and create migration

1e76a26

vijfhoek changed the title ~~Related link tags~~ Fix ref links in issue overviews for tags Oct 30, 2019

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 30, 2019

lunny previously requested changes Oct 30, 2019

View reviewed changes

models/issue.go Outdated Show resolved Hide resolved

modules/git/utils_test.go Show resolved Hide resolved

lafriks reviewed Oct 30, 2019

View reviewed changes

models/migrations/v105.go Outdated Show resolved Hide resolved

vijfhoek added 6 commits October 30, 2019 17:07

Add copyright head to utils_test

daf590d

Use a raw query for the ref migration

543b287

Remove semicolon

dfaca34

Quote column and table names in migration SQL

7df5048

Change || to CONCAT, since MSSQL does not support ||

4e607fc

Make migration engine aware

dbdc39f

guillep2k reviewed Oct 31, 2019

View reviewed changes

models/migrations/v105.go Show resolved Hide resolved

vijfhoek added 2 commits October 31, 2019 14:57

Add missing import

2840c20

Merge branch 'master' of https://github.com/go-gitea/gitea into relat…

59b3a3c

…ed-link-tags

lunny added backport/v1.10 type/bug labels Nov 2, 2019

lunny modified the milestones: 1.9.6, 1.11.0 Nov 2, 2019

vijfhoek added 4 commits November 3, 2019 14:09

Move ref EndName and URL to the issue service

e69f3a9

Merge branch 'master' of https://github.com/go-gitea/gitea into relat…

8f28995

…ed-link-tags

Fix tests

0d076ed

Add test for commit refs

2eebad7

vijfhoek requested review from guillep2k, lafriks and lunny November 3, 2019 15:20

vijfhoek added 2 commits November 3, 2019 23:21

Merge branch 'master' of https://github.com/go-gitea/gitea into relat…

c543c03

…ed-link-tags

Update issue.go

28d8d8e

6543 requested changes May 11, 2020

View reviewed changes

vijfhoek added 2 commits May 11, 2020 11:58

Check for refs/* before prepending in migration

85bbb9b

Merge branch 'master' of https://github.com/go-gitea/gitea into relat…

a6f5850

…ed-link-tags

6543 approved these changes May 11, 2020

View reviewed changes

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 May 11, 2020

lafriks approved these changes May 11, 2020

View reviewed changes

Merge branch 'master' of https://github.com/go-gitea/gitea into relat…

16d4358

…ed-link-tags

Merge branch 'master' into related-link-tags

0cc3313

techknowlogick reviewed May 14, 2020

View reviewed changes

services/issue/issue_test.go Outdated Show resolved Hide resolved

modules/git/utils_test.go Outdated Show resolved Hide resolved

techknowlogick added 2 commits May 14, 2020 18:24

Update services/issue/issue_test.go

a071c26

Update modules/git/utils_test.go

a16f5e8

techknowlogick merged commit 66a9ef9 into go-gitea:master May 14, 2020

kolaente mentioned this pull request May 15, 2020

Fix issue ref migration #11419

Merged

zeripath pushed a commit that referenced this pull request May 15, 2020

Fix issue ref migration (#11419)

57217ca

The migration introduced in #8742 breaks mysql installations. This pr fixes that by correctly using CONCAT. Signed-off-by: kolaente <[email protected]>

vijfhoek deleted the related-link-tags branch May 22, 2020 15:48

go-gitea locked and limited conversation to collaborators Nov 24, 2020

Uh oh!

Fix ref links in issue overviews for tags #8742

Fix ref links in issue overviews for tags #8742

Uh oh!

Conversation

vijfhoek commented Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vijfhoek commented May 10, 2020

Uh oh!

lafriks commented May 10, 2020

Uh oh!

6543 left a comment

Choose a reason for hiding this comment

Uh oh!

vijfhoek commented May 11, 2020

Uh oh!

lafriks commented May 11, 2020

Uh oh!

vijfhoek commented May 14, 2020

Uh oh!

Uh oh!

Uh oh!

techknowlogick commented May 14, 2020

Uh oh!

kolaente commented May 15, 2020

Uh oh!

kolaente commented May 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kolaente commented May 15, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

vijfhoek commented Oct 30, 2019 •

edited

Loading

codecov-io commented Oct 30, 2019 •

edited

Loading

kolaente commented May 15, 2020 •

edited

Loading