Merge pull request '5187-error-permisos' (!1366) from 5187-error-permisos into dev

Reviewed-on: #1366
Reviewed-by: Joan Sanchez <joan@verdnatura.es>

db/changes/230601/00-acl_claim.sql

@@ -0,0 +1,6 @@
+INSERT INTO `salix`.`ACL` (model, property, accessType, permission, principalType, principalId) 
+	VALUES('ClaimBeginning', 'isEditable', 'READ', 'ALLOW', 'ROLE', 'employee');
+
+DELETE FROM `salix`.`ACL`
+	WHERE model='Claim' AND property='isEditable';
+

db/dump/fixtures.sql

@@ -1759,12 +1759,12 @@ INSERT INTO `vn`.`clientSample`(`id`, `clientFk`, `typeFk`, `created`, `workerFk
 INSERT INTO `vn`.`claimState`(`id`, `code`, `description`, `roleFk`, `priority`, `hasToNotify`)
     VALUES
         ( 1, 'pending',     'Pendiente',   1,   1,  0),
-        ( 2, 'managed',     'Gestionado',  1,   5,  0),
+        ( 2, 'managed',     'Gestionado',  72,  5,  0),
         ( 3, 'resolved',    'Resuelto',    72,  7,  0),
         ( 4, 'canceled',    'Anulado',     72,  6,  1),
-        ( 5, 'incomplete',  'Incompleta',  72,  3,  1),
-        ( 6, 'mana',        'Mana',        1,   4,  0),
-        ( 7, 'lack',        'Faltas',      1,   2,  0);
+        ( 5, 'incomplete',  'Incompleta',  1,   3,  1),
+        ( 6, 'mana',        'Mana',        72,  4,  0),
+        ( 7, 'lack',        'Faltas',      72,  2,  0);
 
 INSERT INTO `vn`.`claim`(`id`, `ticketCreated`, `claimStateFk`, `clientFk`, `workerFk`, `responsibility`, `isChargedToMana`, `created`, `packages`, `rma`)
     VALUES

loopback/locale/en.json

@@ -147,8 +147,10 @@
 	"Receipt's bank was not found": "Receipt's bank was not found",
 	"This receipt was not compensated": "This receipt was not compensated",
 	"Client's email was not found": "Client's email was not found",
-    "Tickets with associated refunds": "Tickets with associated refunds can't be deleted. This ticket is associated with refund Nº {{id}}",
+	"Tickets with associated refunds": "Tickets with associated refunds can't be deleted. This ticket is associated with refund Nº {{id}}",
 	"It is not possible to modify tracked sales": "It is not possible to modify tracked sales",
 	"It is not possible to modify sales that their articles are from Floramondo": "It is not possible to modify sales that their articles are from Floramondo",
-	"It is not possible to modify cloned sales": "It is not possible to modify cloned sales"
-}
+	"It is not possible to modify cloned sales": "It is not possible to modify cloned sales",
+	"Valid priorities: 1,2,3": "Valid priorities: 1,2,3",
+	"Tickets with associated refunds can't be deleted. This ticket is associated with refund Nº 2": "Tickets with associated refunds can't be deleted. This ticket is associated with refund Nº 2"
+}

modules/claim/back/methods/claim-state/isEditable.js

@@ -1,12 +1,12 @@
 module.exports = Self => {
     Self.remoteMethodCtx('isEditable', {
-        description: 'Check if a claim is editable',
+        description: 'Check if an state is editable',
         accessType: 'READ',
         accepts: [{
             arg: 'id',
             type: 'number',
             required: true,
-            description: 'the claim id',
+            description: 'the state id',
             http: {source: 'path'}
         }],
         returns: {
@@ -21,25 +21,18 @@ module.exports = Self => {
 
     Self.isEditable = async(ctx, id, options) => {
         const userId = ctx.req.accessToken.userId;
+        const models = Self.app.models;
         const myOptions = {};
 
         if (typeof options == 'object')
             Object.assign(myOptions, options);
-
-        const isClaimManager = await Self.app.models.Account.hasRole(userId, 'claimManager', myOptions);
-
-        const claim = await Self.app.models.Claim.findById(id, {
-            fields: ['claimStateFk'],
-            include: [{
-                relation: 'claimState'
-            }]
-        }, myOptions);
-
-        const isClaimResolved = claim && claim.claimState().code == 'resolved';
-
-        if (!claim || (isClaimResolved && !isClaimManager))
-            return false;
-
-        return true;
+    
+            const state = await models.ClaimState.findById(id, {
+                include: {
+                    relation: 'writeRole'
+                }
+            }, myOptions);
+            const roleWithGrants = state && state.writeRole().name;
+            return await models.Account.hasRole(userId, roleWithGrants, myOptions);
     };
 };

modules/claim/back/methods/claim-state/specs/isEditable.spec.js

@@ -1,16 +1,16 @@
 const app = require('vn-loopback/server/server');
 
-describe('claim isEditable()', () => {
-    const salesPerdonId = 18;
+describe('claimstate isEditable()', () => {
+    const salesPersonId = 18;
     const claimManagerId = 72;
-    it('should return false if the given claim does not exist', async() => {
+    it('should return false if the given state does not exist', async() => {
         const tx = await app.models.Claim.beginTransaction({});
 
         try {
             const options = {transaction: tx};
 
             const ctx = {req: {accessToken: {userId: claimManagerId}}};
-            const result = await app.models.Claim.isEditable(ctx, 99999, options);
+            const result = await app.models.ClaimState.isEditable(ctx, 9999, options);
 
             expect(result).toEqual(false);
 
@@ -27,8 +27,8 @@ describe('claim isEditable()', () => {
         try {
             const options = {transaction: tx};
 
-            const ctx = {req: {accessToken: {userId: salesPerdonId}}};
-            const result = await app.models.Claim.isEditable(ctx, 4, options);
+            const ctx = {req: {accessToken: {userId: salesPersonId}}};
+            const result = await app.models.ClaimState.isEditable(ctx, 3, options);
 
             expect(result).toEqual(false);
 
@@ -46,7 +46,7 @@ describe('claim isEditable()', () => {
             const options = {transaction: tx};
 
             const ctx = {req: {accessToken: {userId: claimManagerId}}};
-            const result = await app.models.Claim.isEditable(ctx, 4, options);
+            const result = await app.models.ClaimState.isEditable(ctx, 3, options);
 
             expect(result).toEqual(true);
 
@@ -63,8 +63,8 @@ describe('claim isEditable()', () => {
         try {
             const options = {transaction: tx};
 
-            const ctx = {req: {accessToken: {userId: salesPerdonId}}};
-            const result = await app.models.Claim.isEditable(ctx, 1, options);
+            const ctx = {req: {accessToken: {userId: claimManagerId}}};
+            const result = await app.models.ClaimState.isEditable(ctx, 7, options);
 
             expect(result).toEqual(true);
 

modules/claim/back/methods/claim/getSummary.js

@@ -65,7 +65,8 @@ module.exports = Self => {
             ]
         };
 
-        promises.push(Self.app.models.Claim.find(filter, myOptions));
+        const models = Self.app.models;
+        promises.push(models.Claim.find(filter, myOptions));
 
         // Claim detail
         filter = {
@@ -82,7 +83,7 @@ module.exports = Self => {
                 }
             ]
         };
-        promises.push(Self.app.models.ClaimBeginning.find(filter, myOptions));
+        promises.push(models.ClaimBeginning.find(filter, myOptions));
 
         // Claim observations
         filter = {
@@ -96,7 +97,7 @@ module.exports = Self => {
                 }
             ]
         };
-        promises.push(Self.app.models.ClaimObservation.find(filter, myOptions));
+        promises.push(models.ClaimObservation.find(filter, myOptions));
 
         // Claim developments
         filter = {
@@ -128,7 +129,7 @@ module.exports = Self => {
                 }
             ]
         };
-        promises.push(Self.app.models.ClaimDevelopment.find(filter, myOptions));
+        promises.push(models.ClaimDevelopment.find(filter, myOptions));
 
         // Claim action
         filter = {
@@ -145,11 +146,11 @@ module.exports = Self => {
                 {relation: 'claimBeggining'}
             ]
         };
-        promises.push(Self.app.models.ClaimEnd.find(filter, myOptions));
+        promises.push(models.ClaimEnd.find(filter, myOptions));
 
         const res = await Promise.all(promises);
 
-        summary.isEditable = await Self.isEditable(ctx, id, myOptions);
+        summary.isEditable = await models.ClaimState.isEditable(ctx, res[0][0].claimStateFk, myOptions);
         [summary.claim] = res[0];
         summary.salesClaimed = res[1];
         summary.observations = res[2];

modules/claim/back/methods/claim/updateClaim.js

@@ -2,6 +2,7 @@ const UserError = require('vn-loopback/util/user-error');
 module.exports = Self => {
     Self.remoteMethod('updateClaim', {
         description: 'Update a claim with privileges',
+        accessType: 'WRITE',
         accepts: [{
             arg: 'ctx',
             type: 'object',
@@ -78,11 +79,11 @@ module.exports = Self => {
 
             // Validate when claimState has been changed
             if (args.claimStateFk) {
-                const canUpdate = await canChangeState(ctx, claim.claimStateFk, myOptions);
-                const hasRights = await canChangeState(ctx, args.claimStateFk, myOptions);
+                const canEditOldState = await models.ClaimState.isEditable(ctx, claim.claimStateFk, myOptions);
+                const canEditNewState = await models.ClaimState.isEditable(ctx, args.claimStateFk, myOptions);
                 const isClaimManager = await models.Account.hasRole(userId, 'claimManager', myOptions);
 
-                if (!canUpdate || !hasRights || changedHasToPickUp && !isClaimManager)
+                if (!canEditOldState || !canEditNewState || changedHasToPickUp && !isClaimManager)
                     throw new UserError(`You don't have enough privileges to change that field`);
             }
 
