Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

feat(articles): Server-side paging for list #1263

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
vm.form = {};
vm.remove = remove;
vm.save = save;
vm.isNew = !article._id;

// Remove existing Article
function remove() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,63 @@
function ArticlesAdminListController(ArticlesService) {
var vm = this;

vm.articles = ArticlesService.query();
vm.error = null;
vm.find = find;
vm.filter = filter;
vm.reset = reset;

activate();

function activate() {
vm.itemsPerPage = 10;
vm.currentPage = 1;
vm.filters = [];
vm.sorting = '-created';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why specify these here when they are only going to be plugged directly into the find() function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point is that you may have multiple views, or user inputs, that change the filters and/or sorts.

Furthermore, you could have many different clients sending API requests to the back-end with varying filters & sorts.

vm.search = null;
vm.filtered = false;

find();
}

function reset() {
// Do something here before
// we re-activate the vm.

activate();
}

function find() {
ArticlesService.find({
take: vm.itemsPerPage,
page: vm.currentPage,
filters: vm.filters,
sorting: vm.sorting
})
.then(onSearchSuccess)
.catch(onSearchError);
}

function filter() {
vm.filters = [{
title: vm.search
}];

// Start at first page for our filter
vm.currentPage = 1;
// Notify vm that a filter has been applied
vm.filtered = true;

find();
}

function onSearchSuccess(response) {
vm.error = null;
vm.totalCount = response.count;
vm.articles = response.articles;
}

function onSearchError(response) {
vm.error = response.data.message;
}
}
}());
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,35 @@
function ArticlesListController(ArticlesService) {
var vm = this;

vm.articles = ArticlesService.query();
vm.error = null;
vm.find = find;

activate();

function activate() {
vm.itemsPerPage = 20;
vm.currentPage = 1;

find();
}

function find() {
ArticlesService.list({
take: vm.itemsPerPage,
page: vm.currentPage
})
.then(onSearchSuccess)
.catch(onSearchError);
}

function onSearchSuccess(response) {
vm.error = null;
vm.totalCount = response.count;
vm.articles = response.articles;
}

function onSearchError(response) {
vm.error = response.data.message;
}
}
}());
23 changes: 23 additions & 0 deletions modules/articles/client/services/articles.client.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,18 @@
}, {
update: {
method: 'PUT'
},
query: {
isArray: false,
params: {
take: '@take',
page: '@page'
}
},
pageSortFilter: {
method: 'POST',
isArray: false,
url: 'api/parameterized-query/articles'
}
});

Expand All @@ -23,6 +35,17 @@
}
});

angular.extend(Article, {
find: function (options) {
var articlesResource = this;
return articlesResource.pageSortFilter(options).$promise;
},
list: function (params) {
var articlesResource = this;
return articlesResource.query(params).$promise;
}
});

return Article;

function createOrUpdate(article) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<h1>{{vm.article._id ? 'Edit Article' : 'New Article'}}</h1>
</div>
<div class="pull-right">
<a class="btn btn-primary" ng-click="vm.remove()">
<a class="btn btn-primary" ng-click="vm.remove()" ng-if="!vm.isNew">
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unrelated to what this PR is trying to accomplish, but I noticed a bug while developing this new feature. The "Delete" button was visible when the user was in "create" mode here.

