Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 50 additions & 2 deletions ProcessMaker/AssignmentRules/ProcessManagerAssigned.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
use ProcessMaker\Exception\ThereIsNoProcessManagerAssignedException;
use ProcessMaker\Models\Process;
use ProcessMaker\Models\ProcessRequest;
use ProcessMaker\Models\ProcessRequestToken;
use ProcessMaker\Models\User;
use ProcessMaker\Nayra\Contracts\Bpmn\ActivityInterface;
use ProcessMaker\Nayra\Contracts\Bpmn\TokenInterface;

Expand All @@ -24,16 +26,62 @@ class ProcessManagerAssigned implements AssignmentRuleInterface
* @param TokenInterface $token
* @param Process $process
* @param ProcessRequest $request
* @return int
* @return int|null
* @throws ThereIsNoProcessManagerAssignedException
*/
public function getNextUser(ActivityInterface $task, TokenInterface $token, Process $process, ProcessRequest $request)
{
$user_id = $request->processVersion->manager_id;
// review for multiple managers
$managers = $request->processVersion->manager_id;
$user_id = $this->getNextManagerAssigned($managers, $task, $request);
if (!$user_id) {
throw new ThereIsNoProcessManagerAssignedException($task);
}

return $user_id;
}

/**
* Get the round robin manager using a true round robin algorithm
*
* @param array $managers
* @param ActivityInterface $task
* @param ProcessRequest $request
* @return int|null
*/
private function getNextManagerAssigned($managers, $task, $request)
{
// Validate input
if (empty($managers) || !is_array($managers)) {
return null;
}

// If only one manager, return it
if (count($managers) === 1) {
return $managers[0];
}

// get the last manager assigned to the task across all requests
$last = ProcessRequestToken::where('process_id', $request->process_id)
->where('element_id', $task->getId())
->whereIn('user_id', $managers)
->orderBy('created_at', 'desc')
->first();

$user_id = $last ? $last->user_id : null;

sort($managers);

$key = array_search($user_id, $managers);
if ($key === false) {
// If no previous manager found, start with the first manager
$key = 0;
} else {
// Move to the next manager in the round-robin
$key = ($key + 1) % count($managers);
}
$user_id = $managers[$key];

return $user_id;
}
}
55 changes: 52 additions & 3 deletions ProcessMaker/Http/Controllers/Api/ProcessController.php
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ public function store(Request $request)

//set manager id
if ($request->has('manager_id')) {
$process->manager_id = $request->input('manager_id', null);
$process->manager_id = $this->validateMaxManagers($request);
}

