4797-lilium-worker-notifications #1229
Labels
No Milestone
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: verdnatura/salix#1229
Loading…
Reference in New Issue
No description provided.
Delete Branch "4797-lilium-worker-notifications"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
@ -0,0 +37,4 @@
myOptions.transaction = tx;
}
Poner validacion del front aqui tambien
@ -9,7 +9,6 @@
"properties": {
"notificationFk": {
"type": "number",
"id": true,
Revisar que se tenga que quitar o dejar sin
Estaba asi para poder borrar por id, cambiado para que ambos tengan id pues ya no se borra asi
@ -0,0 +7,4 @@
let modifiedUser = await models.Worker.findOne({
fields: ['id', 'bossFk'],
where: {
id: ctx.instance.userFk
Pasar a funcion la parte de comprobar el usuario, ya que se repite en el before y en la ruta de delete
@ -0,0 +14,4 @@
});
Self.remoteMethod('deleteNotification', {
description: 'Gets the current user data',
Cambiar descripción
@ -0,0 +44,4 @@
}
});
Self.deleteNotification = async function(ctx) {
Self.deleteNotification = async function(ctx, userId, notificationId)
Y asi te ahorras poner
@ -0,0 +46,4 @@
Self.deleteNotification = async function(ctx) {
const models = Self.app.models;
const user = await ctx.args.authorId;
No seria mejor utilizar ctx.req.accessToken.userId y no pasarlo por parametro?
@ -0,0 +22,4 @@
http: {source: 'context'}
},
{
arg: 'userId',
Renombrar a userFk. De todas formas, se podría evitar pasarle este parámetro, si en vez de especificar el id de la notificación genérica, se especificara el id de la notificación del usuario (Actualmente no existe el campo id autoincremental en la tabla notificationSubscription, pero creo que hubiese sido lo ideal)
@ -0,0 +30,4 @@
type: 'number'
},
{
arg: 'authorId',
Quitar el parámetro authorId que lo tienes en el context
@ -0,0 +48,4 @@
const models = Self.app.models;
const user = await ctx.args.authorId;
const notificationId = await ctx.args.notificationId;
const userId = await ctx.args.userId;
Además de lo que comenta alex, quitar el await.
@ -0,0 +8,4 @@
const modifiedUser = await getUserToModify(null, user, models);
if (userId == modifiedUser.id || userId == modifiedUser.bossFk)
return;
Te ahorres fer if else, fent:
I ns si faria falta ficar return al final o no
@ -19726,9 +19726,10 @@ DROP TABLE IF EXISTS `notificationSubscription`;
/*!40101 SET @saved_cs_client = @@character_set_client */;
/*!40101 SET character_set_client = utf8 */;
CREATE TABLE `notificationSubscription` (
`Id` int(11) NOT NULL AUTO_INCREMENT,
Minúscula
@ -19729,3 +19730,3 @@
`notificationFk` int(11) NOT NULL,
`userFk` int(10) unsigned NOT NULL,
PRIMARY KEY (`notificationFk`,`userFk`),
PRIMARY KEY (`Id`),
Minúscula
@ -19730,3 +19731,3 @@
`userFk` int(10) unsigned NOT NULL,
PRIMARY KEY (`notificationFk`,`userFk`),
PRIMARY KEY (`Id`),
KEY `notificationSubscription_ibfk_2` (`userFk`),
Hace falta añadir
KEY
notificationSubscription_ibfk_1(
notificationFk)
?@ -0,0 +49,4 @@
throw new UserError('You dont have permission to modify this user');
};
async function getUserToModify(notificationId = null, userFk = null, models) {
Pq per defecte els dos parametres son null?
@ -0,0 +42,4 @@
const user = ctx.req.accessToken.userId;
const modifiedUser = await getUserToModify(notificationId, null, models);
if (user == modifiedUser.id || modifiedUser.bossFk == user) {
Te ahorres fer if else, fent:
@ -0,0 +40,4 @@
Self.deleteNotification = async function(ctx, notificationId) {
const models = Self.app.models;
const user = ctx.req.accessToken.userId;
const modifiedUser = await getUserToModify(notificationId, null, models);
No estas comprobando el usuario
No se comprueba el usuario porque se le pasa un id de notificationSubscription ya existente, por lo cual dentro del metodo getUserToModify se cumpliria el requisito del if-else y se realizaria la consulta en la que se sacaria a que usuario pertenece esa notificación
@ -0,0 +47,4 @@
};
async function getUserToModify(notificationId, userFk, models) {
if (notificationId != null) {
Provar:
let userToCheck = userFk;
if (notificationId != null)
userToCheck = await models.NotificationSubscription.findById(notificationId).userFk;
Provar refactor
@ -0,0 +1,22 @@
const app = require('vn-loopback/server/server');
const models = require('vn-loopback/server/server').models;
@ -0,0 +1,22 @@
const app = require('vn-loopback/server/server');
describe('loopback model NotificationSubscription', () => {
it('Should fail to delete a notification if the user is not editing itself or a subordinate', async() => {
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
@ -0,0 +1,4 @@
INSERT INTO `salix`.`ACL` (model,property,accessType,principalId)
Cambiar a la versión 230401
@ -19726,9 +19726,11 @@ DROP TABLE IF EXISTS `notificationSubscription`;
/*!40101 SET @saved_cs_client = @@character_set_client */;
/*!40101 SET character_set_client = utf8 */;
CREATE TABLE `notificationSubscription` (
`id` int(11) NOT NULL AUTO_INCREMENT,
Esto no debería estar en el changes también???
@ -0,0 +1,4 @@
INSERT INTO `salix`.`ACL` (model,property,accessType,principalId)
VALUES ('NotificationSubscription','*','*','employee');
INSERT INTO `salix`.`ACL` (model,property,accessType,principalId)
INSERT INTO
salix
.ACL
(model,property,accessType,principalId)VALUES ('NotificationAcl','*','READ','employee');
@ -0,0 +1,10 @@
CREATE OR REPLACE TABLE `util`.`notificationSubscription` (
Has contemplat que se borren tots els registres de la taula?
@ -0,0 +2,4 @@
`id` int(11) NOT NULL AUTO_INCREMENT,
`notificationFk` int(11) NOT NULL,
`userFk` int(10) unsigned NOT NULL,
PRIMARY KEY (`id`),
Posali una UNIQUE_KEY pq al llevar PRIMARY KEY (
notificationFk
,userFk
), el mateix usuari pot estar dos vegades en la matiexa notificació i no deuria@ -0,0 +1,4 @@
ALTER TABLE
deuria anar en la 230401
@ -0,0 +1,4 @@
ALTER TABLE util.notificationSubscription
ADD Id int(11) auto_increment NULL,
id
@ -0,0 +1,4 @@
ALTER TABLE util.notificationSubscription
ADD Id int(11) auto_increment NULL,
DROP PRIMARY KEY,
ADD CONSTRAINT PRIMARY KEY (Id);
id