Allow multiple petty cash assigners and authorizers by pakricard · Pull Request #914 · timschofield/webERP

pakricard · 2026-04-26T08:10:54Z

Allow multiple petty cash assigners and authorizers.
DB changes related: 60.php already committed.

Copilot

Pull request overview

Enables assigning/authorizing petty cash tabs by multiple users by switching tab maintenance to multi-select user pickers and adjusting downstream tab lookups to accommodate multi-user fields.

Changes:

Updated petty cash tab maintenance (PcTabs.php) to accept multiple assigners and cash/expense authorisers and persist them as comma-separated values.
Updated tab-selection queries in reporting/authorization/assignment scripts to use substring matching against the multi-user fields.
Minor form/layout adjustments in the expenses authorization tab selector.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
PcTabs.php	Adds multi-select UI and writes assigner/authoriser fields as CSV strings.
PcReportTab.php	Broadens tab visibility filter to match multi-authoriser/assigner values.
PcReportExpense.php	Broadens expense report filters to match multi-authoriser/assigner values.
PcAuthorizeExpenses.php	Broadens eligible tabs list for expense authorization and tweaks the selector markup.
PcAuthorizeCash.php	Broadens eligible tabs list for cash authorization to match multi-authorisers.
PcAssignCashToTab.php	Broadens eligible tabs list for cash assignment to match multi-assigners.

Comments suppressed due to low confidence (1)

PcAuthorizeExpenses.php:361

