From a0491de9f99495d6686474c060fb335c2f8c648b Mon Sep 17 00:00:00 2001 From: alexandre Date: Wed, 11 Jan 2023 13:24:55 +0100 Subject: [PATCH] refs #5036 refactor transaction and fixing ticket logs --- loopback/common/models/loggable.js | 411 ++++++++++-------- loopback/util/log.js | 6 +- .../back/methods/ticket/componentUpdate.js | 17 +- 3 files changed, 246 insertions(+), 188 deletions(-) diff --git a/loopback/common/models/loggable.js b/loopback/common/models/loggable.js index 9f1cb29d1..fd355a59a 100644 --- a/loopback/common/models/loggable.js +++ b/loopback/common/models/loggable.js @@ -7,190 +7,154 @@ module.exports = function(Self) { }; Self.observe('before save', async function(ctx) { - const loopBackContext = LoopBackContext.getCurrentContext(); const models = ctx.Model.app.models; const definition = ctx.Model.definition; const Model = models[definition.name]; - const settings = definition.settings; let opts = ctx.options; if (!opts) opts = ctx.options = {}; - // Check for transactions - if (opts.txLevel) - opts.txLevel++; - else if (!opts.transaction) { - opts.txLevel = 1; - opts.transaction = await Model.beginTransaction({}); + // await txBegin(ctx); + + // try { + // await grabUserLog(ctx, 'login'); + + let oldInstance; + let newInstance; + if (ctx.data) { + const changes = pick(ctx.currentInstance, Object.keys(ctx.data)); + newInstance = ctx.data; + oldInstance = changes; + + if (ctx.where && !ctx.currentInstance) { + const fields = Object.keys(ctx.data); + + ctx.oldInstances = await Model.find({ + where: ctx.where, + fields: fields + }, opts); + } } - try { - // Check if grabUser is active - if (loopBackContext && settings.log && settings.log.grabUser) { - const userId = loopBackContext.active.accessToken.userId; - const user = await models.Account.findById(userId, {fields: ['name']}, opts); - await Model.rawSql(`CALL account.myUser_loginWithName(?)`, [user.name], opts); - } + // Get changes from created instance + if (ctx.isNewInstance) + newInstance = ctx.instance.__data; - let oldInstance; - let newInstance; - if (ctx.data) { - const changes = pick(ctx.currentInstance, Object.keys(ctx.data)); - newInstance = ctx.data; - oldInstance = changes; - - if (ctx.where && !ctx.currentInstance) { - const fields = Object.keys(ctx.data); - - ctx.oldInstances = await Model.find({ - where: ctx.where, - fields: fields - }, opts); - } - } - - // Get changes from created instance - if (ctx.isNewInstance) - newInstance = ctx.instance.__data; - - ctx.hookState.oldInstance = oldInstance; - ctx.hookState.newInstance = newInstance; - } catch (e) { - await txEnd(ctx, true); - } + ctx.hookState.oldInstance = oldInstance; + ctx.hookState.newInstance = newInstance; + // } catch (e) { + // await txEnd(ctx, true); + // } }); - async function txEnd(ctx, undoChanges) { - const opts = ctx.options || {}; - if (opts.txLevel) { - opts.txLevel--; - if (opts.txLevel === 0) { - const tx = opts.transaction; - delete opts.txLevel; - delete opts.transaction; - - if (undoChanges) - await tx.rollback(); - else - await tx.commit(); - } - } - } - Self.observe('after save', async function(ctx) { const loopBackContext = LoopBackContext.getCurrentContext(); - await logInModel(ctx, loopBackContext); - }); - - async function logInModel(ctx, loopBackContext) { const models = ctx.Model.app.models; const definition = ctx.Model.definition; - const Model = models[definition.name]; const settings = ctx.Model.definition.settings; const relations = ctx.Model.relations; let opts = ctx.options; if (!opts) opts = ctx.options = {}; - try { - let primaryKey; - for (let property in definition.properties) { - if (definition.properties[property].id) { - primaryKey = property; - break; - } + // try { + let primaryKey; + for (let property in definition.properties) { + if (definition.properties[property].id) { + primaryKey = property; + break; } - - if (!primaryKey) throw new Error('Primary key not found'); - let originId; - - // RELATIONS LOG - let changedModelId; - - if (ctx.instance && !settings.log.relation) { - originId = ctx.instance.id; - changedModelId = ctx.instance.id; - } else if (settings.log.relation) { - primaryKey = relations[settings.log.relation].keyFrom; - - if (ctx.where && ctx.where[primaryKey]) - originId = ctx.where[primaryKey]; - else if (ctx.instance) { - originId = ctx.instance[primaryKey]; - changedModelId = ctx.instance.id; - } - } else { - originId = ctx.currentInstance.id; - changedModelId = ctx.currentInstance.id; - } - - // Sets the changedModelValue to save and the instances changed in case its an updateAll - let showField = settings.log.showField; - let where; - if (showField && (!ctx.instance || !ctx.instance[showField]) && ctx.where) { - changedModelId = []; - where = []; - let changedInstances = await models[definition.name].find({ - where: ctx.where, - fields: ['id', showField, primaryKey] - }, opts); - - changedInstances.forEach(element => { - where.push(element[showField]); - changedModelId.push(element.id); - originId = element[primaryKey]; - }); - } else if (ctx.hookState.oldInstance) - where = ctx.instance[showField]; - - // Set oldInstance, newInstance, userFk and action - let oldInstance = {}; - if (ctx.hookState.oldInstance) - Object.assign(oldInstance, ctx.hookState.oldInstance); - - let newInstance = {}; - if (ctx.hookState.newInstance) - Object.assign(newInstance, ctx.hookState.newInstance); - - let userFk; - if (loopBackContext) - userFk = loopBackContext.active.accessToken.userId; - - const action = setActionType(ctx); - - removeUnloggable(definition, oldInstance); - removeUnloggable(definition, newInstance); - - oldInstance = await fkToValue(oldInstance, ctx); - newInstance = await fkToValue(newInstance, ctx); - - // Prevent log with no new changes - const hasNewChanges = Object.keys(newInstance).length; - if (!hasNewChanges) return; - - let logRecord = { - originFk: originId, - userFk: userFk, - action: action, - changedModel: definition.name, - changedModelId: changedModelId, // Model property with an different data type will throw a NaN error - changedModelValue: where, - oldInstance: oldInstance, - newInstance: newInstance - }; - - const logsToSave = setLogsToSave(where, changedModelId, logRecord, ctx); - const logModel = settings.log.model; - await models[logModel].create(logsToSave, opts); - - // Check if grabUser is active - if (settings.log && settings.log.grabUser) await Model.rawSql(`CALL account.myUser_logout()`, null, opts); - } catch (e) { - await txEnd(ctx, true); - throw e; } - await txEnd(ctx); - } + if (!primaryKey) throw new Error('Primary key not found'); + let originId; + + // RELATIONS LOG + let changedModelId; + + if (ctx.instance && !settings.log.relation) { + originId = ctx.instance.id; + changedModelId = ctx.instance.id; + } else if (settings.log.relation) { + primaryKey = relations[settings.log.relation].keyFrom; + + if (ctx.where && ctx.where[primaryKey]) + originId = ctx.where[primaryKey]; + else if (ctx.instance) { + originId = ctx.instance[primaryKey]; + changedModelId = ctx.instance.id; + } + } else { + originId = ctx.currentInstance.id; + changedModelId = ctx.currentInstance.id; + } + + // Sets the changedModelValue to save and the instances changed in case its an updateAll + let showField = settings.log.showField; + let where; + if (showField && (!ctx.instance || !ctx.instance[showField]) && ctx.where) { + changedModelId = []; + where = []; + let changedInstances = await models[definition.name].find({ + where: ctx.where, + fields: ['id', showField, primaryKey] + }, opts); + + changedInstances.forEach(element => { + where.push(element[showField]); + changedModelId.push(element.id); + originId = element[primaryKey]; + }); + } else if (ctx.hookState.oldInstance) + where = ctx.instance[showField]; + + // Set oldInstance, newInstance, userFk and action + let oldInstance = {}; + if (ctx.hookState.oldInstance) + Object.assign(oldInstance, ctx.hookState.oldInstance); + + let newInstance = {}; + if (ctx.hookState.newInstance) + Object.assign(newInstance, ctx.hookState.newInstance); + + let userFk; + if (loopBackContext) + userFk = loopBackContext.active.accessToken.userId; + + const action = setActionType(ctx); + + removeUnloggable(definition, oldInstance); + removeUnloggable(definition, newInstance); + + oldInstance = await fkToValue(oldInstance, ctx); + newInstance = await fkToValue(newInstance, ctx); + + // Prevent log with no new changes + const hasNewChanges = Object.keys(newInstance).length; + if (!hasNewChanges) return; + + let logRecord = { + originFk: originId, + userFk: userFk, + action: action, + changedModel: definition.name, + changedModelId: changedModelId, // Model property with an different data type will throw a NaN error + changedModelValue: where, + oldInstance: oldInstance, + newInstance: newInstance + }; + + const logsToSave = setLogsToSave(where, changedModelId, logRecord, ctx); + const logModel = settings.log.model; + await models[logModel].create(logsToSave, opts); + + // await grabUserLog(ctx, 'logout'); + // } catch (e) { + // await txEnd(ctx, true); + // throw e; + // } + + // await txEnd(ctx); + }); Self.observe('before delete', async function(ctx) { const models = ctx.Model.app.models; @@ -199,6 +163,11 @@ module.exports = function(Self) { let opts = ctx.options; if (!opts) opts = ctx.options = {}; + // await txBegin(ctx); + + // try { + // await grabUserLog(ctx, 'login'); + if (ctx.where) { const affectedModel = definition.name; let deletedInstances = await models[affectedModel].find({ @@ -220,43 +189,52 @@ module.exports = function(Self) { ctx.hookState.oldInstance = arrangedDeletedInstances; } } + // } catch (e) { + // await txEnd(ctx, true); + // } }); Self.observe('after delete', async function(ctx) { const loopBackContext = LoopBackContext.getCurrentContext(); - if (ctx.hookState.oldInstance) logDeletedInstances(ctx, loopBackContext); - }); - - async function logDeletedInstances(ctx, loopBackContext) { const models = ctx.Model.app.models; const definition = ctx.Model.definition; const settings = definition.settings; let opts = ctx.options; if (!opts) opts = ctx.options = {}; - ctx.hookState.oldInstance.forEach(async instance => { - let userFk; - if (loopBackContext) - userFk = loopBackContext.active.accessToken.userId; + // try { + if (ctx.hookState.oldInstance) { + ctx.hookState.oldInstance.forEach(async instance => { + let userFk; + if (loopBackContext) + userFk = loopBackContext.active.accessToken.userId; - const changedModelValue = settings.log.changedModelValue; - const logRecord = { - originFk: instance.originFk, - userFk: userFk, - action: 'delete', - changedModel: definition.name, - changedModelId: instance.id, - changedModelValue: instance[changedModelValue], - oldInstance: instance, - newInstance: {} - }; + const changedModelValue = settings.log.changedModelValue; + const logRecord = { + originFk: instance.originFk, + userFk: userFk, + action: 'delete', + changedModel: definition.name, + changedModelId: instance.id, + changedModelValue: instance[changedModelValue], + oldInstance: instance, + newInstance: {} + }; - delete instance.originFk; + delete instance.originFk; - const logModel = settings.log.model; - await models[logModel].create(logRecord, opts); - }); - } + const logModel = settings.log.model; + await models[logModel].create(logRecord, opts); + }); + } + // await grabUserLog(ctx, 'logout'); + // } catch (e) { + // await txEnd(ctx, true); + // throw e; + // } + + // await txEnd(ctx); + }); // Get log values from a foreign key async function fkToValue(instance, ctx) { @@ -381,4 +359,69 @@ module.exports = function(Self) { return 'delete'; } + + /** + * The functions txBegin and txEnd are used to transactionate the loggable. + * When a new transaction is created, they add a txLevel because in some cases + * the transactions are performed in a recursive way. + * + * The function grabUserLog check if the option grabUser is active in the Model and + * login or logout the user in the database depending on the action. + * + * Now they are commented out because the way to handle the errors + * that occur in the database that leave opened transactions has not been found. + * (https://redmine.verdnatura.es/issues/5036) + **/ + + // async function txBegin(ctx) { + // const opts = ctx.options || {}; + // const models = ctx.Model.app.models; + // const definition = ctx.Model.definition; + // const Model = models[definition.name]; + + // if (opts.txLevel) + // opts.txLevel++; + // else if (!opts.transaction) { + // opts.txLevel = 1; + // opts.transaction = await Model.beginTransaction({}); + // } + // } + + // async function txEnd(ctx, undoChanges) { + // const opts = ctx.options || {}; + + // if (opts.txLevel) { + // opts.txLevel--; + // if (opts.txLevel === 0) { + // const tx = opts.transaction; + // delete opts.txLevel; + // delete opts.transaction; + + // if (undoChanges) + // await tx.rollback(); + // else + // await tx.commit(); + // } + // } + // } + + // async function grabUserLog(ctx, logAction) { + // const opts = ctx.options || {}; + // const models = ctx.Model.app.models; + // const definition = ctx.Model.definition; + // const settings = definition.settings; + // const Model = models[definition.name]; + // const hasGrabUser = settings.log && settings.log.grabUser; + + // if (logAction === 'login' && hasGrabUser) { + // const loopBackContext = LoopBackContext.getCurrentContext(); + + // if (loopBackContext) { + // const userId = loopBackContext.active.accessToken.userId; + // const user = await models.Account.findById(userId, {fields: ['name']}, opts); + // await Model.rawSql(`CALL account.myUser_loginWithName(?)`, [user.name], opts); + // } + // } else if (logAction === 'logout' && hasGrabUser) + // await Model.rawSql(`CALL account.myUser_logout()`, null, opts); + // } }; diff --git a/loopback/util/log.js b/loopback/util/log.js index e26022c35..76e87781d 100644 --- a/loopback/util/log.js +++ b/loopback/util/log.js @@ -91,7 +91,11 @@ exports.getChanges = (original, changes) => { const isPrivate = firstChar == '$'; if (isPrivate) return; - if (changes[property] != original[property]) { + const hasChanges = original[property] instanceof Date ? + changes[property]?.getTime() != original[property]?.getTime() : + changes[property] != original[property]; + + if (hasChanges) { newChanges[property] = changes[property]; if (original[property] != undefined) diff --git a/modules/ticket/back/methods/ticket/componentUpdate.js b/modules/ticket/back/methods/ticket/componentUpdate.js index f4a4bb98d..e6725e9d2 100644 --- a/modules/ticket/back/methods/ticket/componentUpdate.js +++ b/modules/ticket/back/methods/ticket/componentUpdate.js @@ -160,18 +160,29 @@ module.exports = Self => { 'shipped', 'landed', 'isDeleted', - 'routeFk' + 'routeFk', + 'nickname' ], include: [ { relation: 'client', scope: { fields: 'salesPersonFk' - } - }] + }, + include: [ + { + relation: 'address', + scope: { + fields: 'nickname' + } + } + ] + }, + ] }, myOptions); args.routeFk = null; + if (args.isWithoutNegatives === false) delete args.isWithoutNegatives; const updatedTicket = Object.assign({}, args); delete updatedTicket.ctx; delete updatedTicket.option;