Skip to content

FOUR-21495 default columns not being listed in the available columns #7909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jan 17, 2025
Merged

FOUR-21495 default columns not being listed in the available columns #7909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
df0315a
fix: get process variables including available columns
devmiguelangel Jan 16, 2025
8772d09
test: add tests for saved search with available columns
devmiguelangel Jan 17, 2025
dd46407
Update tests to work with and without package variable finder
caleeli Jan 17, 2025
958f1ae
feat: add option to toggle VariableFinder usage in ProcessVariableCon…
caleeli Jan 17, 2025
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
57 changes: 50 additions & 7 deletions ProcessMaker/Http/Controllers/Api/V1_1/ProcessVariableController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class ProcessVariableController extends Controller

const CACHE_TTL = 60;
private static bool $mockData = false;
private static bool $useVarFinder = true;

/**
* @OA\Schema(
Expand Down Expand Up @@ -203,11 +204,6 @@ private function getProcessesVariablesFromMock(array $processIds, $excludeSavedS
*/
public function getProcessesVariables(array $processIds, $excludeSavedSearch, $page, $perPage, $request)
{
// If the classes or tables do not exist, fallback to a saved search approach.
if (!class_exists(ProcessVariable::class) || !Schema::hasTable('process_variables')) {
return $this->getProcessesVariablesFrom($processIds);
}

// Determine which columns to exclude based on the saved search
$activeColumns = [];
if ($excludeSavedSearch) {
Expand All @@ -217,6 +213,27 @@ public function getProcessesVariables(array $processIds, $excludeSavedSearch, $p
}
}

// If the classes or tables do not exist, fallback to a saved search approach.
if (
!class_exists(ProcessVariable::class)
|| !Schema::hasTable('process_variables')
|| !self::$useVarFinder
) {
$paginator = $this->getProcessesVariablesFrom($processIds);
if ($request->has('onlyAvailable')) {
// Get the available columns from the saved search
$availableColumns = $savedSearch->available_columns;
// Merge the available columns with the paginated items
$availableColumns = $availableColumns->merge($paginator->items());
// Set the collection to the merged collection
$paginator->setCollection($availableColumns);

return $paginator;
}

return $paginator;
}

// Build a single query that joins process_variables, asset_variables, and var_finder_variables
// and applies filtering for excluded fields.
$query = DB::table('var_finder_variables AS vfv')
Expand All @@ -230,7 +247,7 @@ public function getProcessesVariables(array $processIds, $excludeSavedSearch, $p
}

// Paginate the query result directly
return $query->select(
$query->select(
'vfv.id',
'pv.process_id',
'vfv.data_type AS format',
Expand All @@ -239,7 +256,23 @@ public function getProcessesVariables(array $processIds, $excludeSavedSearch, $p
DB::raw('NULL AS `default`'),
'vfv.created_at',
'vfv.updated_at',
)->paginate($perPage, ['*'], 'page', $page);
);

// Return the paginated result
$paginator = $query->paginate($perPage, ['*'], 'page', $page);

if ($request->has('onlyAvailable')) {
// Get the available columns from the saved search
$availableColumns = $savedSearch->available_columns;
// Merge the available columns with the paginated items
$availableColumns = $availableColumns->merge($paginator->items());
// Set the collection to the merged collection
$paginator->setCollection($availableColumns);

return $paginator;
}

return $query->paginate($perPage, ['*'], 'page', $page);
}

/**
Expand All @@ -252,6 +285,16 @@ public static function mock(bool $value = true)
static::$mockData = $value;
}

/**
* Change ProcessVariableController to not use VariableFinder
*
* @return void
*/
public static function useVarFinder(bool $value = true)
{
static::$useVarFinder = $value;
}

/**
* Retrieve process variables from its screens.
*
Expand Down
186 changes: 180 additions & 6 deletions tests/Feature/Api/V1_1/ProcessVariableControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,20 @@ public function setupCreateUser()
// Check if the VariableFinder package is enabled
$this->isVariablesFinderEnabled = class_exists(ProcessVariable::class) && Schema::hasTable('process_variables');

// Clear process variables cache
$this->clearCache([1, 2, 3]);
$this->clearCache([1, 2]);

// Create the processes variables
if (!$this->isVariablesFinderEnabled) {
// Mock the ProcessVariableController to use mock data instead of VariableFinder package
ProcessVariableController::mock();
ProcessVariableController::mock(true);
ProcessVariableController::useVarFinder(false);
$this->mockVariableFinder([1, 2, 3], null);
$this->mockVariableFinder([1, 2], null);
} else {
ProcessVariableController::mock(false);
ProcessVariableController::useVarFinder(true);
$this->loadVariableFinderData([1, 2, 3]);
}
}
Expand Down Expand Up @@ -90,21 +97,36 @@ public function test_can_get_process_variables_with_pagination(): void
}

/**
* Test successful variables retrieval with pagination
* Test successful variables retrieval with pagination without us
*/
public function test_can_get_process_variables_from_process_screens_with_pagination(): void
{
ProcessVariableController::mock(false);
ProcessVariableController::useVarFinder(false);

$bpmn = file_get_contents(base_path('tests/Feature/Api/bpmnPatterns/SimpleTaskProcess.bpmn'));
ProcessVariableController::mock(false);
$screen1 = $this->createScreenWithFields(1, 10);
$screen2 = $this->createScreenWithFields(2, 10);
$screen3 = $this->createScreenWithFields(3, 10);
Process::factory()->create(['id' => 1, 'bpmn' => str_replace('pm:screenRef="2"', 'pm:screenRef="' . $screen1->id . '"', $bpmn)]);
Process::factory()->create(['id' => 2, 'bpmn' => str_replace('pm:screenRef="2"', 'pm:screenRef="' . $screen2->id . '"', $bpmn)]);
Process::factory()->create(['id' => 3, 'bpmn' => str_replace('pm:screenRef="2"', 'pm:screenRef="' . $screen3->id . '"', $bpmn)]);
$processIds = [];
$processIds[] = Process::factory()->create([
'bpmn' => str_replace('pm:screenRef="2"', 'pm:screenRef="' . $screen1->id . '"', $bpmn)
])->id;
$processIds[] = Process::factory()->create([
'bpmn' => str_replace('pm:screenRef="2"', 'pm:screenRef="' . $screen2->id . '"', $bpmn)
])->id;
$processIds[] = Process::factory()->create([
'bpmn' => str_replace('pm:screenRef="2"', 'pm:screenRef="' . $screen3->id . '"', $bpmn)
])->id;
$route = route('api.1.1.process_variables.index', [
'processIds' => implode(',', $processIds),
'page' => 1,
'per_page' => 15
]);

// Make request to the endpoint
$response = $this->apiCall('GET', '/api/1.1/processes/variables?processIds=1,2,3&page=1&per_page=15');
$response = $this->apiCall('GET', $route);

// Assert response structure and status
$response->assertStatus(200)
Expand Down Expand Up @@ -175,6 +197,12 @@ private function mockVariableFinder(array $processIds, $excludeSavedSearch)
return $variables;
}

private function clearCache(array $processIds)
{
$cacheKey = 'process_variables_' . implode('_', $processIds);
Cache::forget($cacheKey);
}

private function getRandomDataType(): string
{
return collect(['string', 'int', 'boolean', 'array'])->random();
Expand Down Expand Up @@ -276,6 +304,10 @@ public function test_pagination_consistency(): void
*/
public function test_process_ids_filtering(): void
{
ProcessVariableController::mock(true);
$this->mockVariableFinder([1, 2, 3], null);
$this->mockVariableFinder([1, 2], null);

$response = $this->apiCall('GET', '/api/1.1/processes/variables?processIds=1,2&per_page=50');

$responseData = $response->json();
Expand Down Expand Up @@ -371,4 +403,146 @@ private function createScreenWithFields(int $processId, int $fieldsCount)
],
]);
}

