4797-lilium-worker-notifications #1229

Merged
pau merged 37 commits from 4797-lilium-worker-notifications into dev 2023-02-02 08:53:29 +00:00
8 changed files with 172 additions and 9 deletions

View File

@ -6,6 +6,16 @@
"table": "util.notificationAcl"
}
},
"properties":{
"notificationFk": {
"id": true,
"type": "number"
},
"roleFk":{
"id": true,
"type": "number"
}
},
"relations": {
"notification": {
"type": "belongsTo",

View File

@ -0,0 +1,62 @@
const UserError = require('vn-loopback/util/user-error');
module.exports = Self => {
Self.observe('before save', async function(ctx) {
const models = Self.app.models;
const userId = ctx.options.accessToken.userId;
const user = await ctx.instance.userFk;
const modifiedUser = await getUserToModify(null, user, models);
if (userId != modifiedUser.id && userId != modifiedUser.bossFk)
throw new UserError('You dont have permission to modify this user');
pau marked this conversation as resolved
Review

Te ahorres fer if else, fent:

        if (userId != modifiedUser.id && userId != modifiedUser.bossFk)
            throw new UserError('You dont have permission to modify this user');

I ns si faria falta ficar return al final o no

Te ahorres fer if else, fent: ``` if (userId != modifiedUser.id && userId != modifiedUser.bossFk) throw new UserError('You dont have permission to modify this user'); ``` *I ns si faria falta ficar return al final o no*
});
Self.remoteMethod('deleteNotification', {
description: 'Deletes a notification subscription',
accepts: [
{
arg: 'ctx',
type: 'object',
http: {source: 'context'}
},
{
arg: 'notificationId',
type: 'number',
required: true
},
],
returns: {
type: 'object',
root: true
},
http: {
verb: 'POST',
pau marked this conversation as resolved
Review

Quitar el parámetro authorId que lo tienes en el context

Quitar el parámetro authorId que lo tienes en el context
path: '/deleteNotification'
}
});
Self.deleteNotification = async function(ctx, notificationId) {
const models = Self.app.models;
const user = ctx.req.accessToken.userId;
const modifiedUser = await getUserToModify(notificationId, null, models);
if (user != modifiedUser.id && user != modifiedUser.bossFk)
throw new UserError('You dont have permission to modify this user');
await models.NotificationSubscription.destroyById(notificationId);
};
pau marked this conversation as resolved
Review

Self.deleteNotification = async function(ctx, userId, notificationId)

Y asi te ahorras poner

const notificationId = await ctx.args.notificationId;
const userId = await ctx.args.userId;
`Self.deleteNotification = async function(ctx, userId, notificationId)` Y asi te ahorras poner ``` const notificationId = await ctx.args.notificationId; const userId = await ctx.args.userId; ```
async function getUserToModify(notificationId, userFk, models) {
let userToModify = userFk;
if (notificationId) {
const subscription = await models.NotificationSubscription.findById(notificationId);
pau marked this conversation as resolved
Review

Pq per defecte els dos parametres son null?

Pq per defecte els dos parametres son null?
userToModify = subscription.userFk;
}
return await models.Worker.findOne({
fields: ['id', 'bossFk'],
where: {
id: userToModify
}
});
}
};

View File

@ -7,15 +7,18 @@
}
},
"properties": {
"notificationFk": {
"id": {
"type": "number",
"id": true,
"description": "Identifier"
"description": "Primary key"
},
"notificationFk": {
"type": "number",
"description": "Foreign key to Notification"
},
"userFk": {
"type": "number",
"id": true,
"description": "Identifier"
"description": "Foreign key to Account"
}
},
"relations": {

View File

@ -0,0 +1,74 @@
const models = require('vn-loopback/server/server').models;
describe('loopback model NotificationSubscription', () => {
it('Should fail to delete a notification if the user is not editing itself or a subordinate', async() => {
pau marked this conversation as resolved
Review

Poner un caso en el que te deje si eres tu mismo y otro que te deje si es tu jefe, te hará falta transaccionar la ruta para poder deshacer el delete

Poner un caso en el que te deje si eres tu mismo y otro que te deje si es tu jefe, te hará falta transaccionar la ruta para poder deshacer el delete
const tx = await models.NotificationSubscription.beginTransaction({});
try {
const options = {transaction: tx};
const user = 9;
const notificationSubscriptionId = 2;
const ctx = {req: {accessToken: {userId: user}}};
const notification = await models.NotificationSubscription.findById(notificationSubscriptionId);
let error;
try {
await models.NotificationSubscription.deleteNotification(ctx, notification.id, options);
} catch (e) {
error = e;
}
expect(error.message).toContain('You dont have permission to modify this user');
await tx.rollback();
} catch (e) {
await tx.rollback();
throw e;
}
});
it('Should delete a notification if the user is editing itself', async() => {
const tx = await models.NotificationSubscription.beginTransaction({});
try {
const options = {transaction: tx};
const user = 9;
const notificationSubscriptionId = 4;
const ctx = {req: {accessToken: {userId: user}}};
const notification = await models.NotificationSubscription.findById(notificationSubscriptionId);
await models.NotificationSubscription.deleteNotification(ctx, notification.id, options);
const deletedNotification = await models.NotificationSubscription.findById(notificationSubscriptionId);
expect(deletedNotification).toBeNull();
await tx.rollback();
} catch (e) {
await tx.rollback();
throw e;
}
});
it('Should delete a notification if the user is editing a subordinate', async() => {
const tx = await models.NotificationSubscription.beginTransaction({});
try {
const options = {transaction: tx};
const user = 9;
const notificationSubscriptionId = 5;
const ctx = {req: {accessToken: {userId: user}}};
const notification = await models.NotificationSubscription.findById(notificationSubscriptionId);
await models.NotificationSubscription.deleteNotification(ctx, notification.id, options);
const deletedNotification = await models.NotificationSubscription.findById(notificationSubscriptionId);
expect(deletedNotification).toBeNull();
await tx.rollback();
} catch (e) {
await tx.rollback();
throw e;
}
});
});

View File

@ -0,0 +1,4 @@
INSERT INTO `salix`.`ACL` (model,property,accessType,principalId)
VALUES
('NotificationSubscription','*','*','employee'),
('NotificationAcl','*','READ','employee');

View File

@ -0,0 +1,4 @@
ALTER TABLE
`util`.`notificationSubscription`
ADD
CONSTRAINT `notificationSubscription_UN` UNIQUE KEY (`notificationFk`, `userFk`);

View File

@ -0,0 +1,7 @@
ALTER TABLE `util`.`notificationSubscription`
ADD `id` int(11) auto_increment NULL,
DROP PRIMARY KEY,
ADD CONSTRAINT PRIMARY KEY (`id`);
ALTER TABLE `util`.`notificationSubscription`
ADD KEY `notificationSubscription_ibfk_1` (`notificationFk`);

View File

@ -1954,10 +1954,6 @@ INSERT INTO `vn`.`workerBusinessType` (`id`, `name`, `isFullTime`, `isPermanent`
(100, 'INDEFINIDO A TIEMPO COMPLETO', 1, 1, 1),
(109, 'CONVERSION DE TEMPORAL EN INDEFINIDO T.COMPLETO', 1, 1, 1);
INSERT INTO `vn`.`businessCategory` (`id`, `description`, `rate`)
VALUES
(1, 'basic employee', 1);
UPDATE `vn`.`business` b
SET `rate` = 7,
`workerBusinessCategoryFk` = 1,
@ -2705,7 +2701,10 @@ INSERT INTO `util`.`notificationSubscription` (`notificationFk`, `userFk`)
VALUES
(1, 1109),
(1, 1110),
(3, 1109);
(3, 1109),
(1,9),
(1,3);
INSERT INTO `vn`.`routeConfig` (`id`, `defaultWorkCenterFk`)
VALUES