Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Statement Deletion): Fixes store recounts when clients have no LRS attached (LL-322) #1515

Merged
merged 7 commits into from
Feb 24, 2020
Merged
Show file tree
Hide file tree
Changes from 6 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
12 changes: 0 additions & 12 deletions api/src/routes/HttpRoutes.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ import personaRESTHandler from 'api/routes/personas/personaRESTHandler';
import personaIdentifierRESTHandler from 'api/routes/personas/personaIdentifierRESTHandler';
import UserOrganisationsRouter from 'api/routes/userOrganisations/router';
import UserOrganisationSettingsRouter from 'api/routes/userOrganisationSettings/router';
import getLrsFromAuthInfo from 'lib/services/auth/authInfoSelectors/getLrsFromAuthInfo';
import { decrementStatementCount } from 'lib/services/lrs';

// CONSTANTS
import * as routes from 'lib/constants/routes';
Expand Down Expand Up @@ -359,18 +357,8 @@ restify.serve(router, Statement, {
return res.send('No ID sent', 400);
}
next();
return;
},
preUpdate: (req, res) => res.sendStatus(405),
postDelete: (req, _, next) => {
// Update LRS.statementCount
const authInfo = getAuthFromRequest(req);
const lrsId = getLrsFromAuthInfo(authInfo);

decrementStatementCount(lrsId)
.then(() => next())
.catch(err => next(err));
},
});
restify.serve(router, StatementForwarding);
restify.serve(router, QueryBuilderCache);
Expand Down
63 changes: 46 additions & 17 deletions lib/models/lrs.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,40 @@ import * as scopes from 'lib/constants/scopes';
import addCRUDFunctions from 'lib/models/plugins/addCRUDFunctions';
import auditRemove from 'lib/models/plugins/auditRemove';

/**
* Plain object structure without mongoose model methods
*
* @typedef {object} Lrs
* @property {string} title
* @property {string} description
* @property {*} owner_id TODO: define type
* @property {*} organisation TODO: define type
* @property {number} statementCount
*/

/** @typedef {module:mongoose.Model<Lrs>} lrsModel */

/**
* @param {lrsModel} lrsModel
* @returns {Promise<void>}
*/
const createDefaultClient = async (lrsModel) => {
ryasmi marked this conversation as resolved.
Show resolved Hide resolved
if (!lrsModel.title) {
lrsModel.title = 'New xAPI store';
}
const ClientModel = getConnection().model('Client');
const client = new ClientModel({
organisation: lrsModel.organisation,
lrs_id: lrsModel._id,
scopes: [scopes.XAPI_ALL],
title: `${lrsModel.title} client`
});
await client.save((err) => {
assert.ifError(err);
});
};

/** @class LrsSchema */
const schema = new mongoose.Schema({
title: { type: String },
description: { type: String },
Expand All @@ -20,22 +54,9 @@ schema.plugin(filterByOrg);
schema.plugin(timestamps);
schema.plugin(addCRUDFunctions);

schema.pre('save', function preSave(next) {
// make a default client
schema.pre('save', async function preSave(next) {
if (this.isNew) {
if (!this.title) {
this.title = 'New xAPI store';
}
const Client = getConnection().model('Client');
const client = new Client({
organisation: this.organisation,
lrs_id: this._id,
scopes: [scopes.XAPI_ALL],
title: `${this.title} client`
});
client.save((err) => {
assert.ifError(err);
});
await createDefaultClient(this);
}
next();
});
Expand All @@ -49,6 +70,10 @@ schema.plugin(auditRemove, {
auditName: 'LRSAudit'
});

/**
* @param {lrsModel} lrs
* @returns {Promise<void>}
*/
schema.statics.updateStatementCount = async (lrs) => {
const Statement = getConnection().model('Statement');

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

schema.statics.decrementStatementCount = async (lrs) => {
/**
ryasmi marked this conversation as resolved.
Show resolved Hide resolved
* @param {string} lrsId
* @returns {Promise<void>}
*/
schema.statics.decrementStatementCount = async (lrsId) => {
getConnection()
.model('Lrs')
.update({ _id: lrs._id }, { $inc: { statementCount: -1 } })
.update({ _id: lrsId }, { $inc: { statementCount: -1 } })
.exec();
};

Expand Down
6 changes: 6 additions & 0 deletions lib/models/statement.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import getScopeFilter from 'lib/services/auth/filters/getScopeFilter';
import filterByOrg from 'lib/models/plugins/filterByOrg';
import decodeDot from 'lib/helpers/decodeDot';
import logger from 'lib/logger';
import Lrs from 'lib/models/lrs';

const ALLOW_AGGREGATION_DISK_USE = boolean(defaultTo(process.env.ALLOW_AGGREGATION_DISK_USE, true));
const AGGREGATION_CACHE_SECONDS = defaultTo(Number(process.env.AGGREGATION_CACHE_SECONDS), 300);
Expand Down Expand Up @@ -94,6 +95,11 @@ schema.plugin(scopeChecks);
schema.plugin(filterByOrg);
schema.plugin(addCRUDFunctions);

schema.post('remove', async (statement, next) => {
await Lrs.decrementStatementCount(statement.lrs_id);
next();
});

/**
* @param {*} - {
* pipeline
Expand Down
5 changes: 0 additions & 5 deletions lib/services/auth/authInfoSelectors/getLrsFromAuthInfo.js

This file was deleted.

7 changes: 0 additions & 7 deletions lib/services/lrs.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,3 @@ export const updateStatementCountsInOrg = async (organisationId) => {
const lrsList = await Lrs.find({ organisation: organisationId });
await Promise.map(lrsList, Lrs.updateStatementCount);
};

export const decrementStatementCount = async (lrsId) => {
const lrs = await Lrs.findOne({ _id: lrsId });
if (lrs) {
await Lrs.decrementStatementCount(lrs);
}
};