✨ Added email.open_rate order option to posts api
#12439
Conversation
core/server/models/plugins/order.js
Outdated
| @@ -39,6 +42,15 @@ const order = function order(Bookshelf) { | |||
| field = match[1].toLowerCase(); | |||
| direction = match[2].toUpperCase(); | |||
|
|
|||
| if (orderRawAttributes && orderRawAttributes[field]) { | |||
| const {orderByRaw, eagerLoad} = orderRawAttributes[field]; | |||
| orderRaw.push(`${orderByRaw} ${direction}`); | |||
There was a problem hiding this comment.
Thinking it would probably be better to pass direction to this.orderRawAttributes() because appending it like this is unlikely to work with all possible raw order queries.
There was a problem hiding this comment.
How would the call to this.orderRawAttributes() look like? Would we be storing full orderRaw value including order inside the orderRawAttributes?
There was a problem hiding this comment.
const orderRawAttributes = this.orderRawAttributes(withRelated, direction);
... orderRawAttributes: function orderRawAttributes(withRelated, direction) {
if (withRelated && withRelated.indexOf('email') > -1) {
return {
'email.open_rate': {
orderByRaw: `emails.opened_count / emails.email_count * 100 ${direction}`,
eagerLoad: 'email.open_rate'
}
};
}
},There was a problem hiding this comment.
It's something we can change in the future if/when needed though. I was thinking ahead to having multi-clause order by queries but it's an edge case, not needed for our current use-cases.
core/server/models/post.js
Outdated
| if (withRelated && withRelated.indexOf('email') > -1) { | ||
| return { | ||
| 'email.open_rate': { | ||
| orderByRaw: 'emails.opened_count / emails.email_count * 100', |
There was a problem hiding this comment.
this orderByRaw expression will become just open_rate once the column is introduced, correct? In that case we might not even need a special "raw" clause and order by relation just like we do with post_meta fields?
There was a problem hiding this comment.
No, the column was a backup in case the on-the-fly ordering wasn't feasible.
|
Added questions about minor things (and a small broken test I used to check the functionality), otherwise LGTM! |
kevinansfield
force-pushed
the
sort-posts-by-open-rate
branch
from
ce08909
to
591505f
Compare
refs TryGhost#12420 - updated `order` bookshelf plugin's `parseOrderOption()` method to return multiple order-related properties - `order` same as before, a key-value object of property-direction - `orderRaw` new property that is a raw SQL order string generated from `orderRawQuery()` method in models - `eagerLoad` new property that is an array of properties the `eagerLoad` plugin should use to join across - updated `pagination.fetchAll()` to apply normal order + raw order if both are available and to handle eager loading / joins when `options.eagerLoad` is populated - updated post model to include details for email relationship and to add `orderRawQuery()` that allows `email.open_rate` to be used as an order option
kevinansfield
force-pushed
the
sort-posts-by-open-rate
branch
from
591505f
to
c30b04b
Compare
…porter * upstream/master: Update dependency @sentry/node to v5.29.0 v3.39.2 Updated Ghost-Admin to v3.39.2 Added guard for page.items existing in Mailgun response Added guard for page.items existing in Mailgun response v3.39.1 Updated Ghost-Admin to v3.39.1 Update dependency eslint to v7.15.0 Fixed posts without open rate being ordered in reverse 🐛 Fixed page preview Changed URL protocol in oembed acceptance tests ✨ Added `email.open_rate` order option to posts api (TryGhost#12439) Updated logging message for webhooks
refs #12420
orderbookshelf plugin'sparseOrderOption()method to return multiple order-related propertiesordersame as before, a key-value object of property-directionorderRawnew property that is a raw SQL order string generated fromorderRawAttributes()method in modelseagerLoadnew property that is an array of properties theeagerLoadplugin should use to join acrosspagination.fetchAll()to apply normal order + raw order if both are available and to handle eager loading / joins whenoptions.eagerLoadis populatedorderRawAttributes()that allowsemail.open_rateto be used as an order option