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

Fixing 404 on page view when using PostgreSQL #1849

Merged
merged 1 commit into from Jan 5, 2014

Conversation

@gimelfarb
Copy link
Contributor

@gimelfarb gimelfarb commented Jan 4, 2014

closes #1801

  • adding fixBools method to server/models/base.js to convert bools to 1/0 to be consistent with MySQL & sqlite3 data providers (based on @ErisDS recommendation)
  • this in turn fixes the check in server/controllers/frontend.js, which does an explicit post.page === 0 comparison (in pgsql this is a boolean, since the schema declares it as "bool" in server/data/schema.js, but MySQL/sqlite3 don't have concept of Boolean, only an integer or bit)
  • any model retrieved from persistence will pass through this (possible future refactoring is to combine fixBools & fixDates into one "canonicalize()" to have a single loop pass)
@gimelfarb gimelfarb mentioned this pull request Jan 4, 2014
@hswolff
Copy link
Contributor

@hswolff hswolff commented Jan 4, 2014

you're a NINJA!

🎊

@ErisDS
Copy link
Member

@ErisDS ErisDS commented Jan 5, 2014

Hi @gimelfarb thanks for taking the time to contribute to Ghost.

There will be a lot of people VERY happy that PG support is not lost in 0.4 👍

Unfortunately, your pull request doesn't currently meet our contributing guidelines.

Please review the following things:

  • incorrectly formatted commit message
@yanivtal
Copy link

@yanivtal yanivtal commented Jan 5, 2014

Ha. No wonder the Ghost commit history looks so clean. Great work @gimelfarb. Thanks!

@gimelfarb
Copy link
Contributor Author

@gimelfarb gimelfarb commented Jan 5, 2014

Guidelines, guidelines ... I seem to be not very good with those. The message is "clean" now. Not sure when you merge pull request whether it takes message from PR or from the original commit? I'll go update the commit just in case.

closes #1801
- adding fixBools method to `server/models/base.js` to convert bools to 1/0 to be consistent with MySQL & sqlite3 data providers (based on @ErisDS recommendation)
- this in turn fixes the check in `server/controllers/frontend.js`, which does an explicit `post.page === 0` comparison (in pgsql this is a `boolean`, since the schema declares it as "bool" in `server/data/schema.js`, but MySQL/sqlite3 don't have concept of Boolean, only an integer or bit)
- any model retrieved from persistence will pass through this (possible future refactoring is to combine fixBools & fixDates into one "canonicalize()" to have a single loop pass)
@ErisDS
Copy link
Member

@ErisDS ErisDS commented Jan 5, 2014

It's the commit message that is important. This is what makes GitHub link issues and prs, close an issue when I merge a PR and as @yanivtal pointed out it means we get a nice clean history :)

@gimelfarb
Copy link
Contributor Author

@gimelfarb gimelfarb commented Jan 5, 2014

I see - still getting to grips as to how GitHub works. I am all for "nice clean history" :) Ok I updated by branch which amended this PR, so now the commit message is following the guideline. Anything else I missed?

ErisDS added a commit that referenced this pull request Jan 5, 2014
Fixing 404 on page view when using PostgreSQL
@ErisDS ErisDS merged commit 28f7a7d into TryGhost:master Jan 5, 2014
1 check passed
1 check passed
@JohnONolan
default The Travis CI build passed
Details
@ErisDS
Copy link
Member

@ErisDS ErisDS commented Jan 5, 2014

Nope! All seems good. Thanks 👍

@gimelfarb gimelfarb deleted the gimelfarb-forks:fix-1801-pg-404 branch Jan 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants