Modify GetPost() to allow inclusion of pinned posts #113
Conversation
|
Agreed this is the best way to go, and is consistent with how we do this sort of thing in other places. Code looks good at first glance, and I'll test soon. |
| rows, err := db.Query("SELECT "+postCols+" FROM posts WHERE collection_id = ? AND pinned_position IS NULL "+timeCondition+" ORDER BY created "+order+limitStr, collID) | ||
| pinnedCondition := "" | ||
| if !includePinned { | ||
| pinnedCondition = "AND pinned_position IS NULL" |
There was a problem hiding this comment.
I think you will need a space on the end, AND pinned_position IS NULL . Otherwise it will run into timeCondition.
There was a problem hiding this comment.
The space is included in the query string on line 1096. :) It's easy enough to move it into pinnedCondition, though.
There was a problem hiding this comment.
oh I totally missed that. 😊
now that I look though, does white space matter here? I mean extra. for example, if you have only timeCondition would the query having two spaces cause an error?
There was a problem hiding this comment.
It doesn't. You can see in the deleted row that it's possible to have a double space if timeCondition is empty, and SQL doesn't seem to care at all. :)
|
Perfect, thanks! |
This PR modifies the
GetPost()function in database.go to add an additional parameter,includePinned. WhenincludePinnedisfalse, the SQL query to fetch posts will include the clauseAND pinned_position IS NULL; whenincludePinnedistrue, that clause is left out.In addition, this PR updates all calls to
GetPost()to include an argument for the new position. Currently this isfalseeverywhere but export.go, where the argument istrueto ensure that all posts (including pinned posts) are exported as JSON.I'm going with this one rather than
fix-json-exportbecause it's easier to maintain in the future (since we won't have two copies of a largely-identical function to manage).This should resolve #112.