The select element posts as name="SelectedTabs", but the code checks $_POST['SelectTabs'] when deciding which option to preselect. This prevents the selected tab from being retained after submission. Use $_POST['SelectedTabs'] consistently.

				<select required="required" name="SelectedTabs">';
	while ($MyRow = DB_fetch_array($Result)) {
		if (isset($_POST['SelectTabs']) and $MyRow['tabcode'] == $_POST['SelectTabs']) {
			echo '<option selected="selected" value="', $MyRow['tabcode'], '">', $MyRow['tabcode'], '</option>';
		} else {
			echo '<option value="', $MyRow['tabcode'], '">', $MyRow['tabcode'], '</option>';

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-26T08:15:17Z

 	while ($MyRow = DB_fetch_array($Result)) {
-		if (isset($_POST['SelectAuthoriserCash']) and $MyRow['userid'] == $_POST['SelectAuthoriserCash']) {
-			echo '<option selected="selected" value="', $MyRow['userid'], '">', $MyRow['userid'], ' - ', $MyRow['realname'], '</option>';
+		$Authorizer = explode(',',$_POST['SelectAuthoriserCash']);
+		if (isset($_POST['SelectAuthoriserCash']) and in_array($MyRow['userid'],$Authorizer)) {
+			echo '<option selected="selected" value="';
 		} else {


Same issue as the assigner select: explode() assumes a string but $_POST['SelectAuthoriserCash'] will be an array after submit, leading to a TypeError and broken re-rendering on validation errors. Normalize the selected values before the loop (handle both CSV string from DB and array from POST).

@copilot apply changes based on this feedback

Copilot · 2026-04-26T08:15:19Z

 	while ($MyRow = DB_fetch_array($Result)) {
-		if (isset($_POST['SelectAuthoriserExpenses']) and $MyRow['userid'] == $_POST['SelectAuthoriserExpenses']) {
-			echo '<option selected="selected" value="', $MyRow['userid'], '">', $MyRow['userid'], ' - ', $MyRow['realname'], '</option>';
+		$Authorizer = explode(',',$_POST['SelectAuthoriserExpenses']);
+		if (isset($_POST['SelectAuthoriserExpenses']) and in_array($MyRow['userid'],$Authorizer)) {
+			echo '<option selected="selected" value="';
 		} else {


Same issue as the cash authoriser select: explode() will receive an array after submit (name="SelectAuthoriserExpenses[]"), causing a TypeError on re-render. Normalize selected values once outside the loop and handle both CSV string and array forms.

Copilot · 2026-04-26T08:15:19Z

+				AND (pctabs.authorizer LIKE '%" . $_SESSION['UserID'] . "%' 
+					OR pctabs.usercode = '" . $_SESSION['UserID'] . "' 
+					OR pctabs.assigner LIKE '%" . $_SESSION['UserID'] . "%' )


This authorization predicate uses LIKE '%%' against a comma-separated list, which can match partial user IDs and leak data. Update the predicate to match whole IDs using delimiter-aware matching (or a normalized join table) rather than substring LIKE.

Suggested change

AND (pctabs.authorizer LIKE '%" . $_SESSION['UserID'] . "%'

OR pctabs.usercode = '" . $_SESSION['UserID'] . "'

OR pctabs.assigner LIKE '%" . $_SESSION['UserID'] . "%' )

AND (FIND_IN_SET('" . $_SESSION['UserID'] . "', REPLACE(pctabs.authorizer, ' ', '')) > 0

OR pctabs.usercode = '" . $_SESSION['UserID'] . "'

OR FIND_IN_SET('" . $_SESSION['UserID'] . "', REPLACE(pctabs.assigner, ' ', '')) > 0)

Copilot · 2026-04-26T08:15:19Z

 	$SQL = "SELECT tabcode
 		FROM pctabs
-		WHERE authorizerexpenses='" . $_SESSION['UserID'] . "'
+		WHERE authorizerexpenses LIKE '%" . $_SESSION['UserID'] . "%'


authorizerexpenses is now treated as a comma-separated list; using LIKE '%%' can match partial IDs and show tabs to the wrong users. Switch to delimiter-aware matching so only whole user IDs match.

Suggested change

WHERE authorizerexpenses LIKE '%" . $_SESSION['UserID'] . "%'

WHERE CONCAT(',', authorizerexpenses, ',') LIKE '%," . $_SESSION['UserID'] . ",%'

Copilot · 2026-04-26T08:15:20Z

+		$i = 0;
+		foreach ($_POST['SelectAuthoriserExpenses'] as $Value) {
+			if ($i) $AuthorisersExpenses .= ',';
+			$AuthorisersExpenses .= $Value;
+			$i++;
+		}
+
+		$AuthorisersCash = '';
+		$i = 0;
+		foreach ($_POST['SelectAuthoriserCash'] as $Value) {
+			if ($i) $AuthorisersCash .= ',';
+			$AuthorisersCash .= $Value;
+			$i++;
+		}
+
+		$Assigners = '';
+		$i = 0;
+		foreach ($_POST['SelectAssigner'] as $Value) {
+			if ($i) $Assigners .= ',';
+			$Assigners .= $Value;
+			$i++;


These foreach loops assume SelectAuthoriserExpenses/SelectAuthoriserCash/SelectAssigner are always set and are arrays. If a later validation fails (e.g., TaxGroup) the page will re-render with these values as arrays, but if the keys are unset you'll get warnings; if they are not arrays you'll get runtime errors. Guard with isset()+is_array(), and consider using implode() to build the comma-separated values.

Suggested change

$i = 0;

foreach ($_POST['SelectAuthoriserExpenses'] as $Value) {

if ($i) $AuthorisersExpenses .= ',';

$AuthorisersExpenses .= $Value;

$i++;

}

$AuthorisersCash = '';

$i = 0;

foreach ($_POST['SelectAuthoriserCash'] as $Value) {

if ($i) $AuthorisersCash .= ',';

$AuthorisersCash .= $Value;

$i++;

}

$Assigners = '';

$i = 0;

foreach ($_POST['SelectAssigner'] as $Value) {

if ($i) $Assigners .= ',';

$Assigners .= $Value;

$i++;

if (isset($_POST['SelectAuthoriserExpenses']) and is_array($_POST['SelectAuthoriserExpenses'])) {

$AuthorisersExpenses = implode(',', $_POST['SelectAuthoriserExpenses']);

}

$AuthorisersCash = '';

if (isset($_POST['SelectAuthoriserCash']) and is_array($_POST['SelectAuthoriserCash'])) {

$AuthorisersCash = implode(',', $_POST['SelectAuthoriserCash']);

}

$Assigners = '';

if (isset($_POST['SelectAssigner']) and is_array($_POST['SelectAssigner'])) {

$Assigners = implode(',', $_POST['SelectAssigner']);

Copilot · 2026-04-26T08:15:20Z

 	echo '<field>
 			<label for="SelectAssigner">', __('Cash Assigner'), ':</label>
-			<select required="required" name="SelectAssigner">';
+			<td><select multiple="multiple" name="SelectAssigner[]">';


The select markup introduces a inside a block and drops the required attribute. This results in invalid HTML (unmatched table cell tags) and allows submitting with no selection. Remove the wrapper and keep required="required" on these multiple selects (or enforce selection server-side).

Suggested change

<td><select multiple="multiple" name="SelectAssigner[]">';

<select multiple="multiple" name="SelectAssigner[]" required="required">';

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

pakricard · 2026-04-26T08:31:42Z

@timschofield : Copilot suggested a bunch of code improvement. The code as I committed was (is) OK and working on my installation. Frist time I faced this Copilot revision on the fly.
Did you set this Copilot function?

timschofield · 2026-04-26T12:35:55Z

@timschofield : Did you set this Copilot function?

Not intentionally but I wouldn't guarantee that I didn't do it by accident. My experience of Copilot is that I would be more comfortable with your code than copilot's suggestions.

Tim

timschofield · 2026-04-26T19:20:52Z

I have changed some settings so hopefully this doesnt happen again

pakricard · 2026-04-27T01:17:05Z

@timschofield : Did you set this Copilot function?

Not intentionally but I wouldn't guarantee that I didn't do it by accident. My experience of Copilot is that I would be more comfortable with your code than copilot's suggestions.

Tim

I think the Copilot suggestions in this case are sensible, and working. So I'm OK leaving them. If you have a strong opinion on this, then I will revert Copilot suggestions

Allow multiple petty cash assigners and authorizers

cb17f44

Copilot AI review requested due to automatic review settings April 26, 2026 08:10

Copilot started reviewing on behalf of pakricard April 26, 2026 08:11 View session

Copilot AI reviewed Apr 26, 2026

View reviewed changes

pakricard and others added 4 commits April 26, 2026 16:18

Update PcReportTab.php

27029d3

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update PcAuthorizeCash.php

dbff8b4

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update PcReportExpense.php

8f61d94

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update PcAssignCashToTab.php

cd3dfea

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

timschofield merged commit e4e69cc into timschofield:master Apr 27, 2026
5 checks passed

pakricard deleted the PettyCashMultipleAuthorizers branch April 28, 2026 01:56

	WHERE authorizerexpenses LIKE '%" . $_SESSION['UserID'] . "%'
	WHERE CONCAT(',', authorizerexpenses, ',') LIKE '%," . $_SESSION['UserID'] . ",%'

	<td><select multiple="multiple" name="SelectAssigner[]">';
	<select multiple="multiple" name="SelectAssigner[]" required="required">';

Conversation

pakricard commented Apr 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

pakricard Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 26, 2026

Choose a reason for hiding this comment

Uh oh!

pakricard commented Apr 26, 2026

Uh oh!

timschofield commented Apr 26, 2026

Uh oh!

timschofield commented Apr 26, 2026

Uh oh!

pakricard commented Apr 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants