From a318bb49527dd13f0fcecc7c500fe472ba3002ba Mon Sep 17 00:00:00 2001 From: Toby Zerner Date: Fri, 6 Feb 2015 10:30:38 +1030 Subject: [PATCH] Load discussion and posts with one request Speeds things up a heap. Also fix a whole bunch of bugs with the post stream. --- .../components/discussions/stream-content.js | 51 ++++++++----- .../components/discussions/stream-scrubber.js | 11 ++- .../core/ember/app/controllers/discussion.js | 31 +------- .../ember/app/initializers/find-query-one.js | 25 ++++++ .../core/ember/app/models/post-stream.js | 42 +++++++--- framework/core/ember/app/routes/discussion.js | 76 +++++++++++++------ framework/core/ember/app/views/discussion.js | 45 +++-------- .../core/src/Flarum/Api/Actions/Base.php | 9 ++- .../Flarum/Api/Actions/Discussions/Show.php | 32 +++++++- .../Actions/Posts/GetsPostsForDiscussion.php | 19 +++++ .../src/Flarum/Api/Actions/Posts/Index.php | 63 +++++++-------- .../Api/Serializers/DiscussionSerializer.php | 5 +- .../src/Flarum/Core/Posts/PostRepository.php | 34 +++++++++ 13 files changed, 279 insertions(+), 164 deletions(-) create mode 100644 framework/core/ember/app/initializers/find-query-one.js create mode 100644 framework/core/src/Flarum/Api/Actions/Posts/GetsPostsForDiscussion.php diff --git a/framework/core/ember/app/components/discussions/stream-content.js b/framework/core/ember/app/components/discussions/stream-content.js index 796c65918..703314ba9 100644 --- a/framework/core/ember/app/components/discussions/stream-content.js +++ b/framework/core/ember/app/components/discussions/stream-content.js @@ -24,6 +24,11 @@ export default Ember.Component.extend({ return this.get('loaded') && ! this.get('paused'); }.property('loaded', 'paused'), + refresh: function() { + this.set('paused', true); + clearTimeout(this.updateStateTimeout); + }.observes('stream'), + didInsertElement: function() { $(window).on('scroll', {view: this}, this.windowWasScrolled); }, @@ -105,41 +110,41 @@ export default Ember.Component.extend({ }, 250); }.observes('active'), - loadingNumber: function(number) { + loadingNumber: function(number, noAnimation) { // The post with this number is being loaded. We want to scroll to where // we think it will appear. We may be scrolling to the edge of the page, // but we don't want to trigger any terminal post gaps to load by doing // that. So, we'll disable the window's scroll handler for now. this.set('paused', true); - this.jumpToNumber(number); + this.jumpToNumber(number, noAnimation); }, - loadedNumber: function(number) { + loadedNumber: function(number, noAnimation) { // The post with this number has been loaded. After we scroll to this // post, we want to resume scroll events. var view = this; Ember.run.scheduleOnce('afterRender', function() { - view.jumpToNumber(number).done(function() { + view.jumpToNumber(number, noAnimation).done(function() { view.set('paused', false); }); }); }, - loadingIndex: function(index) { + loadingIndex: function(index, noAnimation) { // The post at this index is being loaded. We want to scroll to where we // think it will appear. We may be scrolling to the edge of the page, // but we don't want to trigger any terminal post gaps to load by doing // that. So, we'll disable the window's scroll handler for now. this.set('paused', true); - this.jumpToIndex(index); + this.jumpToIndex(index, noAnimation); }, - loadedIndex: function(index) { + loadedIndex: function(index, noAnimation) { // The post at this index has been loaded. After we scroll to this post, // we want to resume scroll events. var view = this; Ember.run.scheduleOnce('afterRender', function() { - view.jumpToIndex(index).done(function() { + view.jumpToIndex(index, noAnimation).done(function() { view.set('paused', false); }); }); @@ -147,7 +152,7 @@ export default Ember.Component.extend({ // Scroll down to a certain post by number (or the gap which we think the // post is in) and highlight it. - jumpToNumber: function(number) { + jumpToNumber: function(number, noAnimation) { // Clear the highlight class from all posts, and attempt to find and // highlight a post with the specified number. However, we don't apply // the highlight to the first post in the stream because it's pretty @@ -165,22 +170,26 @@ export default Ember.Component.extend({ $item = this.findNearestToNumber(number); } - return this.scrollToItem($item); + return this.scrollToItem($item, noAnimation); }, // Scroll down to a certain post by index (or the gap the post is in.) - jumpToIndex: function(index) { + jumpToIndex: function(index, noAnimation) { var $item = this.findNearestToIndex(index); - return this.scrollToItem($item); + return this.scrollToItem($item, noAnimation); }, - scrollToItem: function($item) { + scrollToItem: function($item, noAnimation) { var $container = $('html, body'); if ($item.length) { var marginTop = this.getMarginTop(); var scrollTop = $item.is(':first-child') ? 0 : $item.offset().top - marginTop; if (scrollTop !== $(document).scrollTop()) { - $container.stop(true).animate({scrollTop: scrollTop}); + if (noAnimation) { + $container.stop(true).scrollTop(scrollTop); + } else { + $container.stop(true).animate({scrollTop: scrollTop}); + } } } return $container.promise(); @@ -221,35 +230,35 @@ export default Ember.Component.extend({ }, actions: { - goToNumber: function(number) { + goToNumber: function(number, noAnimation) { number = Math.max(number, 1); // Let's start by telling our listeners that we're going to load // posts near this number. Elsewhere we will listen and // consequently scroll down to the appropriate position. - this.trigger('loadingNumber', number); + this.trigger('loadingNumber', number, noAnimation); // Now we have to actually make sure the posts around this new start // position are loaded. We will tell our listeners when they are. // Again, a listener will scroll down to the appropriate post. var controller = this; this.get('stream').loadNearNumber(number).then(function() { - controller.trigger('loadedNumber', number); + controller.trigger('loadedNumber', number, noAnimation); }); }, - goToIndex: function(index, backwards) { + goToIndex: function(index, backwards, noAnimation) { // Let's start by telling our listeners that we're going to load // posts at this index. Elsewhere we will listen and consequently // scroll down to the appropriate position. - this.trigger('loadingIndex', index); + this.trigger('loadingIndex', index, noAnimation); // Now we have to actually make sure the posts around this index // are loaded. We will tell our listeners when they are. Again, a // listener will scroll down to the appropriate post. var controller = this; this.get('stream').loadNearIndex(index, backwards).then(function() { - controller.trigger('loadedIndex', index); + controller.trigger('loadedIndex', index, noAnimation); }); }, @@ -276,4 +285,4 @@ export default Ember.Component.extend({ this.get('stream').loadRange(start, end, backwards); } } -}); \ No newline at end of file +}); diff --git a/framework/core/ember/app/components/discussions/stream-scrubber.js b/framework/core/ember/app/components/discussions/stream-scrubber.js index 29ee00631..95c855e4d 100644 --- a/framework/core/ember/app/components/discussions/stream-scrubber.js +++ b/framework/core/ember/app/components/discussions/stream-scrubber.js @@ -218,10 +218,13 @@ export default Ember.Component.extend({ // after a bunch of posts have been loaded), then we want to update the // scrubber scrollbar according to the window's current scroll position. resume: function() { - if (this.get('streamContent.active')) { - this.update(); - this.updateScrollbar(true); - } + var scrubber = this; + Ember.run.scheduleOnce('afterRender', function() { + if (scrubber.get('streamContent.active')) { + scrubber.update(); + scrubber.updateScrollbar(true); + } + }); }.observes('streamContent.active'), // Update the index/visible/description properties according to the diff --git a/framework/core/ember/app/controllers/discussion.js b/framework/core/ember/app/controllers/discussion.js index b2a3d97bf..5c26b53cf 100644 --- a/framework/core/ember/app/controllers/discussion.js +++ b/framework/core/ember/app/controllers/discussion.js @@ -1,12 +1,10 @@ import Ember from 'ember'; -import PostStream from '../models/post-stream'; import ComposerReply from '../components/discussions/composer-reply'; import ActionButton from '../components/ui/controls/action-button'; import AlertMessage from '../components/alert-message'; export default Ember.ObjectController.extend(Ember.Evented, { - needs: ['application', 'composer'], queryParams: ['start'], @@ -16,31 +14,6 @@ export default Ember.ObjectController.extend(Ember.Evented, { loaded: false, stream: null, - setup: function(discussion) { - this.set('model', discussion); - - // Set up the post stream object. It needs to know about the discussion - // its representing the posts for, and we also need to inject the Ember - // data store. - var stream = PostStream.create(); - stream.set('discussion', discussion); - stream.set('store', this.get('store')); - this.set('stream', stream); - - // Next, we need to load a list of the discussion's post IDs into the - // post stream object. If we don't already have this information, we'll - // need to reload the discussion model. - var promise = discussion.get('posts') ? Ember.RSVP.resolve(discussion) : discussion.reload(); - - // When we know we have the post IDs, we can set up the post stream with - // them. Then the view will trigger the stream to load as it sees fit. - var controller = this; - promise.then(function(discussion) { - stream.setup(discussion.get('postIds')); - controller.set('loaded', true); - }); - }, - // Save a reply. This may be called by a composer-reply component that was // set up on a different discussion, so we require a discussion model to // be explicitly passed rather than using the controller's implicit one. @@ -61,7 +34,6 @@ export default Ember.ObjectController.extend(Ember.Evented, { composer.send('hide'); discussion.setProperties({ - posts: discussion.get('posts')+','+post.get('id'), lastTime: post.get('time'), lastUser: post.get('user'), lastPost: post, @@ -74,8 +46,7 @@ export default Ember.ObjectController.extend(Ember.Evented, { // If we're currently viewing the discussion which this reply was // made in, then we can add the post to the end of the post // stream. - if (discussion == controller.get('model')) { - stream.set('ids', discussion.get('postIds')); + if (discussion == controller.get('model') && stream) { stream.addPostToEnd(post); } else { // Otherwise, we'll create an alert message to inform the user diff --git a/framework/core/ember/app/initializers/find-query-one.js b/framework/core/ember/app/initializers/find-query-one.js new file mode 100644 index 000000000..6e9adc934 --- /dev/null +++ b/framework/core/ember/app/initializers/find-query-one.js @@ -0,0 +1,25 @@ +import DS from 'ember-data'; + +// This can be removed when +// https://github.com/emberjs/data/pull/2584 is implemented. + +export default { + name: 'find-query-one', + initialize: function(container) { + DS.Store.reopen({ + findQueryOne: function(type, id, query) { + var store = this; + var typeClass = store.modelFor(type); + var adapter = store.adapterFor(typeClass); + var serializer = store.serializerFor(typeClass); + var url = adapter.buildURL(type, id); + var ajaxPromise = adapter.ajax(url, 'GET', { data: query }); + + return ajaxPromise.then(function(rawPayload) { + var extractedPayload = serializer.extract(store, typeClass, rawPayload, id, 'find'); + return store.push(typeClass, extractedPayload); + }); + } + }); + } +}; \ No newline at end of file diff --git a/framework/core/ember/app/models/post-stream.js b/framework/core/ember/app/models/post-stream.js index 1eb1c4e32..c759cbe90 100644 --- a/framework/core/ember/app/models/post-stream.js +++ b/framework/core/ember/app/models/post-stream.js @@ -15,15 +15,6 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { postLoadCount: 20, - _init: function() { - this.clear(); - }.on('init'), - - setup: function(ids) { - this.set('ids', ids); - this.get('content').objectAt(0).set('indexEnd', this.get('count') - 1); - }, - count: Ember.computed.alias('ids.length'), loadedCount: function() { @@ -40,12 +31,35 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { return last && last.content; }.property('content.@each'), + init: function() { + this._super(); + this.set('ids', Ember.A()); + this.clear(); + }, + + setup: function(ids) { + // Set our ids to the array provided and reset the content of the + // stream to a big gap that covers the amount of posts we now have. + this.set('ids', ids); + this.clear(); + + // Look in the store and see if we already have the data for any of + // these posts. If we do, we can add them to the stream. + var posts = []; + var store = this.get('store'); + ids.forEach(function(id) { + if (store.hasRecordForId('post', id)) { + posts.pushObject(store.getById('post', id)); + } + }); + this.addPosts(posts); + }, + // Clear the contents of the post stream, resetting it to one big gap. clear: function() { var content = Ember.A(); content.clear().pushObject(this.makeItem(0, this.get('count') - 1).set('loading', true)); this.set('content', content); - this.set('ids', Ember.A()); }, loadRange: function(start, end, backwards) { @@ -124,9 +138,12 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { this.trigger('postsLoaded', posts); var stream = this; + var content = this.get('content'); + content.beginPropertyChanges(); posts.forEach(function(post) { stream.addPost(post); }); + content.endPropertyChanges(); this.trigger('postsAdded'); }, @@ -156,6 +173,7 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { }, addPostToEnd: function(post) { + this.get('ids').pushObject(post.get('id')); var index = this.get('count') - 1; this.get('content').pushObject(this.makeItem(index, index, post)); }, @@ -171,10 +189,10 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { findNearestTo: function(index, property) { var nearestItem; this.get('content').some(function(item) { - nearestItem = item; if (item.get(property) > index) { return true; } + nearestItem = item; }); return nearestItem; }, @@ -184,6 +202,6 @@ export default Ember.ArrayProxy.extend(Ember.Evented, { }, findNearestToIndex: function(index) { - return this.findNearestTo(index, 'indexEnd'); + return this.findNearestTo(index, 'indexStart'); } }); diff --git a/framework/core/ember/app/routes/discussion.js b/framework/core/ember/app/routes/discussion.js index a818d4934..0731f0c77 100644 --- a/framework/core/ember/app/routes/discussion.js +++ b/framework/core/ember/app/routes/discussion.js @@ -1,13 +1,22 @@ import Ember from 'ember'; -export default Ember.Route.extend({ +import PostStream from '../models/post-stream'; +export default Ember.Route.extend({ queryParams: { start: {replace: true} }, model: function(params) { - return this.store.find('discussion', params.id); + // Each time we view a discussion we want to reload its posts from + // scratch so that we have the most up-to-date data. Also, if we were + // to leave them in the store, the stream would try and render them + // which has the potential to be slow. + this.store.unloadAll('post'); + return this.store.findQueryOne('discussion', params.id, { + include: 'posts', + near: params.start + }); }, resetController: function(controller) { @@ -19,18 +28,42 @@ export default Ember.Route.extend({ controller.set('stream', null); }, - setupController: function(controller, model) { - controller.setup(model); + setupController: function(controller, discussion) { + controller.set('model', discussion); - // Tell the discussions controller that the discussions list should be - // displayed as a pane, hidden on the side of the screen. Also set the - // application back button's target as the discussions controller. - this.controllerFor('index').set('paned', true); - this.controllerFor('application').set('backButtonTarget', this.controllerFor('index')); + // Set up the post stream object. It needs to know about the discussion + // it's representing the posts for, and we also need to inject the Ember + // Data store. + var stream = PostStream.create({ + discussion: discussion, + store: this.store + }); + controller.set('stream', stream); + + // Next, we need to make sure we have a list of the discussion's post + // IDs. If we don't already have this information, we'll need to + // reload the discussion model. + var promise = discussion.get('posts') ? Ember.RSVP.resolve(discussion) : this.model({ + id: discussion.get('id'), + start: controller.get('start') + }); + + // When we know we have the post IDs, we can set up the post stream with + // them. Then we will tell the view that we have finished loading so that + // it can scroll down to the appropriate post. + promise.then(function(discussion) { + stream.setup(discussion.get('postIds')); + controller.store.push('discussion', {id: discussion.get('id'), posts: ''}); + if (controller.get('model') === discussion) { + controller.set('loaded', true); + Ember.run.scheduleOnce('afterRender', function() { + controller.trigger('loaded'); + }); + } + }); }, actions: { - queryParamsDidChange: function(params) { // If the ?start param has changed, we want to tell the view to // tell the streamContent component to jump to this start point. @@ -39,26 +72,25 @@ export default Ember.Route.extend({ // queryParamsDidChange is fired before the controller is reset. // Thus, controller.loaded would still be true and the // startWasChanged event would be triggered inappropriately. - var controller = this.get('controller'), - oldStart = parseInt(this.get('controller.start')); + var newStart = parseInt(params.start) || 1; + var controller = this.controllerFor('discussion'); + var oldStart = parseInt(controller.get('start')); Ember.run.next(function() { - if (! params.start || ! controller || ! controller.get('loaded') || parseInt(params.start) === oldStart) { - return; + if (controller.get('loaded') && newStart !== oldStart) { + controller.trigger('startWasChanged', newStart); } - controller.trigger('startWasChanged', params.start); }); }, - willTransition: function() { + didTransition: function() { // When we transition into a new discussion, we want to hide the // discussions list pane. This means that when the user selects a // different discussion within the pane, the pane will slide away. - this.controllerFor('index').set('paneShowing', false); + // We also minimize the composer. + this.controllerFor('index') + .set('paned', true) + .set('paneShowing', false); + this.controllerFor('composer').send('minimize'); }, - - didTransition: function() { - this.controllerFor('composer').send('minimize'); - } - } }); diff --git a/framework/core/ember/app/views/discussion.js b/framework/core/ember/app/views/discussion.js index c9585f749..59da3d6ea 100644 --- a/framework/core/ember/app/views/discussion.js +++ b/framework/core/ember/app/views/discussion.js @@ -11,58 +11,37 @@ export default Ember.View.extend(Ember.Evented, { sidebarItems: null, - loadingNumber: false, - didInsertElement: function() { // Create and populate an array of items to be rendered in the sidebar. var sidebarItems = TaggedArray.create(); this.trigger('populateSidebar', sidebarItems); this.set('sidebarItems', sidebarItems); - // By this stage the discussion controller has initialized the post - // stream object, and there may or may not be posts loaded into it. - // Either way, we want to tell our stream content component to jump - // down to the start position specified in the controller's query - // params. - this.loadStreamContentForNewDiscussion(); - - // For that matter, whenever the controller's start query param - // changes, we want to tell our stream content component to jump down - // to it. - this.get('controller').on('startWasChanged', this, this.goToNumber); - this.get('streamContent').on('loadedNumber', this, this.loadedNumber); + this.get('controller').on('loaded', this, this.loaded); + this.get('controller').on('startWasChanged', this, this.startWasChanged); }, willDestroyElement: function() { - this.get('controller').off('startWasChanged', this, this.goToNumber); - this.get('streamContent').off('loadedNumber', this, this.loadedNumber); + this.get('controller').off('loaded', this, this.loaded); + this.get('controller').off('startWasChanged', this, this.startWasChanged); }, - goToNumber: function(start) { - // We can only proceed if the controller has loaded the discussion - // details and the view has been rendered. - if (this.get('controller.loaded') && this.get('streamContent') && ! this.get('loadingNumber')) { - this.get('streamContent').send('goToNumber', start); - this.set('loadingNumber', true); - } + // When the controller has finished loading, we want to scroll down to the + // appropriate post instantly (without animation). + loaded: function() { + this.get('streamContent').send('goToNumber', this.get('controller.start'), true); }, - loadedNumber: function() { - this.set('loadingNumber', false); + // When the start position of the discussion changes, we want to scroll + // down to the appropriate post. + startWasChanged: function(start) { + this.get('streamContent').send('goToNumber', start); }, // ------------------------------------------------------------------------ // OBSERVERS // ------------------------------------------------------------------------ - // Whenever the controller has switched out the old discussion model for a - // new one, we want to begin loading posts according to the ?start param. - loadStreamContentForNewDiscussion: function() { - if (this.get('controller.loaded')) { - this.goToNumber(this.get('controller.start')); - } - }.observes('controller.loaded'), - // Whenever the model's title changes, we want to update that document's // title the reflect the new title. updateTitle: function() { diff --git a/framework/core/src/Flarum/Api/Actions/Base.php b/framework/core/src/Flarum/Api/Actions/Base.php index a90b75602..df43dc5d4 100644 --- a/framework/core/src/Flarum/Api/Actions/Base.php +++ b/framework/core/src/Flarum/Api/Actions/Base.php @@ -26,7 +26,7 @@ abstract class Base extends Controller abstract protected function run(); - public function handle($request, $parameters) + public function handle($request, $parameters = []) { $this->registerErrorHandlers(); @@ -39,6 +39,11 @@ abstract class Base extends Controller return $this->run(); } + public function setRequest($request) + { + $this->request = $request; + } + public function param($key, $default = null) { return array_get($this->parameters, $key, $default); @@ -74,7 +79,7 @@ abstract class Base extends Controller protected function included($available) { $requested = explode(',', $this->input('include')); - return array_intersect($available, $requested); + return array_intersect((array) $available, $requested); } protected function explodeIds($ids) diff --git a/framework/core/src/Flarum/Api/Actions/Discussions/Show.php b/framework/core/src/Flarum/Api/Actions/Discussions/Show.php index 3b6e1238a..a5ad5e6d0 100644 --- a/framework/core/src/Flarum/Api/Actions/Discussions/Show.php +++ b/framework/core/src/Flarum/Api/Actions/Discussions/Show.php @@ -1,11 +1,32 @@ posts = $posts; + } + /** * Show a single discussion. * @@ -13,10 +34,19 @@ class Show extends Base */ protected function run() { - $include = $this->included(['startPost', 'lastPost']); + $include = $this->included(['startPost', 'lastPost', 'posts']); $discussion = Discussion::whereCanView()->findOrFail($this->param('id')); + if (in_array('posts', $include)) { + $relations = ['user', 'user.groups', 'editUser', 'deleteUser']; + $discussion->posts = $this->getPostsForDiscussion($this->posts, $discussion->id, $relations); + + $include = array_merge($include, array_map(function ($relation) { + return 'posts.'.$relation; + }, $relations)); + } + // Set up the discussion serializer, which we will use to create the // document's primary resource. As well as including the requested // relations, we will specify that we want the 'posts' relation to be diff --git a/framework/core/src/Flarum/Api/Actions/Posts/GetsPostsForDiscussion.php b/framework/core/src/Flarum/Api/Actions/Posts/GetsPostsForDiscussion.php new file mode 100644 index 000000000..809f2bc7d --- /dev/null +++ b/framework/core/src/Flarum/Api/Actions/Posts/GetsPostsForDiscussion.php @@ -0,0 +1,19 @@ +sort(['time']); + $count = $this->count(20, 50); + + if (($near = $this->input('near')) > 1) { + $start = $repository->getIndexForNumber($discussionId, $near); + $start = max(0, $start - $count / 2); + } else { + $start = 0; + } + + return $repository->findByDiscussion($discussionId, $relations, $sort['by'], $sort['order'] ?: 'asc', $count, $start); + } +} diff --git a/framework/core/src/Flarum/Api/Actions/Posts/Index.php b/framework/core/src/Flarum/Api/Actions/Posts/Index.php index 862e71dbb..8385fe458 100644 --- a/framework/core/src/Flarum/Api/Actions/Posts/Index.php +++ b/framework/core/src/Flarum/Api/Actions/Posts/Index.php @@ -2,11 +2,31 @@ use Illuminate\Database\Eloquent\ModelNotFoundException; use Flarum\Core\Posts\Post; +use Flarum\Core\Posts\PostRepository; use Flarum\Api\Actions\Base; use Flarum\Api\Serializers\PostSerializer; class Index extends Base { + use GetsPostsForDiscussion; + + /** + * The post repository. + * + * @var PostRepository + */ + protected $posts; + + /** + * Instantiate the action. + * + * @param PostRepository $posts + */ + public function __construct(PostRepository $posts) + { + $this->posts = $posts; + } + /** * Show posts from a discussion, or by providing an array of IDs. * @@ -14,47 +34,16 @@ class Index extends Base */ protected function run() { - $discussionId = $this->input('discussions'); $postIds = (array) $this->input('ids'); + $include = ['user', 'user.groups', 'editUser', 'deleteUser']; - $count = $this->count(20, 50); - - if ($near = $this->input('near')) { - // fetch the nearest post - $post = Post::orderByRaw('ABS(CAST(number AS SIGNED) - ?)', [$near])->whereNotNull('number')->where('discussion_id', $discussionId)->take(1)->first(); - - $start = max( - 0, - Post::whereCanView() - ->where('discussion_id', $discussionId) - ->where('time', '<=', $post->time) - ->count() - round($count / 2) - ); - } else { - $start = $this->start(); - } - - $include = $this->included([]); - $sort = $this->sort(['time']); - - $relations = array_merge(['user', 'user.groups', 'editUser', 'deleteUser'], $include); - - // @todo move to post repository - $posts = Post::with($relations) - ->whereCanView(); - - if ($discussionId) { - $posts->where('discussion_id', $discussionId); - } if (count($postIds)) { - $posts->whereIn('id', $postIds); + $posts = $this->posts->findMany($postIds, $include); + } else { + $discussionId = $this->input('discussions'); + $posts = $this->getPostsForDiscussion($this->posts, $discussionId, $include); } - $posts = $posts->skip($start) - ->take($count) - ->orderBy($sort['by'], $sort['order'] ?: 'asc') - ->get(); - if (! count($posts)) { throw new ModelNotFoundException; } @@ -62,7 +51,7 @@ class Index extends Base // Finally, we can set up the post serializer and use it to create // a post resource or collection, depending on how many posts were // requested. - $serializer = new PostSerializer($relations); + $serializer = new PostSerializer($include); $this->document->setPrimaryElement($serializer->collection($posts)); return $this->respondWithDocument(); diff --git a/framework/core/src/Flarum/Api/Serializers/DiscussionSerializer.php b/framework/core/src/Flarum/Api/Serializers/DiscussionSerializer.php index 67032adde..764e23358 100644 --- a/framework/core/src/Flarum/Api/Serializers/DiscussionSerializer.php +++ b/framework/core/src/Flarum/Api/Serializers/DiscussionSerializer.php @@ -58,7 +58,8 @@ class DiscussionSerializer extends DiscussionBasicSerializer } /** - * Get a collection containing a discussion's viewable posts. + * Get a collection containing a discussion's viewable posts. Assumes that + * the discussion model's posts attributes has been filled. * * @param Discussion $discussion * @param array $relations @@ -66,7 +67,7 @@ class DiscussionSerializer extends DiscussionBasicSerializer */ public function includePosts(Discussion $discussion, $relations) { - return (new PostSerializer($relations))->collection($discussion->posts()->whereCanView()->get()); + return (new PostSerializer($relations))->collection($discussion->posts); } /** diff --git a/framework/core/src/Flarum/Core/Posts/PostRepository.php b/framework/core/src/Flarum/Core/Posts/PostRepository.php index 4fa42333a..8718fa829 100755 --- a/framework/core/src/Flarum/Core/Posts/PostRepository.php +++ b/framework/core/src/Flarum/Core/Posts/PostRepository.php @@ -18,6 +18,40 @@ class PostRepository return $query->findOrFail($id); } + public function getIndexForNumber($discussionId, $number) + { + return Post::whereCanView() + ->where('discussion_id', $discussionId) + ->where('time', '<', function ($query) use ($discussionId, $number) { + $query->select('time') + ->from('posts') + ->where('discussion_id', $discussionId) + ->whereNotNull('number') + ->orderByRaw('ABS(CAST(number AS SIGNED) - ?)', [$number]) + ->take(1); + }) + ->count(); + } + + public function findByDiscussion($discussionId, $relations = [], $sortBy = 'time', $sortOrder = 'asc', $count = null, $start = 0) + { + return Post::with($relations) + ->whereCanView() + ->where('discussion_id', $discussionId) + ->skip($start) + ->take($count) + ->orderBy($sortBy, $sortOrder) + ->get(); + } + + public function findMany($ids, $relations = []) + { + return Post::with($relations) + ->whereCanView() + ->whereIn('id', $ids) + ->get(); + } + public function save(Post $post) { $post->assertValid();