public function test_saved_search_with_all_available_columns(): void
{
ProcessVariableController::mock(false);
// Create a saved search with specific columns
$savedSearch = SavedSearch::factory()->create([
'type' => 'request',
'meta' => [
"icon" => "bath",
"file" => null,
"collection_id" => null,
"columns" => [],
],
'pmql' => '',
]);

$response = $this->apiCall('GET', '/api/1.1/processes/variables?processIds=1&savedSearchId=' . $savedSearch->id . '&onlyAvailable=');

$responseData = $response->json();

$filteredFields = collect($responseData['data'])->pluck('field');

$this->assertTrue($filteredFields->contains('case_number'));
$this->assertTrue($filteredFields->contains('case_title'));
$this->assertTrue($filteredFields->contains('name'));
$this->assertTrue($filteredFields->contains('active_tasks'));
$this->assertTrue($filteredFields->contains('process_version_alternative'));
$this->assertTrue($filteredFields->contains('participants'));
$this->assertTrue($filteredFields->contains('status'));
$this->assertTrue($filteredFields->contains('initiated_at'));
$this->assertTrue($filteredFields->contains('completed_at'));
}

public function test_saved_search_with_remaining_available_columns(): void
{
ProcessVariableController::mock(false);
// Create a saved search with specific columns
$savedSearch = SavedSearch::factory()->create([
'type' => 'request',
'meta' => [
"icon" => "bath",
"file" => null,
"collection_id" => null,
"columns" => [
[
"label" => "Case Number",
"field" => "case_number",
],
[
"label" => "Case Title",
"field" => "case_title",
],
],
],
'pmql' => '',
]);

$response = $this->apiCall('GET', '/api/1.1/processes/variables?processIds=1&savedSearchId=' . $savedSearch->id . '&onlyAvailable=');

$responseData = $response->json();

$filteredFields = collect($responseData['data'])->pluck('field');

$this->assertFalse($filteredFields->contains('case_number'));
$this->assertFalse($filteredFields->contains('case_title'));

$this->assertTrue($filteredFields->contains('name'));
$this->assertTrue($filteredFields->contains('active_tasks'));
$this->assertTrue($filteredFields->contains('process_version_alternative'));
$this->assertTrue($filteredFields->contains('participants'));
$this->assertTrue($filteredFields->contains('status'));
$this->assertTrue($filteredFields->contains('initiated_at'));
$this->assertTrue($filteredFields->contains('completed_at'));
}

