verdnatura/salix
verdnatura
/
salix
3
5
Fork 2
Code Issues Pull Requests 68 Releases Wiki Activity

3795-client_administraive #938

Merged
joan merged 9 commits from 3795-client_administraive into dev 2022-05-10 08:26:42 +00:00
Conversation 0 Commits 9 Files Changed 13 +149 -22
alexm commented 2022-04-12 05:59:10 +00:00
Member
Copy Link
No description provided.
alexm added the
CR / Tests passed
 label 2022-04-12 05:59:10 +00:00
alexm added 2 commits 2022-04-12 05:59:11 +00:00
gitea/salix/pipeline/head This commit looks good Details
4a714a4b6f feat(client): add privileges to administrative role
gitea/salix/pipeline/head This commit looks good Details
ad149b3024 intro
carlosjr requested changes 2022-04-13 09:29:54 +00:00
e2e/paths/02-client/12_lock_of_verified_data.spec.js Outdated
@ -126,7 +126,7 @@ describe('Client lock verified data path', () => {
it('should confirm verified data button is enabled for salesAssistant', async() => {
carlosjr commented 2022-04-13 09:22:02 +00:00
Contributor
Copy Link

button doens't seem to be enabled.

button doens't seem to be enabled.
alexm marked this conversation as resolved
e2e/paths/02-client/12_lock_of_verified_data.spec.js Outdated
@ -130,3 +129,4 @@
expect(isDisabled).toBeTruthy();
});
it('should now edit the social name', async() => {
carlosjr commented 2022-04-13 09:22:26 +00:00
Contributor
Copy Link

doesn't seem to be changing data

doesn't seem to be changing data
alexm marked this conversation as resolved
alexm added 2 commits 2022-04-13 11:05:28 +00:00
gitea/salix/pipeline/head This commit looks good Details
36addcdb6c fix name tests
alexm requested review from carlosjr 2022-04-13 11:05:46 +00:00
joan added 1 commit 2022-04-21 07:29:10 +00:00
gitea/salix/pipeline/head This commit looks good Details
db7c1bc092 Merge branch 'dev' into 3795-client_administraive
alexm added 1 commit 2022-04-29 10:49:56 +00:00
carlosjr requested changes 2022-05-09 13:14:20 +00:00
e2e/paths/02-client/12_lock_of_verified_data.spec.js Outdated
@ -127,3 +127,3 @@
const isDisabled = await page.isDisabled(selectors.clientFiscalData.verifiedDataCheckbox);
expect(isDisabled).toBeFalsy();
expect(isDisabled).toBeTruthy();
carlosjr commented 2022-05-09 12:47:53 +00:00
Contributor
Copy Link

when evaluating booleans use boolean matchers ie: toBeTrue or toEqual(true)

any defined variable would pass as truthy, therefore may be a false negative.

when evaluating booleans use boolean matchers ie: toBeTrue or toEqual(true) any defined variable would pass as truthy, therefore may be a false negative.
alexm marked this conversation as resolved
modules/client/back/methods/client/updateFiscalData.js Outdated
@ -130,3 +130,3 @@
if (!isSalesAssistant && client.isTaxDataChecked)
if (!isAdministrative && client.isTaxDataChecked)
throw new UserError(`You can't make changes on a client with verified data`);
carlosjr commented 2022-05-09 12:56:33 +00:00
Contributor
Copy Link

the error shown here doesn't mention the lack of privileges, we believe the following error would be better:

throw new UserError(Not enough privileges to edit a client with verified data);

don't forget to add translation to it please.

the error shown here doesn't mention the lack of privileges, we believe the following error would be better: `throw new UserError(`Not enough privileges to edit a client with verified data`);` don't forget to add translation to it please.
alexm marked this conversation as resolved
modules/client/back/models/client.js
@ -211,0 +229,4 @@
const sageTransactionType = hasChanges && (changes.sageTransactionTypeFk || orgData.sageTransactionTypeFk);
const sageTransactionTypeChanged = hasChanges && orgData.sageTransactionTypeFk != sageTransactionType;
if (isTaxDataCheckedChanged && !isAdministrative)
carlosjr commented 2022-05-09 13:00:31 +00:00
Contributor
Copy Link

To avoid code repetition you can extract conditions to constants and then use a single "if"

const cantEditVerifiedData = isTaxDataCheckedChanged && !isAdministrative;
const cantChangeSageData = (sageTaxTypeChanged || sageTransactionTypeChanged) && !isSalesAssistant;        

if (cantEditVerifiedData || cantChangeSageData)
            throw new UserError(`You don't have enough privileges`);
To avoid code repetition you can extract conditions to constants and then use a single "if" ``` const cantEditVerifiedData = isTaxDataCheckedChanged && !isAdministrative; const cantChangeSageData = (sageTaxTypeChanged || sageTransactionTypeChanged) && !isSalesAssistant; if (cantEditVerifiedData || cantChangeSageData) throw new UserError(`You don't have enough privileges`); ```
alexm marked this conversation as resolved
carlosjr removed the
CR / Tests passed
 label 2022-05-09 13:14:31 +00:00
alexm added 2 commits 2022-05-10 06:49:06 +00:00
gitea/salix/pipeline/head This commit looks good Details
5bad0592a5 change errors
alexm requested review from carlosjr 2022-05-10 06:55:32 +00:00
alexm added the
CR / Tests passed
 label 2022-05-10 06:55:47 +00:00
joan added 1 commit 2022-05-10 08:24:43 +00:00
gitea/salix/pipeline/head This commit looks good Details
aa431bbe0f Merge branch 'dev' into 3795-client_administraive
joan merged commit 7f19f0bebe into dev 2022-05-10 08:26:42 +00:00
joan deleted branch 3795-client_administraive 2022-05-10 08:26:43 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
carlosjr
Labels
Clear labels   
CR / Tests passed

Unit tests and e2e passed

  
e2e

Implement e2e

  
Missing tests

Missing tests

  
Needs refactor

   
Simple

Simple check / No CR needed

No Label
CR / Tests passed
e2e
Missing tests
Needs refactor
Simple
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: verdnatura/salix#938
No description provided.
Delete Branch "3795-client_administraive"

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?