verdnatura/salix
verdnatura
/
salix
3
4
Fork 0
Code Issues Pull Requests 57 Releases Wiki Activity

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
Conversation 0 Commits 37 Files Changed 8 +2 -2
1 changed files with 2 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 3b69d76525 - Show all commits

4
back/models/notificationSubscription.js
View File

@ -5,7 +5,7 @@ module.exports = Self => {
const models = Self.app.models;
const userId = ctx.options.accessToken.userId;
const user = await ctx.instance.userFk;
const modifiedUser = await getUserToModify(user, models);
const modifiedUser = await getUserToModify(null, user, models);
if (userId == modifiedUser.id || userId == modifiedUser.bossFk)
pau marked this conversation as resolved Outdated
alexm commented 2022-12-27 13:50:30 +00:00
Outdated
Review
Copy Link

Pasar a funcion la parte de comprobar el usuario, ya que se repite en el before y en la ruta de delete

Pasar a funcion la parte de comprobar el usuario, ya que se repite en el before y en la ruta de delete
return;
pau marked this conversation as resolved
alexm commented 2023-01-09 08:36:35 +00:00
Review
Copy Link

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*
@ -40,7 +40,7 @@ module.exports = Self => {
Self.deleteNotification = async function(ctx, notificationId) {
const models = Self.app.models;
const user = ctx.req.accessToken.userId;
const modifiedUser = await getUserToModify(notificationId, models);
const modifiedUser = await getUserToModify(notificationId, null, models);
pau marked this conversation as resolved Outdated
alexm commented 2023-01-09 08:43:53 +00:00
Outdated
Review
Copy Link

No estas comprobando el usuario

No estas comprobando el usuario
pau commented 2023-01-18 06:40:48 +00:00
Outdated
Review
Copy Link

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

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
if (user == modifiedUser.id || modifiedUser.bossFk == user) {
pau marked this conversation as resolved Outdated
alexm commented 2023-01-09 08:40:24 +00:00
Outdated
Review
Copy Link

Te ahorres fer if else, fent:

        if (user != modifiedUser.id && user != modifiedUser.bossFk)
            throw new UserError('You dont have permission to modify this user');
            
        await models.NotificationSubscription.destroyById(notificationId);
Te ahorres fer if else, fent: ``` if (user != modifiedUser.id && user != modifiedUser.bossFk) throw new UserError('You dont have permission to modify this user'); await models.NotificationSubscription.destroyById(notificationId); ```
await models.NotificationSubscription.destroyById(notificationId);