<i class="glyphicon glyphicon-trash"></i>
</a>
</div>
Expand Down
45 changes: 38 additions & 7 deletions modules/articles/client/views/admin/list-articles.client.view.html
Original file line number Diff line number Diff line change
@@ -1,11 +1,28 @@
<section>
<div class="page-header">
<h1>
Articles
<a class="btn btn-primary pull-right" data-ui-sref="admin.articles.create">
<i class="glyphicon glyphicon-plus"></i>
</a>
</h1>
<div class="row">
<div class="col-md-12">
<h1>
Articles
<a class="btn btn-primary pull-right" data-ui-sref="admin.articles.create">
<i class="glyphicon glyphicon-plus"></i>
</a>
</h1>
</div>
<div class="row" ng-if="vm.articles.length || (vm.filtered)" ng-cloak>
<div class="col-md-4 col-sm-8 col-xs-8" style="margin-top: 2em; margin-left: 1em;">
<div class="form-group">
<input class="form-control" type="text" ng-model="vm.search" placeholder="Search by Title" />
</div>
</div>
<div class="col-md-4 col-sm-8 col-xs-8" style="margin-top: 2em; margin-left: 1em;">
<div class="form-group">
<button class="btn btn-primary" ng-disabled="!vm.search" ng-click="vm.filter();">Search</button>
<button class="btn btn-default" ng-disabled="!vm.filtered" ng-click="vm.reset();">Reset</button>
</div>
</div>
</div>
</div>
</div>
<div class="list-group">
<a data-ng-repeat="article in vm.articles" data-ui-sref="admin.articles.edit({articleId: article._id})" class="list-group-item">
Expand All @@ -20,7 +37,21 @@ <h4 class="list-group-item-heading" data-ng-bind="article.title"></h4>
<p class="list-group-item-text" data-ng-bind="article.content"></p>
</a>
</div>
<div class="alert alert-warning text-center" data-ng-if="articles.$resolved && !articles.length">
<div class="alert alert-warning text-center ng-cloak" ng-if="!vm.filtered && !vm.articles.length" ng-cloak>
No articles yet, why don't you <a data-ui-sref="admin.articles.create">create one</a>?
</div>
<div ng-show="vm.error" class="text-danger">
<strong ng-bind="vm.error"></strong>
</div>
<div class="row" ng-if="vm.articles.length">
<div class="col-md-10">
<uib-pagination boundary-links="true" max-size="10" items-per-page="vm.itemsPerPage" total-items="vm.totalCount" ng-model="vm.currentPage" ng-change="vm.find()"></uib-pagination>
</div>
<div class="col-md-2">
<p class="text-muted">Showing {{vm.articles.length}} of {{vm.totalCount}}</p>
</div>
</div>
<div class="alert alert-warning text-center" ng-if="vm.filtered && !vm.articles.length" ng-cloak>
No results found. Please provide new search criteria, or reset the form.
</div>
</section>
12 changes: 12 additions & 0 deletions modules/articles/client/views/list-articles.client.view.html
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,16 @@ <h4 class="list-group-item-heading" ng-bind="article.title"></h4>
<p class="list-group-item-text" ng-bind="article.content"></p>
</a>
</div>
<div ng-show="vm.error" class="text-danger">
<strong ng-bind="vm.error"></strong>
</div>
<div class="row">
<div class="col-md-10">
<uib-pagination boundary-links="true" max-size="10" items-per-page="vm.itemsPerPage" total-items="vm.totalCount" ng-model="vm.currentPage" ng-change="vm.find()"></uib-pagination>
</div>
<div class="col-md-2">
<p class="text-muted">Showing {{vm.articles.length}} of {{vm.totalCount}}</p>
</div>
</div>

</section>
113 changes: 103 additions & 10 deletions modules/articles/server/controllers/articles.server.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
var path = require('path'),
mongoose = require('mongoose'),
Article = mongoose.model('Article'),
errorHandler = require(path.resolve('./modules/core/server/controllers/errors.server.controller'));
errorHandler = require(path.resolve('./modules/core/server/controllers/errors.server.controller')),
MongooseFiltering = require(path.resolve('./modules/core/server/controllers/mongoose-filtering.server.controller'));

