Skip to content

Commit 8ce4e3b

Browse files
authored
fix(Statement Deletion): Fixes store recounts when clients have no LRS attached (#1515 - [LL-322](https://learningpool.atlassian.net/browse/LL-322))
#1515 https://learningpool.atlassian.net/browse/LL-322
1 parent 9b59592 commit 8ce4e3b

5 files changed

Lines changed: 52 additions & 41 deletions

File tree

api/src/routes/HttpRoutes.js

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,6 @@ import personaRESTHandler from 'api/routes/personas/personaRESTHandler';
5858
import personaIdentifierRESTHandler from 'api/routes/personas/personaIdentifierRESTHandler';
5959
import UserOrganisationsRouter from 'api/routes/userOrganisations/router';
6060
import UserOrganisationSettingsRouter from 'api/routes/userOrganisationSettings/router';
61-
import getLrsFromAuthInfo from 'lib/services/auth/authInfoSelectors/getLrsFromAuthInfo';
62-
import { decrementStatementCount } from 'lib/services/lrs';
6361

6462
// CONSTANTS
6563
import * as routes from 'lib/constants/routes';
@@ -359,18 +357,8 @@ restify.serve(router, Statement, {
359357
return res.send('No ID sent', 400);
360358
}
361359
next();
362-
return;
363360
},
364361
preUpdate: (req, res) => res.sendStatus(405),
365-
postDelete: (req, _, next) => {
366-
// Update LRS.statementCount
367-
const authInfo = getAuthFromRequest(req);
368-
const lrsId = getLrsFromAuthInfo(authInfo);
369-
370-
decrementStatementCount(lrsId)
371-
.then(() => next())
372-
.catch(err => next(err));
373-
},
374362
});
375363
restify.serve(router, StatementForwarding);
376364
restify.serve(router, QueryBuilderCache);

lib/models/lrs.js

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,40 @@ import * as scopes from 'lib/constants/scopes';
99
import addCRUDFunctions from 'lib/models/plugins/addCRUDFunctions';
1010
import auditRemove from 'lib/models/plugins/auditRemove';
1111

12+
/**
13+
* Plain object structure without mongoose model methods
14+
*
15+
* @typedef {object} Lrs
16+
* @property {string} title
17+
* @property {string} description
18+
* @property {*} owner_id TODO: define type
19+
* @property {*} organisation TODO: define type
20+
* @property {number} statementCount
21+
*/
22+
23+
/** @typedef {module:mongoose.Model<Lrs>} lrsModel */
24+
25+
/**
26+
* @param {lrsModel} lrsModel
27+
* @returns {Promise<void>}
28+
*/
29+
const createDefaultClient = async (lrsModel) => {
30+
if (!lrsModel.title) {
31+
lrsModel.title = 'New xAPI store';
32+
}
33+
const ClientModel = getConnection().model('Client');
34+
const client = new ClientModel({
35+
organisation: lrsModel.organisation,
36+
lrs_id: lrsModel._id,
37+
scopes: [scopes.XAPI_ALL],
38+
title: `${lrsModel.title} client`
39+
});
40+
await client.save((err) => {
41+
assert.ifError(err);
42+
});
43+
};
44+
45+
/** @class LrsSchema */
1246
const schema = new mongoose.Schema({
1347
title: { type: String },
1448
description: { type: String },
@@ -20,22 +54,9 @@ schema.plugin(filterByOrg);
2054
schema.plugin(timestamps);
2155
schema.plugin(addCRUDFunctions);
2256

23-
schema.pre('save', function preSave(next) {
24-
// make a default client
57+
schema.pre('save', async function preSave(next) {
2558
if (this.isNew) {
26-
if (!this.title) {
27-
this.title = 'New xAPI store';
28-
}
29-
const Client = getConnection().model('Client');
30-
const client = new Client({
31-
organisation: this.organisation,
32-
lrs_id: this._id,
33-
scopes: [scopes.XAPI_ALL],
34-
title: `${this.title} client`
35-
});
36-
client.save((err) => {
37-
assert.ifError(err);
38-
});
59+
await createDefaultClient(this);
3960
}
4061
next();
4162
});
@@ -49,6 +70,10 @@ schema.plugin(auditRemove, {
4970
auditName: 'LRSAudit'
5071
});
5172

73+
/**
74+
* @param {lrsModel} lrs
75+
* @returns {Promise<void>}
76+
*/
5277
schema.statics.updateStatementCount = async (lrs) => {
5378
const Statement = getConnection().model('Statement');
5479

@@ -58,10 +83,14 @@ schema.statics.updateStatementCount = async (lrs) => {
5883
});
5984
};
6085

61-
schema.statics.decrementStatementCount = async (lrs) => {
86+
/**
87+
* @param {string} lrsId
88+
* @returns {Promise<void>}
89+
*/
90+
schema.statics.decrementStatementCount = async (lrsId) => {
6291
getConnection()
6392
.model('Lrs')
64-
.update({ _id: lrs._id }, { $inc: { statementCount: -1 } })
93+
.update({ _id: lrsId }, { $inc: { statementCount: -1 } })
6594
.exec();
6695
};
6796

lib/models/statement.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import getScopeFilter from 'lib/services/auth/filters/getScopeFilter';
2323
import filterByOrg from 'lib/models/plugins/filterByOrg';
2424
import decodeDot from 'lib/helpers/decodeDot';
2525
import logger from 'lib/logger';
26+
import Lrs from 'lib/models/lrs';
2627

2728
const ALLOW_AGGREGATION_DISK_USE = boolean(defaultTo(process.env.ALLOW_AGGREGATION_DISK_USE, true));
2829
const AGGREGATION_CACHE_SECONDS = defaultTo(Number(process.env.AGGREGATION_CACHE_SECONDS), 300);
@@ -94,6 +95,11 @@ schema.plugin(scopeChecks);
9495
schema.plugin(filterByOrg);
9596
schema.plugin(addCRUDFunctions);
9697

98+
schema.post('remove', async (statement, next) => {
99+
await Lrs.decrementStatementCount(statement.lrs_id);
100+
next();
101+
});
102+
97103
/**
98104
* @param {*} - {
99105
* pipeline

lib/services/auth/authInfoSelectors/getLrsFromAuthInfo.js

Lines changed: 0 additions & 5 deletions
This file was deleted.

lib/services/lrs.js

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,3 @@ export const updateStatementCountsInOrg = async (organisationId) => {
66
const lrsList = await Lrs.find({ organisation: organisationId });
77
await Promise.map(lrsList, Lrs.updateStatementCount);
88
};
9-
10-
export const decrementStatementCount = async (lrsId) => {
11-
const lrs = await Lrs.findOne({ _id: lrsId });
12-
if (lrs) {
13-
await Lrs.decrementStatementCount(lrs);
14-
}
15-
};

0 commit comments

Comments
 (0)