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

prevent future posts from showing in pins #169

Open
wants to merge 3 commits into
base: develop
from

Conversation

@robjloranger
Copy link
Member

commented Aug 12, 2019

this changes GetPinnedPosts to accept an includeFutre bool, which
returns future dated pinned posts when true.


  • I have signed the CLA

closes #158

prevent future posts from showing in pins
this changes GetPinnedPosts to accept an includeFutre bool, which
returns future dated pinned posts when true.
if isOwner {
displayPage.PinnedPosts, _ = app.db.GetPinnedPosts(coll.CollectionObj, true)
} else {
displayPage.PinnedPosts, _ = app.db.GetPinnedPosts(coll.CollectionObj, false)

This comment has been minimized.

Copy link
@thebaer

thebaer Aug 12, 2019

Member

We can simplify this kind of logic back to one line, and just use isOwner for the second parameter.

database.go Outdated
if includeFuture {
rows, err = db.Query("SELECT id, slug, title, "+db.clip("content", 80)+", pinned_position FROM posts WHERE collection_id = ? AND pinned_position IS NOT NULL ORDER BY pinned_position ASC", coll.ID)
} else {
rows, err = db.Query("SELECT id, slug, title, "+db.clip("content", 80)+", pinned_position FROM posts WHERE collection_id = ? AND pinned_position IS NOT NULL AND created <= "+db.now()+" ORDER BY pinned_position ASC", coll.ID)

This comment has been minimized.

Copy link
@thebaer

thebaer Aug 12, 2019

Member

I think we should keep this in one db.Query() call and append the time condition if needed, exactly like we do in GetPinnedPosts(). That'll keep things consistent and help prevent potential bugs from any future changes to the query.

robjloranger added some commits Aug 12, 2019

@robjloranger

This comment has been minimized.

Copy link
Member Author

commented Aug 12, 2019

concerns addressed

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