public function test_saved_search_with_no_available_columns(): void
{
// Create a saved search with specific columns
$savedSearch = SavedSearch::factory()->create([
'type' => 'request',
'meta' => [
"icon" => "bath",
"file" => null,
"collection_id" => null,
"columns" => [
[
"label" => "Case Number",
"field" => "case_number",
],
[
"label" => "Case Title",
"field" => "case_title",
],
[
"label" => "Name",
"field" => "name",
],
[
'label' => 'Active Tasks',
'field' => 'active_tasks',
],
[
'label' => 'Process Version Alternative',
'field' => 'process_version_alternative',
],
[
'label' => 'Participants',
'field' => 'participants',
],
[
'label' => 'Status',
'field' => 'status',
],
[
'label' => 'Initiated At',
'field' => 'initiated_at',
],
[
'label' => 'Completed At',
'field' => 'completed_at',
],
],
],
'pmql' => '',
]);

$response = $this->apiCall('GET', '/api/1.1/processes/variables?processIds=1&savedSearchId=' . $savedSearch->id . '&onlyAvailable=');

$responseData = $response->json();

$filteredFields = collect($responseData['data'])->pluck('field');

$this->assertFalse($filteredFields->contains('case_number'));
$this->assertFalse($filteredFields->contains('case_title'));
$this->assertFalse($filteredFields->contains('name'));
$this->assertFalse($filteredFields->contains('active_tasks'));
$this->assertFalse($filteredFields->contains('process_version_alternative'));
$this->assertFalse($filteredFields->contains('participants'));
$this->assertFalse($filteredFields->contains('status'));
$this->assertFalse($filteredFields->contains('initiated_at'));
$this->assertFalse($filteredFields->contains('completed_at'));
}
}
Loading