@@ -113,21 +114,6 @@ module.exports = Self => {
         }
     };
 
-    async function canChangeState(ctx, id, options) {
-        let models = Self.app.models;
-        let userId = ctx.req.accessToken.userId;
-
-        let state = await models.ClaimState.findById(id, {
-            include: {
-                relation: 'writeRole'
-            }
-        }, options);
-        let stateRole = state.writeRole().name;
-        let canUpdate = await models.Account.hasRole(userId, stateRole, options);
-
-        return canUpdate;
-    }
-
     async function notifyStateChange(ctx, workerId, claim, state) {
         const models = Self.app.models;
         const origin = ctx.req.headers.origin;

modules/claim/back/models/claim-beginning.js

@@ -22,8 +22,28 @@ module.exports = Self => {
     async function claimIsEditable(ctx) {
         const loopBackContext = LoopBackContext.getCurrentContext();
         const httpCtx = {req: loopBackContext.active};
+        const models = Self.app.models;
+        const myOptions = {};
+
+        if (ctx.options && ctx.options.transaction)
+            myOptions.transaction = ctx.options.transaction;
+
         const claimBeginning = await Self.findById(ctx.where.id);
-        const isEditable = await Self.app.models.Claim.isEditable(httpCtx, claimBeginning.claimFk);
+                
+        const filter = {
+                    where: {id: claimBeginning.claimFk},
+                    include: [
+                        {
+                            relation: 'claimState',
+                            scope: {
+                                fields: ['id', 'code', 'description']
+                            }
+                        }
+                    ]
+                };
+        
+        const [claim] = await models.Claim.find(filter, myOptions);
+        const isEditable = await models.ClaimState.isEditable(httpCtx, claim.ClaimState());
 
         if (!isEditable)
             throw new UserError(`The current claim can't be modified`);

modules/claim/back/models/claim-state.js

@@ -0,0 +1,3 @@
+module.exports = Self => {
+    require('../methods/claim-state/isEditable')(Self);
+};

modules/claim/back/models/claim.js

@@ -6,7 +6,6 @@ module.exports = Self => {
     require('../methods/claim/regularizeClaim')(Self);
     require('../methods/claim/uploadFile')(Self);
     require('../methods/claim/updateClaimAction')(Self);
-    require('../methods/claim/isEditable')(Self);
     require('../methods/claim/updateClaimDestination')(Self);
     require('../methods/claim/downloadFile')(Self);
     require('../methods/claim/claimPickupPdf')(Self);

modules/claim/front/detail/index.js

@@ -151,7 +151,7 @@ class Controller extends Section {
     isClaimEditable() {
         if (!this.claim) return;
 
-        this.$http.get(`Claims/${this.claim.id}/isEditable`).then(res => {
+        this.$http.get(`ClaimStates/${this.claim.id}/isEditable`).then(res => {
             this.isRewritable = res.data;
         });
     }

modules/claim/front/detail/index.spec.js

@@ -17,7 +17,7 @@ describe('claim', () => {
             $httpBackend = _$httpBackend_;
             $httpBackend.whenGET('Claims/ClaimBeginnings').respond({});
             $httpBackend.whenGET(`Tickets/1/isEditable`).respond(true);
-            $httpBackend.whenGET(`Claims/2/isEditable`).respond(true);
+            $httpBackend.whenGET(`ClaimStates/2/isEditable`).respond(true);
             const $element = angular.element('<vn-claim-detail></vn-claim-detail>');
             controller = $componentController('vnClaimDetail', {$element, $scope});
             controller.claim = {