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

Various wiki bug fixes #2996

Merged
merged 2 commits into from Nov 28, 2017

Conversation

6 participants
@ethantkoenig
Copy link
Member

ethantkoenig commented Nov 27, 2017

This PR does the following:

  • Updates macaron
  • Fixes #2869
  • Fixes #2965
  • Fixes a bug where trying to create a wiki page with a reserved name would result in a "This page already exists" error being shown to the user. This error message is misleading, because pages with reserved names cannot exist. Instead, display a "This page name is reserved" error.
  • Adds more unit tests to models/wiki_test.go, addressing existing TODOs
@@ -32,7 +32,6 @@ func CreateTestEngine(fixturesDir string) error {
if err = x.StoreEngine("InnoDB").Sync2(tables...); err != nil {
return err
}
x.ShowSQL(true)

This comment has been minimized.

Copy link
@techknowlogick

techknowlogick Nov 27, 2017

Member

This change is from your other PR #2995 and unrelated to this PR. It is unlikely to create merge conflicts, but perhaps it should be removed just in case.

This comment has been minimized.

Copy link
@ethantkoenig

ethantkoenig Nov 27, 2017

Author Member

done

@@ -61,7 +61,7 @@ func (t *TwoFactor) getEncryptionKey() []byte {

// SetSecret sets the 2FA secret.
func (t *TwoFactor) SetSecret(secret string) error {
secretBytes, err := com.AESEncrypt(t.getEncryptionKey(), []byte(secret))

This comment has been minimized.

Copy link
@techknowlogick

techknowlogick Nov 27, 2017

Member

The changes to 2FA are unrelated to this PR, could they be pulled out an resubmitted under a diff PR?

This comment has been minimized.

Copy link
@ethantkoenig

ethantkoenig Nov 27, 2017

Author Member

They are necessary, because of the macaron update.

This comment has been minimized.

Copy link
@techknowlogick

techknowlogick Nov 27, 2017

Member

Thanks for the clarification :)

@ethantkoenig ethantkoenig force-pushed the ethantkoenig:fix/wiki branch 2 times, most recently from ab49b95 to 01d5a88 Nov 27, 2017

@lunny lunny added the kind/bug label Nov 27, 2017

@lunny lunny added this to the 1.4.0 milestone Nov 27, 2017

@@ -25,6 +25,7 @@ import (
"golang.org/x/net/html/charset"
"golang.org/x/text/transform"
"gopkg.in/editorconfig/editorconfig-core-go.v1"
"net/url"

This comment has been minimized.

Copy link
@lunny

lunny Nov 27, 2017

Member

wrong import order

This comment has been minimized.

Copy link
@ethantkoenig

ethantkoenig Nov 27, 2017

Author Member

done

@@ -85,7 +85,7 @@
{{end}}
</div>
{{if .footerPresent}}
<div class="ui grey segment">
<div class="ui segment">

This comment has been minimized.

Copy link
@lunny

lunny Nov 27, 2017

Member

why removegrey?

This comment has been minimized.

Copy link
@ethantkoenig

ethantkoenig Nov 27, 2017

Author Member

Because the footer was the only one with a grey line on the top; neither the main wiki page nor the sidebar had one, so I removed it for consistency.

@ethantkoenig ethantkoenig force-pushed the ethantkoenig:fix/wiki branch from 01d5a88 to 7d768f6 Nov 27, 2017

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Nov 27, 2017

Tests seem to fail

@ethantkoenig ethantkoenig force-pushed the ethantkoenig:fix/wiki branch from 51ec52a to fa7f540 Nov 28, 2017

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Nov 28, 2017

Codecov Report

Merging #2996 into master will increase coverage by 0.18%.
The diff coverage is 15.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2996      +/-   ##
==========================================
+ Coverage   32.53%   32.72%   +0.18%     
==========================================
  Files         267      267              
  Lines       39271    39188      -83     
==========================================
+ Hits        12778    12825      +47     
+ Misses      24700    24548     -152     
- Partials     1793     1815      +22
Impacted Files Coverage Δ
routers/routes/routes.go 86.34% <ø> (-0.03%) ⬇️
modules/templates/helper.go 52.47% <ø> (ø) ⬆️
routers/repo/wiki.go 3.05% <0%> (-0.02%) ⬇️
models/error.go 28.93% <0%> (-0.48%) ⬇️
models/twofactor.go 7.69% <0%> (ø) ⬆️
modules/context/context.go 48.97% <100%> (ø) ⬆️
models/wiki.go 52.51% <53.48%> (+40.21%) ⬆️
models/repo_indexer.go 49% <0%> (-2.98%) ⬇️
modules/indexer/repo.go 60.86% <0%> (-2.61%) ⬇️
... and 2 more

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 6a58e3f...ad14378. Read the comment docs.

@ethantkoenig ethantkoenig force-pushed the ethantkoenig:fix/wiki branch from fa7f540 to ad14378 Nov 28, 2017

@@ -608,7 +608,6 @@ func RegisterRoutes(m *macaron.Macaron) {

m.Group("/wiki", func() {
m.Get("/raw/*", repo.WikiRaw)
m.Get("/*", repo.WikiRaw)

This comment has been minimized.

Copy link
@lafriks

lafriks Nov 28, 2017

Member

Why this route is removed?

This comment has been minimized.

Copy link
@ethantkoenig

ethantkoenig Nov 28, 2017

Author Member

It cannot be reached (aside from a few corner cases), since the /:page route takes precedence.

@lunny

lunny approved these changes Nov 28, 2017

@lunny

This comment has been minimized.

Copy link
Member

lunny commented Nov 28, 2017

LGTM

@tboerger tboerger added lgtm/need 1 and removed lgtm/need 2 labels Nov 28, 2017

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Nov 28, 2017

LGTM

@tboerger tboerger added lgtm/done and removed lgtm/need 1 labels Nov 28, 2017

@lunny lunny merged commit b7ebaf6 into go-gitea:master Nov 28, 2017

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr the build was successful
Details

@ethantkoenig ethantkoenig deleted the ethantkoenig:fix/wiki branch Nov 28, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.