From 65981099b62c914080a0e2ecdf9872d1f8be5830 Mon Sep 17 00:00:00 2001 From: Carlos Jimenez Ruiz Date: Thu, 26 Sep 2019 09:05:18 +0200 Subject: [PATCH 1/3] #1719 claim.detail cambiar descuento falla --- e2e/helpers/selectors.js | 2 +- .../methods/sale/specs/updateConcept.spec.js | 39 +++++++++++++++++++ .../ticket/back/methods/sale/updateConcept.js | 2 +- .../ticket-tracking/specs/changeState.spec.js | 2 +- .../ticket/specs/transferSales.spec.js | 4 +- .../ticket/specs/updateEditableTicket.spec.js | 2 +- 6 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 modules/ticket/back/methods/sale/specs/updateConcept.spec.js diff --git a/e2e/helpers/selectors.js b/e2e/helpers/selectors.js index 41825e5ec..9be6bc0dd 100644 --- a/e2e/helpers/selectors.js +++ b/e2e/helpers/selectors.js @@ -309,7 +309,7 @@ export default { ticketSummary: { header: 'vn-ticket-summary > vn-card > div > h5', state: 'vn-ticket-summary vn-label-value[label="State"] > section > span', - route: 'vn-ticket-summary vn-label-value[label="Route"] > section > span', + route: 'vn-ticket-summary vn-label-value[label="Route"] > section > a', total: 'vn-ticket-summary vn-one.taxes > p:nth-child(3) > strong', sale: 'vn-ticket-summary [name="sales"] vn-table > div > vn-tbody > vn-tr', firstSaleItemId: 'vn-ticket-summary [name="sales"] vn-table > div > vn-tbody > vn-tr > vn-td:nth-child(2) > span', diff --git a/modules/ticket/back/methods/sale/specs/updateConcept.spec.js b/modules/ticket/back/methods/sale/specs/updateConcept.spec.js new file mode 100644 index 000000000..ab27bfdca --- /dev/null +++ b/modules/ticket/back/methods/sale/specs/updateConcept.spec.js @@ -0,0 +1,39 @@ +const app = require('vn-loopback/server/server'); + +describe('sale updateConcept()', () => { + const saleId = 1; + let originalSale; + + beforeAll(async done => { + originalSale = await app.models.Sale.findById(saleId); + + done(); + }); + + afterAll(async done => { + await originalSale.save(); + + done(); + }); + + + it('should throw if ID was undefined', async() => { + const newConcept = 'I am he new concept'; + + await app.models.Sale.updateConcept(undefined, newConcept) + .catch(response => { + expect(response).toEqual(new Error(`Model::findById requiere el argumento id`)); + error = response; + }); + + expect(error).toBeDefined(); + }); + + it('should update the sale concept', async() => { + const newConcept = 'I am the new concept'; + + let response = await app.models.Sale.updateConcept(saleId, newConcept); + + expect(response.concept).toEqual(newConcept); + }); +}); diff --git a/modules/ticket/back/methods/sale/updateConcept.js b/modules/ticket/back/methods/sale/updateConcept.js index 8bbe235b7..d95cf1202 100644 --- a/modules/ticket/back/methods/sale/updateConcept.js +++ b/modules/ticket/back/methods/sale/updateConcept.js @@ -25,7 +25,7 @@ module.exports = Self => { }); Self.updateConcept = async(id, newConcept) => { - let currentLine = await Self.app.models.Sale.findOne({where: {id: id}}); + let currentLine = await Self.app.models.Sale.findById(id); return await currentLine.updateAttributes({concept: newConcept}); }; diff --git a/modules/ticket/back/methods/ticket-tracking/specs/changeState.spec.js b/modules/ticket/back/methods/ticket-tracking/specs/changeState.spec.js index 74a1ebde0..3c6c51252 100644 --- a/modules/ticket/back/methods/ticket-tracking/specs/changeState.spec.js +++ b/modules/ticket/back/methods/ticket-tracking/specs/changeState.spec.js @@ -17,7 +17,7 @@ describe('ticket changeState()', () => { done(); }); - it('should throw an error if the ticket is not editable and the user isnt production', async() => { + it('should throw if the ticket is not editable and the user isnt production', async() => { let ctx = {req: {accessToken: {userId: 18}}}; let params = {ticketFk: 2, stateFk: 3}; diff --git a/modules/ticket/back/methods/ticket/specs/transferSales.spec.js b/modules/ticket/back/methods/ticket/specs/transferSales.spec.js index 6e8496f43..17996649e 100644 --- a/modules/ticket/back/methods/ticket/specs/transferSales.spec.js +++ b/modules/ticket/back/methods/ticket/specs/transferSales.spec.js @@ -8,11 +8,11 @@ describe('sale transferSales()', () => { done(); }); - it('should throw an error if the ticket is not editable', async() => { + it('should throw an error as the ticket is not editable', async() => { const ctx = {req: {accessToken: {userId: 101}}}; let error; - const currentTicketId = 10; + const currentTicketId = 1; const receiverTicketId = undefined; const sales = []; diff --git a/modules/ticket/back/methods/ticket/specs/updateEditableTicket.spec.js b/modules/ticket/back/methods/ticket/specs/updateEditableTicket.spec.js index e8007a0ce..16d8d1edb 100644 --- a/modules/ticket/back/methods/ticket/specs/updateEditableTicket.spec.js +++ b/modules/ticket/back/methods/ticket/specs/updateEditableTicket.spec.js @@ -13,7 +13,7 @@ describe('ticket updateEditableTicket()', () => { done(); }); - it('should throw an error if the ticket is not editable', async() => { + it('should now throw an error if the ticket is not editable', async() => { let error; let ctx = {req: {accessToken: {userId: 9}}}; From 156c9ec3ab0b65bfa3f0ea172e337fcb1961359a Mon Sep 17 00:00:00 2001 From: Carlos Jimenez Ruiz Date: Thu, 26 Sep 2019 09:05:54 +0200 Subject: [PATCH 2/3] =?UTF-8?q?#1716=20ticket.summary=20a=C3=B1adir=20link?= =?UTF-8?q?=20a=20la=20Ruta?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../components/label-value/label-value.html | 10 ++++- .../components/label-value/label-value.js | 29 +++++++++++--- .../label-value/label-value.spec.js | 40 +++++++++++++++++++ modules/ticket/front/summary/index.html | 4 +- 4 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 front/core/components/label-value/label-value.spec.js diff --git a/front/core/components/label-value/label-value.html b/front/core/components/label-value/label-value.html index 714c0f278..138e593c2 100644 --- a/front/core/components/label-value/label-value.html +++ b/front/core/components/label-value/label-value.html @@ -1,6 +1,12 @@
- - + + + + + { + let $element; + let controller; + + beforeEach(angular.mock.module('vnCore', $translateProvider => { + $translateProvider.translations('en', {}); + })); + + beforeEach(angular.mock.inject($componentController => { + const $attrs = {}; + $element = angular.element(`${template}`); + controller = $componentController('vnLabelValue', {$element, $attrs}); + })); + + describe('applyTextContent()', () => { + it(`should set the value on the span element as there's no navigation setted`, () => { + const value = 'I am the value'; + controller.value = value; + controller.title = value; + controller.applyTextContent(); + + expect(controller.element.querySelector('span').textContent).toEqual(value); + expect(controller.element.querySelector('span').title).toEqual(value); + }); + + it(`should set the value on the anchor element as there's a navigation setted`, () => { + const value = 'I am the value'; + controller.value = value; + controller.title = value; + controller.state = 'some.state.to.go'; + controller.applyTextContent(); + + expect(controller.element.querySelector('a').textContent).toEqual(value); + expect(controller.element.querySelector('a').title).toEqual(value); + }); + }); +}); diff --git a/modules/ticket/front/summary/index.html b/modules/ticket/front/summary/index.html index 7105e0515..383d02ec9 100644 --- a/modules/ticket/front/summary/index.html +++ b/modules/ticket/front/summary/index.html @@ -36,7 +36,9 @@ value="{{$ctrl.summary.landed | dateTime: 'dd/MM/yyyy'}}"> + value="{{$ctrl.summary.routeFk}}" + state="route.card.summary" + state-params="{id: $ctrl.summary.routeFk}"> From f28305de9b461725ca8f335941c239545f13d8f5 Mon Sep 17 00:00:00 2001 From: Bernat Date: Thu, 26 Sep 2019 09:13:18 +0200 Subject: [PATCH 3/3] Refactor #1698 claim.basic-data al llamar a updateClaim tener en cuenta el campo roleFk --- db/changes/10090-iberflora/00-ACL.sql | 1 + db/changes/10090-iberflora/00-claimState.sql | 2 + .../methods/claim/specs/updateClaim.spec.js | 84 ++++++------------- .../claim/specs/updateClaimAction.spec.js | 43 ++++++++++ .../claim/back/methods/claim/updateClaim.js | 54 ++++++------ .../back/methods/claim/updateClaimAction.js | 43 ++++++++++ modules/claim/back/models/claim-state.json | 2 +- modules/claim/back/models/claim.js | 1 + modules/claim/front/action/index.js | 4 +- 9 files changed, 147 insertions(+), 87 deletions(-) create mode 100644 db/changes/10090-iberflora/00-ACL.sql create mode 100644 db/changes/10090-iberflora/00-claimState.sql create mode 100644 modules/claim/back/methods/claim/specs/updateClaimAction.spec.js create mode 100644 modules/claim/back/methods/claim/updateClaimAction.js diff --git a/db/changes/10090-iberflora/00-ACL.sql b/db/changes/10090-iberflora/00-ACL.sql new file mode 100644 index 000000000..3a20e3ecb --- /dev/null +++ b/db/changes/10090-iberflora/00-ACL.sql @@ -0,0 +1 @@ +INSERT INTO `salix`.`ACL` (`model`, `property`, `accessType`, `permission`, `principalType`, `principalId`) VALUES ('Claim', 'updateClaimAction', 'WRITE', 'ALLOW', 'ROLE', 'salesAssistant'); diff --git a/db/changes/10090-iberflora/00-claimState.sql b/db/changes/10090-iberflora/00-claimState.sql new file mode 100644 index 000000000..7a964979b --- /dev/null +++ b/db/changes/10090-iberflora/00-claimState.sql @@ -0,0 +1,2 @@ +UPDATE `vn`.`claimState` SET `roleFk` = '21' WHERE (`id` = '3'); +UPDATE `vn`.`claimState` SET `roleFk` = '21' WHERE (`id` = '5'); diff --git a/modules/claim/back/methods/claim/specs/updateClaim.spec.js b/modules/claim/back/methods/claim/specs/updateClaim.spec.js index 5c95c6cb8..095e374bd 100644 --- a/modules/claim/back/methods/claim/specs/updateClaim.spec.js +++ b/modules/claim/back/methods/claim/specs/updateClaim.spec.js @@ -26,24 +26,21 @@ describe('Update Claim', () => { done(); }); - it('should throw error if isSaleAssistant is false and try to modify a forbidden field', async() => { - let params = { - ticketFk: 3, - clientFk: 101, - ticketCreated: newDate, - workerFk: 18, - isChargedToMana: false, - responsibility: 3, - observation: 'another' + it(`should throw an error as the user doesn't have rights`, async() => { + const forbiddenState = 3; + const salesPersonId = 18; + let data = { + claimStateFk: forbiddenState, + observation: 'valid observation' }; let ctx = { req: { accessToken: { - userId: 18 + userId: salesPersonId } } }; - await app.models.Claim.updateClaim(ctx, newInstance.id, params) + await app.models.Claim.updateClaim(ctx, newInstance.id, data) .catch(e => { error = e; }); @@ -51,74 +48,47 @@ describe('Update Claim', () => { expect(error.message).toEqual(`You don't have enough privileges to change that field`); }); - it('should throw error if isSaleAssistant is false and try to modify a valid field but a forbidden stated', async() => { - let params = { - ticketFk: 3, - clientFk: 101, - ticketCreated: newDate, - workerFk: 18, - claimStateFk: 4, - observation: 'another' + it(`should success to update the claim within privileges `, async() => { + const correctState = 4; + const salesPersonId = 18; + let data = { + observation: 'valid observation', + claimStateFk: correctState, }; let ctx = { req: { accessToken: { - userId: 18 + userId: salesPersonId } } }; - await app.models.Claim.updateClaim(ctx, newInstance.id, params) - .catch(e => { - error = e; - }); - - expect(error.message).toEqual(`You don't have enough privileges to change that field`); - }); - - it('should change field observation', async() => { - let params = { - ticketCreated: newDate, - observation: 'another3' - }; - let ctx = { - req: { - accessToken: { - userId: 18 - } - } - }; - await app.models.Claim.updateClaim(ctx, newInstance.id, params); + await app.models.Claim.updateClaim(ctx, newInstance.id, data); let claimUpdated = await app.models.Claim.findById(newInstance.id); - expect(claimUpdated.observation).toEqual(params.observation); + expect(claimUpdated.observation).toEqual(data.observation); }); - it('should change sensible fields as salesAssistant', async() => { - let params = { - ticketFk: 3, - clientFk: 101, - ticketCreated: newDate, - workerFk: 18, + it('should change some sensible fields as salesAssistant', async() => { + const salesAssistantId = 21; + let data = { claimStateFk: 3, - isChargedToMana: true, - responsibility: 3, - observation: 'another' + workerFk: 5, + observation: 'another valid observation' }; let ctx = { req: { accessToken: { - userId: 21 + userId: salesAssistantId } } }; - await app.models.Claim.updateClaim(ctx, newInstance.id, params); + await app.models.Claim.updateClaim(ctx, newInstance.id, data); let claimUpdated = await app.models.Claim.findById(newInstance.id); - expect(claimUpdated.observation).toEqual(params.observation); - expect(claimUpdated.claimStateFk).toEqual(params.claimStateFk); - expect(claimUpdated.responsibility).toEqual(params.responsibility); - expect(claimUpdated.isChargedToMana).toEqual(params.isChargedToMana); + expect(claimUpdated.observation).toEqual(data.observation); + expect(claimUpdated.claimStateFk).toEqual(data.claimStateFk); + expect(claimUpdated.workerFk).toEqual(data.workerFk); }); }); diff --git a/modules/claim/back/methods/claim/specs/updateClaimAction.spec.js b/modules/claim/back/methods/claim/specs/updateClaimAction.spec.js new file mode 100644 index 000000000..3d3404ac5 --- /dev/null +++ b/modules/claim/back/methods/claim/specs/updateClaimAction.spec.js @@ -0,0 +1,43 @@ +const app = require('vn-loopback/server/server'); + +describe('Update Claim', () => { + let newDate = new Date(); + let newInstance; + let original = { + ticketFk: 3, + clientFk: 101, + ticketCreated: newDate, + workerFk: 18, + claimStateFk: 2, + isChargedToMana: true, + responsibility: 4, + observation: 'observation' + }; + + beforeAll(async done => { + newInstance = await app.models.Claim.create(original); + + done(); + }); + + afterAll(async done => { + await app.models.Claim.destroyById(newInstance.id); + done(); + }); + + it('should update the claim isChargedToMana attribute', async() => { + const data = {isChargedToMana: false}; + const result = await app.models.Claim.updateClaimAction(newInstance.id, data); + + expect(result.id).toEqual(newInstance.id); + expect(result.isChargedToMana).toBeFalsy(); + }); + + it('should update the claim responsibility attribute', async() => { + const data = {responsibility: 2}; + const result = await app.models.Claim.updateClaimAction(newInstance.id, data); + + expect(result.id).toEqual(newInstance.id); + expect(result.responsibility).toEqual(2); + }); +}); diff --git a/modules/claim/back/methods/claim/updateClaim.js b/modules/claim/back/methods/claim/updateClaim.js index 66b409d09..51623a7a8 100644 --- a/modules/claim/back/methods/claim/updateClaim.js +++ b/modules/claim/back/methods/claim/updateClaim.js @@ -1,26 +1,23 @@ const UserError = require('vn-loopback/util/user-error'); -let pick = require('object.pick'); -let diff = require('object-diff'); - module.exports = Self => { Self.remoteMethodCtx('updateClaim', { description: 'Update a claim with privileges', accessType: 'WRITE', accepts: [{ arg: 'id', - type: 'string', + type: 'number', required: true, - description: 'Client id', + description: 'Claim id', http: {source: 'path'} }, { - arg: 'params', + arg: 'data', type: 'object', required: true, - description: 'ticketFk, stateFk', + description: 'Data to update on the model', http: {source: 'body'} }], returns: { - type: 'string', + type: 'object', root: true }, http: { @@ -29,28 +26,31 @@ module.exports = Self => { } }); - Self.updateClaim = async(ctx, id, params) => { + Self.updateClaim = async(ctx, id, data) => { let models = Self.app.models; - let isSalesAssistant; - let currentUserId = ctx.req.accessToken.userId; + let claim = await models.Claim.findById(id); - isSalesAssistant = await models.Account.hasRole(currentUserId, 'salesAssistant'); + let canUpdate = await canChangeState(ctx, claim.claimStateFk); + let hasRights = await canChangeState(ctx, data.claimStateFk); - if (!isSalesAssistant) { - let oldClaim = await models.Claim.findById(id); - let notModifiable = ['id', 'responsibility', 'isChargedToMana']; - let changedFields = diff(oldClaim, params); - let changedFieldsPicked = pick(changedFields, notModifiable); - let statesViables = ['Gestionado', 'Pendiente', 'Anulado', 'Mana']; - let oldState = await models.ClaimState.findOne({where: {id: oldClaim.claimStateFk}}); - let newState = await models.ClaimState.findOne({where: {id: params.claimStateFk}}); - let canChangeState = statesViables.includes(oldState.description) - && statesViables.includes(newState.description); - if (Object.keys(changedFieldsPicked).length != 0 || !canChangeState) - throw new UserError(`You don't have enough privileges to change that field`); - } + if (!canUpdate || !hasRights) + throw new UserError(`You don't have enough privileges to change that field`); - let claim = await Self.findById(id); - return await claim.updateAttributes(params); + return await claim.updateAttributes(data); }; + + async function canChangeState(ctx, id) { + let models = Self.app.models; + let userId = ctx.req.accessToken.userId; + + let state = await models.ClaimState.findById(id, { + include: { + relation: 'writeRole' + } + }); + let stateRole = state.writeRole().name; + let canUpdate = await models.Account.hasRole(userId, stateRole); + + return canUpdate; + } }; diff --git a/modules/claim/back/methods/claim/updateClaimAction.js b/modules/claim/back/methods/claim/updateClaimAction.js new file mode 100644 index 000000000..c9c4f1043 --- /dev/null +++ b/modules/claim/back/methods/claim/updateClaimAction.js @@ -0,0 +1,43 @@ + +module.exports = Self => { + Self.remoteMethod('updateClaimAction', { + description: 'Update a claim with privileges', + accessType: 'WRITE', + accepts: [{ + arg: 'id', + type: 'number', + required: true, + description: 'Claim id', + http: {source: 'path'} + }, { + arg: 'data', + type: 'object', + required: true, + description: 'Data to update on the model', + http: {source: 'body'} + }], + returns: { + type: 'object', + root: true + }, + http: { + path: `/:id/updateClaimAction`, + verb: 'post' + } + }); + + Self.updateClaimAction = async(id, data) => { + let models = Self.app.models; + + let claim = await models.Claim.findById(id); + let updatedData = {}; + + if (data.hasOwnProperty('responsibility')) + updatedData.responsibility = data.responsibility; + + if (data.hasOwnProperty('isChargedToMana')) + updatedData.isChargedToMana = data.isChargedToMana; + + return await claim.updateAttributes(updatedData); + }; +}; diff --git a/modules/claim/back/models/claim-state.json b/modules/claim/back/models/claim-state.json index 1707253b0..bf2554f38 100644 --- a/modules/claim/back/models/claim-state.json +++ b/modules/claim/back/models/claim-state.json @@ -18,7 +18,7 @@ } }, "relations": { - "role": { + "writeRole": { "type": "belongsTo", "model": "Role", "foreignKey": "roleFk" diff --git a/modules/claim/back/models/claim.js b/modules/claim/back/models/claim.js index 3d3971027..234a95434 100644 --- a/modules/claim/back/models/claim.js +++ b/modules/claim/back/models/claim.js @@ -5,4 +5,5 @@ module.exports = Self => { require('../methods/claim/updateClaim')(Self); require('../methods/claim/regularizeClaim')(Self); require('../methods/claim/uploadFile')(Self); + require('../methods/claim/updateClaimAction')(Self); }; diff --git a/modules/claim/front/action/index.js b/modules/claim/front/action/index.js index 5193b5114..439b70d39 100644 --- a/modules/claim/front/action/index.js +++ b/modules/claim/front/action/index.js @@ -167,14 +167,14 @@ class Controller { } saveResponsibility(value) { - let query = `/api/Claims/${this.$stateParams.id}/updateClaim`; + let query = `/api/Claims/${this.$stateParams.id}/updateClaimAction`; this.$http.post(query, {responsibility: value}).then(() => { this.vnApp.showSuccess(this.$translate.instant('Data saved!')); }); } saveMana(value) { - let query = `/api/Claims/${this.$stateParams.id}/updateClaim`; + let query = `/api/Claims/${this.$stateParams.id}/updateClaimAction`; this.$http.post(query, {isChargedToMana: value}).then(() => { this.vnApp.showSuccess(this.$translate.instant('Data saved!'));