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 +18 -18
1 changed files with 18 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 0a602892e6 - Show all commits

36
back/models/notificationSubscription.js
View File

@ -2,14 +2,10 @@ const UserError = require('vn-loopback/util/user-error');
module.exports = Self => {
Self.observe('before save', async function(ctx) {
let models = Self.app.models;
let userId = ctx.options.accessToken.userId;
let modifiedUser = await models.Worker.findOne({
fields: ['id', 'bossFk'],
where: {
id: ctx.instance.userFk
}
});
const models = Self.app.models;
const userId = ctx.options.accessToken.userId;
const user = await ctx.instance.userFk;
const modifiedUser = await getUserToModify(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*
@ -49,16 +45,11 @@ module.exports = Self => {
});
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); ```
Self.deleteNotification = async function(ctx) {
pau marked this conversation as resolved
alexm commented 2023-01-02 07:08:03 +00:00
Review
Copy Link

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; ```
let models = Self.app.models;
let user = await ctx.args.authorId;
let notificationId = await ctx.args.notificationId;
let userId = await ctx.args.userId;
let modifiedUser = await models.Worker.findOne({
fields: ['id', 'bossFk'],
where: {
id: ctx.args.userId
}
});
const models = Self.app.models;
const user = await ctx.args.authorId;
pau marked this conversation as resolved Outdated
alexm commented 2023-01-02 07:06:40 +00:00
Outdated
Review
Copy Link

No seria mejor utilizar ctx.req.accessToken.userId y no pasarlo por parametro?

No seria mejor utilizar ctx.req.accessToken.userId y no pasarlo por parametro?
const notificationId = await ctx.args.notificationId;
pau marked this conversation as resolved Outdated
alexm commented 2023-01-18 06:59:51 +00:00
Outdated
Review
Copy Link

Provar:
let userToCheck = userFk;
if (notificationId != null)
userToCheck = await models.NotificationSubscription.findById(notificationId).userFk;

return await models.Worker.findOne({
    fields: ['id', 'bossFk'],
    where: {
        id: userToCheck
    }
});
Provar: let userToCheck = userFk; if (notificationId != null) userToCheck = await models.NotificationSubscription.findById(notificationId).userFk; return await models.Worker.findOne({ fields: ['id', 'bossFk'], where: { id: userToCheck } });
const userId = await ctx.args.userId;
pau marked this conversation as resolved Outdated
joan commented 2023-01-02 14:00:12 +00:00
Outdated
Review
Copy Link

Además de lo que comenta alex, quitar el await.

Además de lo que comenta alex, quitar el await.
const modifiedUser = await getUserToModify(userId, models);
pau marked this conversation as resolved
alexm commented 2023-01-09 08:40:07 +00:00
Review
Copy Link

Pq per defecte els dos parametres son null?

Pq per defecte els dos parametres son null?
if (user == modifiedUser.id || modifiedUser.bossFk == user) {
const query = `DELETE FROM util.notificationSubscription
@ -70,4 +61,13 @@ module.exports = Self => {
} else
throw new UserError('You dont have permission to modify this user');
};
async function getUserToModify(user, models) {
return await models.Worker.findOne({
fields: ['id', 'bossFk'],
where: {
id: user
}
});
}
};