if (isset($data['bpmn'])) {
Expand Down Expand Up @@ -542,7 +542,7 @@ public function update(Request $request, Process $process)

$process->fill($request->except('notifications', 'task_notifications', 'notification_settings', 'cancel_request', 'cancel_request_id', 'start_request_id', 'edit_data', 'edit_data_id', 'projects'));
if ($request->has('manager_id')) {
$process->manager_id = $request->input('manager_id', null);
$process->manager_id = $this->validateMaxManagers($request);
}

if ($request->has('user_id')) {
Expand Down Expand Up @@ -621,6 +621,55 @@ public function update(Request $request, Process $process)
return new Resource($process->refresh());
}

private function validateMaxManagers(Request $request)
{
$managerIds = $request->input('manager_id', []);

// Handle different input types
if (is_string($managerIds)) {
// If it's a string, try to decode it as JSON
if (empty($managerIds)) {
$managerIds = [];
} else {
$decoded = json_decode($managerIds, true);

// Handle JSON decode failure
if (json_last_error() !== JSON_ERROR_NONE) {
throw new \Illuminate\Validation\ValidationException(
validator([], []),
['manager_id' => [__('Invalid JSON format for manager_id')]]
);
}

$managerIds = $decoded;
}
}

// Ensure we have an array
if (!is_array($managerIds)) {
// If it's a single value (not array), convert to array
$managerIds = [$managerIds];
}

// Filter out null, empty values and validate each manager ID
$managerIds = array_filter($managerIds, function ($id) {
return $id !== null && $id !== '' && is_numeric($id) && $id > 0;
});

// Re-index the array to remove gaps from filtered values
$managerIds = array_values($managerIds);

// Validate maximum number of managers
if (count($managerIds) > 10) {
throw new \Illuminate\Validation\ValidationException(
validator([], []),
['manager_id' => [__('Maximum number of managers is :max', ['max' => 10])]]
);
}

return $managerIds;
}

/**
* Validate the structure of stages.
*
Expand Down Expand Up @@ -1714,7 +1763,7 @@ protected function checkUserCanStartProcess($event, $currentUser, $process, $req
}
break;
case 'process_manager':
$response = $currentUser === $process->manager_id;
$response = in_array($currentUser, $process->manager_id ?? []);
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion ProcessMaker/Http/Controllers/CasesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public function show($case_number)
// The user can see the comments
$canViewComments = (Auth::user()->hasPermissionsFor('comments')->count() > 0) || class_exists(PackageServiceProvider::class);
// The user is Manager from the main request
$isProcessManager = $request->process?->manager_id === Auth::user()->id;
$isProcessManager = in_array(Auth::user()->id, $request->process?->manager_id ?? []);
// Check if the user has permission print for request
$canPrintScreens = $canOpenCase = $this->canUserCanOpenCase($allRequests);
if (!$canOpenCase && !$isProcessManager) {
Expand Down
38 changes: 32 additions & 6 deletions ProcessMaker/ImportExport/Exporters/ExporterBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,8 @@ public function toArray()
'dependents' => array_map(fn ($d) => $d->toArray(), $this->dependents),
'name' => $this->getName($this->model),
'description' => $this->getDescription(),
'process_manager' => $this->getProcessManager()['managerName'],
'process_manager_id' => $this->getProcessManager()['managerId'],
'process_manager' => $this->getProcessManager(),
'process_manager_id' => $this->getProcessManagerIds(),
'attributes' => $this->getExportAttributes(),
'extraAttributes' => $this->getExtraAttributes($this->model),
'references' => $this->references,
Expand Down Expand Up @@ -383,10 +383,36 @@ public function getExtraAttributes($model): array

public function getProcessManager(): array
{
return [
'managerId' => $this->model->manager?->id ? $this->model->manager->id : null,
'managerName' => $this->model->manager?->fullname ? $this->model->manager->fullname : '',
];
// Check if the model has the getManagers method
if (!method_exists($this->model, 'getManagers')) {
return [];
}

$managers = $this->model->getManagers() ?? [];

$managerNames = [];
foreach ($managers as $manager) {
$managerNames[] = $manager->fullname;
}

return $managerNames;
}

public function getProcessManagerIds(): array
{
// Check if the model has the getManagers method
if (!method_exists($this->model, 'getManagers')) {
return [];
}

$managers = $this->model->getManagers() ?? [];

$managerIds = [];
foreach ($managers as $manager) {
$managerIds[] = $manager->id;
}

return $managerIds;
}

public function getLastModifiedBy() : array
Expand Down
12 changes: 9 additions & 3 deletions ProcessMaker/ImportExport/Exporters/ProcessExporter.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,12 @@ public function export() : void
$this->addDependent('user', $process->user, UserExporter::class);
}

if ($process->manager) {
$this->addDependent('manager', $process->manager, UserExporter::class, null, ['properties']);
$managers = $process->getManagers();

if ($managers) {
foreach ($managers as $manager) {
$this->addDependent('manager', $manager, UserExporter::class, null, ['properties']);
}
}

$this->exportScreens();
Expand Down Expand Up @@ -99,9 +103,11 @@ public function import($existingAssetInDatabase = null, $importingFromTemplate =
$process->user_id = User::where('is_administrator', true)->firstOrFail()->id;
}

$managers = [];
foreach ($this->getDependents('manager') as $dependent) {
$process->manager_id = $dependent->model->id;
$managers[] = $dependent->model->id;
}
$process->manager_id = $managers;

// Avoid associating the category from the manifest with processes imported from templates.
// Use the user-selected category instead.
Expand Down
9 changes: 5 additions & 4 deletions ProcessMaker/Jobs/ErrorHandling.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,11 @@ private function requeue($job)
public function sendExecutionErrorNotification(string $message)
{
if ($this->processRequestToken) {
$user = $this->processRequestToken->processRequest->processVersion->manager;
if ($user !== null) {
Log::info('Send Execution Error Notification: ' . $message);
Notification::send($user, new ErrorExecutionNotification($this->processRequestToken, $message, $this->bpmnErrorHandling));
// review no multiple managers
$mangers = $this->processRequestToken->processRequest->processVersion->getManagers();
foreach ($mangers as $manager) {
Log::info('Send Execution Error Notification: ' . $message . ' to manager: ' . $manager->username);
Notification::send($manager, new ErrorExecutionNotification($this->processRequestToken, $message, $this->bpmnErrorHandling));
}
}
}
Expand Down
17 changes: 16 additions & 1 deletion ProcessMaker/Models/FormalExpression.php
Original file line number Diff line number Diff line change
Expand Up @@ -239,8 +239,23 @@ function ($__data, $user_id, $assigned_groups) {
// If no manager is found, then assign the task to the Process Manager.
$request = ProcessRequest::find($__data['_request']['id']);
$process = $request->processVersion;
$managers = $process->manager_id ?? [];

return $process->manager_id;
if (empty($managers)) {
return null;
}

// Sort managers to ensure consistent round robin distribution
sort($managers);

// Use a combination of process ID and request ID for better distribution
// This ensures different processes don't interfere with each other's round robin
$processId = $process->id ?? 0;
$requestId = $__data['_request']['id'] ?? 0;
$seed = $processId + $requestId;
$managerIndex = $seed % count($managers);

return $managers[$managerIndex];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Manager Assignment Conflict

The group_manager function now uses a deterministic manager assignment based on process and request IDs. This conflicts with the true round-robin logic in ProcessManagerAssigned.php, leading to inconsistent and unpredictable manager assignments.

Fix in Cursor Fix in Web

}

return $user->manager_id;
Expand Down
22 changes: 14 additions & 8 deletions ProcessMaker/Models/Process.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
* @OA\Property(property="self_service_tasks", type="object"),
* @OA\Property(property="signal_events", type="array", @OA\Items(type="object")),
* @OA\Property(property="category", type="object", @OA\Schema(ref="#/components/schemas/ProcessCategory")),
* @OA\Property(property="manager_id", type="integer", format="id"),
* @OA\Property(property="manager_id", type="array", @OA\Items(type="integer", format="id")),
* ),
* @OA\Schema(
* schema="Process",
Expand Down Expand Up @@ -589,7 +589,7 @@ public function collaborations()
* Get the user to whom to assign a task.
*
* @param ActivityInterface $activity
* @param TokenInterface $token
* @param ProcessRequestToken $token
*
* @return User
*/
Expand All @@ -613,14 +613,14 @@ public function getNextUser(ActivityInterface $activity, ProcessRequestToken $to
if ($userByRule !== null) {
$user = $this->scalateToManagerIfEnabled($userByRule->id, $activity, $token, $assignmentType);

return $this->checkAssignment($token->processRequest, $activity, $assignmentType, $escalateToManager, $user ? User::where('id', $user)->first() : null);
return $this->checkAssignment($token->processRequest, $activity, $assignmentType, $escalateToManager, $user ? User::where('id', $user)->first() : null, $token);
}
}

if (filter_var($assignmentLock, FILTER_VALIDATE_BOOLEAN) === true) {
$user = $this->getLastUserAssignedToTask($activity->getId(), $token->getInstance()->getId());
if ($user) {
return $this->checkAssignment($token->processRequest, $activity, $assignmentType, $escalateToManager, User::where('id', $user)->first());
return $this->checkAssignment($token->processRequest, $activity, $assignmentType, $escalateToManager, User::where('id', $user)->first(), $token);
}
}

Expand Down Expand Up @@ -665,7 +665,7 @@ public function getNextUser(ActivityInterface $activity, ProcessRequestToken $to

$user = $this->scalateToManagerIfEnabled($user, $activity, $token, $assignmentType);

return $this->checkAssignment($token->getInstance(), $activity, $assignmentType, $escalateToManager, $user ? User::where('id', $user)->first() : null);
return $this->checkAssignment($token->getInstance(), $activity, $assignmentType, $escalateToManager, $user ? User::where('id', $user)->first() : null, $token);
}

/**
Expand All @@ -676,10 +676,11 @@ public function getNextUser(ActivityInterface $activity, ProcessRequestToken $to
* @param string $assignmentType
* @param bool $escalateToManager
* @param User|null $user
* @param ProcessRequestToken $token
*
* @return User|null
*/
private function checkAssignment(ProcessRequest $request, ActivityInterface $activity, $assignmentType, $escalateToManager, User $user = null)
private function checkAssignment(ProcessRequest $request, ActivityInterface $activity, $assignmentType, $escalateToManager, User $user = null, ProcessRequestToken $token = null)
{
$config = $activity->getProperty('config') ? json_decode($activity->getProperty('config'), true) : [];
$selfServiceToggle = array_key_exists('selfService', $config ?? []) ? $config['selfService'] : false;
Expand All @@ -693,10 +694,15 @@ private function checkAssignment(ProcessRequest $request, ActivityInterface $act
if ($isSelfService && !$escalateToManager) {
return null;
}
$user = $request->processVersion->manager;
$rule = new ProcessManagerAssigned();
if ($token === null) {
throw new ThereIsNoProcessManagerAssignedException($activity);
}
$user = $rule->getNextUser($activity, $token, $this, $request);
if (!$user) {
throw new ThereIsNoProcessManagerAssignedException($activity);
}
$user = User::find($user);
}

return $user;
Expand Down Expand Up @@ -1147,7 +1153,7 @@ public function getStartEvents($filterWithPermissions = false, $filterWithoutAss
}
}
} elseif (isset($startEvent['assignment']) && $startEvent['assignment'] === 'process_manager') {
$access = $this->manager && $this->manager->id && $this->manager->id === $user->id;
$access = in_array($user->id, $this->manager_id ?? []);
} else {
$access = false;
}
Expand Down
2 changes: 1 addition & 1 deletion ProcessMaker/Models/ProcessRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ public function getNotifiableUserIds($notifiableType)
case 'participants':
return $this->participants()->get()->pluck('id');
case 'manager':
return collect([$this->process()->first()->manager_id]);
return collect($this->process()->first()->manager_id ?? []);
default:
return collect([]);
}
Expand Down
2 changes: 1 addition & 1 deletion ProcessMaker/Models/ProcessRequestToken.php
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ public function getNotifiableUserIds($notifiableType)
case 'manager':
$process = $this->process()->first();

return collect([$process?->manager_id]);
return collect($process?->manager_id ?? []);
break;
default:
return collect([]);
Expand Down
Loading
Loading