NEW - See the documents of my subordinates #28318

BB2A-Anthony · 2024-02-20T22:49:04Z

NEW|New [See the documents of my subordinates ]

[This function is essential in my opinion]

proposal following comment from PR #24035

@eldy if good for you i change for propal invoice and thirdparty in other PR

eldy · 2024-02-21T09:46:05Z

This will generate a lot of trouble in permission systems and data view will not be consistent between screen, but this is not a pb as long as it remains a hidden option.
However what is your definition of "order of subordinates" ?

Order that are on thirdparties with one contact that is an external users that is among subordinates ?
Or order that has a sale representative that is among subordinates ?

BB2A-Anthony · 2024-02-21T13:47:54Z

This will generate a lot of trouble in permission systems and data view will not be consistent between screen, but this is not a pb as long as it remains a hidden option. However what is your definition of "order of subordinates" ?
* Order that are on thirdparties with one contact that is an external users that is among subordinates ?

* Or order that has a sale representative that is among subordinates ?

for me it's no longer this case "order that has a sale representative that is among subordinates"

What do you mean by
"This will generate a lot of trouble in permission systems and data view will not be consistent between screen"
Can you give me more details?

eldy · 2024-02-21T16:25:04Z

This will generate a lot of trouble in permission systems and data view will not be consistent between screen, but this is not a pb as long as it remains a hidden option. However what is your definition of "order of subordinates" ?
* Order that are on thirdparties with one contact that is an external users that is among subordinates ?

* Or order that has a sale representative that is among subordinates ?
for me it's no longer this case "order that has a sale representative that is among subordinates"

What do you mean by "This will generate a lot of trouble in permission systems and data view will not be consistent between screen" Can you give me more details?

100% of the code is base in this rule:

Permission on HR modules are based on perm + subordinates
Permission on project/tasks are based on perm + who is assigned
Permission on all the rest is based on perm only
Here you introduce a rule that is a mix of A and C, so this will generate troubles. User will surely not understand why sometimes he can see and sometimes he can not see some data.

If your definition is "order that has a sale representative that is among subordinates", the modification must not be done by changing the sql at each place by by forcing the search_salerepresentative to the list of id of sale representative that are subordinates.

So the code that you have removed

if (!$user->hasRight('societe', 'client', 'voir')) {
	$search_sale = $user->id;
}

should not be removed, but replaced with

if (!$user->hasRight('societe', 'client', 'voir')) {
	$search_sale = list of allowed id,...,...,...
}

So we force the filter at begin of code in the head/security section.

then in sql instead of doing a filter on search_sale, it must be a filter on IN ($db->sanitize($search_sale))

BB2A-Anthony · 2024-02-22T08:43:54Z

This will generate a lot of trouble in permission systems and data view will not be consistent between screen, but this is not a pb as long as it remains a hidden option. However what is your definition of "order of subordinates" ?
* Order that are on thirdparties with one contact that is an external users that is among subordinates ?

* Or order that has a sale representative that is among subordinates ?
for me it's no longer this case "order that has a sale representative that is among subordinates"
What do you mean by "This will generate a lot of trouble in permission systems and data view will not be consistent between screen" Can you give me more details?
100% of the code is base in this rule:
* Permission on HR modules are based on perm + subordinates

* Permission on project/tasks are based on perm + who is assigned

* Permission on all the rest is based on perm only
  Here you introduce a rule that is a mix of A and C, so this will generate troubles. User will surely not understand why sometimes he can see and sometimes he can not see some data.
If your definition is "order that has a sale representative that is among subordinates", the modification must not be done by changing the sql at each place by by forcing the search_salerepresentative to the list of id of sale representative that are subordinates.

So the code that you have removed
if (!$user->hasRight('societe', 'client', 'voir')) {
	$search_sale = $user->id;
}
should not be removed, but replaced with
if (!$user->hasRight('societe', 'client', 'voir')) {
	$search_sale = list of allowed id,...,...,...
}
So we force the filter at begin of code in the head/security section.

then in sql instead of doing a filter on search_sale, it must be a filter on IN ($db->sanitize($search_sale))

I understand. for me it doesn't make sense to mix research with security.

$search_sale lives up to its name and is made for search.

If I display the orders of my subordinates, how can I filter them to one of them or myself. Since the field is preselected with my name.

The goal is to have this behavior on all views and the user will not be lost since he knows that he is the supervisor. It is normal for a supervisor to see the work of his subordinates.

I agree that this should be an option to keep the initial behavior by default.

There is also a security problem because if you clear the filters it displays the complete list without restriction.

It is necessary that

if (!$user->hasRight('societe', 'client', 'voir')) {
	$search_sale = $user->id;
}