/**
* Create an article
Expand Down Expand Up @@ -77,19 +78,111 @@ exports.delete = function (req, res) {
});
};

exports.parameterizedQuery = function (req, res) {
var searchRequest = req.body;

// Add default sorting if not provided with the request
if (!searchRequest.sorting || !searchRequest.sorting.length) {
// Note: It may be a good idea to disable sorting
// on large collections due to performance issues.
// For instance, when the collection size is greater
// than 100,000. Sorting on an indexed field should
// yield good performance, even on large
// collections.
searchRequest.sorting = '_id';
}

// Set base query.
var query = Article.find();

var filterService = new MongooseFiltering(query, searchRequest);

// Build parameterized query based on the specific
// request, using the base query. The resolved
// promise will return the modified query.
filterService.pageSortFilter(true)
.then(onQueryBuildSuccess)
.catch(onQueryBuildError);

// On successful building of the parameterized query.
function onQueryBuildSuccess(result) {
query = result.query;

// Now we can add any additional querying logic
// that might be specific to this controller method

// Populate the User field
query.populate('user', 'displayName');

// Finally, execute the Mongoose find()
// method on our modified query.
query.exec('find', function (err, articles) {
if (err) {
return res.status(400).send({
message: errorHandler.getErrorMessage(err)
});
} else {
res.json({
count: result.count,
articles: articles
});
}
});
}

function onQueryBuildError(err) {
return res.status(400).send({
message: errorHandler.getErrorMessage(err)
});
}
};

/**
* List of Articles
*/
exports.list = function (req, res) {
Article.find().sort('-created').populate('user', 'displayName').exec(function (err, articles) {
if (err) {
return res.status(400).send({
message: errorHandler.getErrorMessage(err)
});
} else {
res.json(articles);
}
});
// Allows paging options to be
// sent as query-string parameters.
var searchRequest = {
take: Number(req.query.take) || null,
page: Number(req.query.page) || null
};

// Set base query
var query = Article.find();

var filterService = new MongooseFiltering(query, searchRequest);

// Build parameterized query based on the specific
// request, using the base query. The resolved
// promise will return the modified query.
filterService.pageSortFilter(true)
.then(onQueryBuildSuccess)
.catch(onQueryBuildError);

// On successful building of the parameterized query.
function onQueryBuildSuccess(result) {
query = result.query;

query.sort('-created').populate('user', 'displayName').exec('find', function (err, articles) {
if (err) {
return res.status(400).send({
message: errorHandler.getErrorMessage(err)
});
} else {
res.json({
count: result.count,
articles: articles
});
}
});
}

function onQueryBuildError(err) {
return res.status(400).send({
message: errorHandler.getErrorMessage(err)
});
}
};

/**
Expand Down
3 changes: 3 additions & 0 deletions modules/articles/server/policies/articles.server.policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ exports.invokeRolesPolicies = function () {
}, {
resources: '/api/articles/:articleId',
permissions: '*'
}, {
resources: '/api/parameterized-query/articles',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see why you called this route parameterized-query, and we probably want to keep backwards-compatibility, but it does make me wonder whether we could retitle the existing articles routes /api/articles/list and /api/articles/list/:articleId and rename your query /api/articles/query

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean, but I definitely prefer /api/articles/:articleId. It's more standard. The reason why I've used parameterized-query/articles is for two reasons. The first, you already pointed out, that there's a conflict with the /:articleId route, if I were to use articles/query. The second, is that any other collections that we may want to expose this feature to, could use the same pattern.

We may end up with something like:
/api/parameterized-query/articles
/api/parameterized-query/users
/api/parameterized-query/some-other-data-collection

I can see this type of API route configuration as a benefit, even if this may seem like a band-aid to a problem with how we're declaring our routes; given that we could have these conflicts. We can definitely look at our router configuration & solve the issue. But it's not something I wanted to do in this PR.

permissions: '*'
}]
}, {
roles: ['user'],
Expand Down
3 changes: 3 additions & 0 deletions modules/articles/server/routes/articles.server.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ module.exports = function (app) {
.get(articles.list)
.post(articles.create);

app.route('/api/parameterized-query/articles').all(articlesPolicy.isAllowed)
.post(articles.parameterizedQuery);

// Single article routes
app.route('/api/articles/:articleId').all(articlesPolicy.isAllowed)
.get(articles.read)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,10 @@

it('should send a GET request and return all articles', inject(function (ArticlesService) {
// Set POST response
$httpBackend.expectGET('api/articles').respond(mockArticleList);

$httpBackend.expectPOST('api/parameterized-query/articles').respond({
articles: mockArticleList,
count: mockArticleList.length
});

$httpBackend.flush();

Expand Down
Loading