Or lower.

BB2A-Anthony · 2024-02-22T09:42:53Z

@eldy I rewrote the code to be simpler and optimized on the principles of holidays like in your example.

eldy · 2024-02-22T12:39:57Z

htdocs/commande/list.php

@@ -879,6 +883,16 @@
 if ($socid > 0) {
 	$sql .= ' AND s.rowid = '.((int) $socid);
 }
+
+// Restriction on sale representative
+if (!$permissiontoreadallthirdparty && !$socid) {


Why
if (!$permissiontoreadallthirdparty && !$socid) {
and not
if (!$permissiontoreadallthirdparty) {

Also the condition on MAIN_SEE_SUBORDINATES must appears.

Ok it's done

eldy · 2024-03-04T18:16:24Z

htdocs/commande/list.php

@@ -223,15 +227,16 @@
 //$arrayfields['anotherfield'] = array('type'=>'integer', 'label'=>'AnotherField', 'checked'=>1, 'enabled'=>1, 'position'=>90, 'csslist'=>'right');
 $arrayfields = dol_sort_array($arrayfields, 'position');

-if (!$user->hasRight('societe', 'client', 'voir')) {
-	$search_sale = $user->id;


It seems we loose this protection when we are in default mode:
If MAIN_SEE_SUBORDINATES not set and user has no permission to read all, this was used to restrict list on $search_sale at line 1011 (in old code file)
So you should restore a restriction into the select. For example in your addition line 888
if (!$permissiontoreadallthirdparty && getDolGlobalInt('MAIN_SEE_SUBORDINATES')) {
you can do

if (!$permissiontoreadallthirdparty) { if (getDolGlobalInt('MAIN_SEE_SUBORDINATES')) { // restrict on list in userschilds } else { // default mode, restrict on user->id only.

yes I see I did

Anthony Berton added 3 commits February 20, 2024 23:47

NEW - See the documents of my subordinates

c79770a

Merge remote-tracking branch 'upstream/develop' into Sales

55554fe

cop

57bd467

eldy added the Discussion Some questions or discussions are opened and wait answers of author or other people to be processed label Feb 21, 2024

Merge branch 'develop' into Sales

47d600c

Rewriting

b599acd

eldy reviewed Feb 22, 2024

View reviewed changes

Anthony Berton and others added 3 commits February 22, 2024 13:57

up

f0d3509

ok

0fb8198

Merge branch 'develop' into Sales

c8fc2bb

eldy reviewed Mar 4, 2024

View reviewed changes

eldy added PR to fix - See feedback in comments PR needs to be fixed to be integrated (some comments should describes the fix to do) and removed Discussion Some questions or discussions are opened and wait answers of author or other people to be processed labels Mar 4, 2024

Anthony Berton and others added 2 commits March 5, 2024 23:58

FIX - Restriction on sale representative

732ca2e

Merge branch 'develop' into Sales

a9b8011

eldy merged commit ff03f86 into Dolibarr:develop Mar 7, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NEW - See the documents of my subordinates #28318

NEW - See the documents of my subordinates #28318

BB2A-Anthony commented Feb 20, 2024

eldy commented Feb 21, 2024

BB2A-Anthony commented Feb 21, 2024

eldy commented Feb 21, 2024 •

edited

Loading

BB2A-Anthony commented Feb 22, 2024 •

edited

Loading

BB2A-Anthony commented Feb 22, 2024

eldy Feb 22, 2024

eldy Feb 22, 2024

BB2A-Anthony Feb 22, 2024

eldy Mar 4, 2024 •

edited

Loading

BB2A-Anthony Mar 5, 2024

NEW - See the documents of my subordinates #28318

NEW - See the documents of my subordinates #28318

Conversation

BB2A-Anthony commented Feb 20, 2024

NEW|New [*See the documents of my subordinates *]

eldy commented Feb 21, 2024

BB2A-Anthony commented Feb 21, 2024

eldy commented Feb 21, 2024 • edited Loading

BB2A-Anthony commented Feb 22, 2024 • edited Loading

BB2A-Anthony commented Feb 22, 2024

eldy Feb 22, 2024

Choose a reason for hiding this comment

eldy Feb 22, 2024

Choose a reason for hiding this comment

BB2A-Anthony Feb 22, 2024

Choose a reason for hiding this comment

eldy Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

BB2A-Anthony Mar 5, 2024

Choose a reason for hiding this comment

NEW|New [See the documents of my subordinates ]

eldy commented Feb 21, 2024 •

edited

Loading

BB2A-Anthony commented Feb 22, 2024 •

edited

Loading

eldy Mar 4, 2024 •

edited

Loading