From 110e7acc6259e8b311999421efae27b73c4271ce Mon Sep 17 00:00:00 2001 From: David Callizaya Date: Mon, 20 Jul 2026 19:20:34 -0400 Subject: [PATCH 1/6] feat: implement asset linking for environment variables --- .../Api/EnvironmentVariablesController.php | 39 ++- ProcessMaker/ImportExport/DependentType.php | 2 + .../Exporters/EnvironmentVariableExporter.php | 43 ++++ ProcessMaker/Models/EnvironmentVariable.php | 111 ++++++++ ...et_link_to_environment_variables_table.php | 29 +++ .../environment-variables/assetTypes.js | 54 ++++ .../components/AssetLinkFields.vue | 239 ++++++++++++++++++ .../CreateEnvironmentVariableModal.vue | 31 +-- .../processes/environment-variables/edit.js | 66 +++++ resources/lang/en/environmentVariables.php | 6 + .../environment-variables/edit.blade.php | 66 ++--- .../Feature/Api/EnvironmentVariablesTest.php | 63 +++++ .../Exporters/ScriptExporterTest.php | 100 +++++++- 13 files changed, 776 insertions(+), 73 deletions(-) create mode 100644 database/migrations/2026_07_20_000000_add_asset_link_to_environment_variables_table.php create mode 100644 resources/js/processes/environment-variables/assetTypes.js create mode 100644 resources/js/processes/environment-variables/components/AssetLinkFields.vue create mode 100644 resources/js/processes/environment-variables/edit.js diff --git a/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php b/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php index b599f49795..db05fad400 100644 --- a/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php +++ b/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php @@ -104,10 +104,12 @@ public function index(Request $request) */ public function store(Request $request) { - $request->validate(EnvironmentVariable::rules(), EnvironmentVariable::messages()); - $environment_variable = EnvironmentVariable::create($request->all()); + $data = $this->prepareAssetLinkInput($request->all()); + validator($data, EnvironmentVariable::rules(), EnvironmentVariable::messages())->validate(); + EnvironmentVariable::validateLinkedAssetExists($data); + $environment_variable = EnvironmentVariable::create($data); // Register the Event - EnvironmentVariablesCreated::dispatch($request->all()); + EnvironmentVariablesCreated::dispatch($data); return new EnvironmentVariableResource($environment_variable); } @@ -172,14 +174,21 @@ public function show(EnvironmentVariable $environment_variable) */ public function update(EnvironmentVariable $environment_variable, Request $request) { - $fields = ['name', 'description']; - if ($request->filled('value')) { + $data = $this->prepareAssetLinkInput($request->all()); + // Validate the request, passing in the existing variable to tweak unique rule on name + validator($data, EnvironmentVariable::rules($environment_variable), EnvironmentVariable::messages())->validate(); + EnvironmentVariable::validateLinkedAssetExists($data); + + $fields = ['name', 'description', 'asset_type', 'asset_uuid']; + // Only accept an explicit value when the variable is not linked to an asset. + if (!empty($data['asset_type']) && !empty($data['asset_uuid'])) { + // value is synced from the linked asset on save + } elseif (array_key_exists('value', $data) && $data['value'] !== null && $data['value'] !== '') { $fields[] = 'value'; } - // Validate the request, passing in the existing variable to tweak unique rule on name - $request->validate(EnvironmentVariable::rules($environment_variable)); + $original = $environment_variable->getOriginal(); - $environment_variable->fill($request->only($fields)); + $environment_variable->fill(collect($data)->only($fields)->all()); $environment_variable->save(); $changes = $environment_variable->getChanges(); @@ -190,6 +199,20 @@ public function update(EnvironmentVariable $environment_variable, Request $reque return new EnvironmentVariableResource($environment_variable); } + /** + * Normalize empty asset link fields to null so both-or-neither validation works. + */ + private function prepareAssetLinkInput(array $data): array + { + foreach (['asset_type', 'asset_uuid'] as $field) { + if (array_key_exists($field, $data) && $data[$field] === '') { + $data[$field] = null; + } + } + + return $data; + } + /** * @OA\Delete( * path="/environment_variables/{environment_variable_id}", diff --git a/ProcessMaker/ImportExport/DependentType.php b/ProcessMaker/ImportExport/DependentType.php index acd2db4e9b..f5000fe6a9 100644 --- a/ProcessMaker/ImportExport/DependentType.php +++ b/ProcessMaker/ImportExport/DependentType.php @@ -22,6 +22,8 @@ abstract class DependentType const ENVIRONMENT_VARIABLE_VALUE = 'environment_variables_value'; + const ENVIRONMENT_VARIABLE_ASSET = 'environment_variable_asset'; + const SCRIPT_EXECUTORS = 'script_executors'; const SUB_PROCESSES = 'sub_processes'; diff --git a/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php b/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php index 2161c90b46..41484992e8 100644 --- a/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php +++ b/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php @@ -2,6 +2,8 @@ namespace ProcessMaker\ImportExport\Exporters; +use Illuminate\Support\Facades\Log; +use ProcessMaker\Enums\ExporterMap; use ProcessMaker\ImportExport\DependentType; class EnvironmentVariableExporter extends ExporterBase @@ -17,10 +19,51 @@ class EnvironmentVariableExporter extends ExporterBase public function export() : void { $this->addReference(DependentType::ENVIRONMENT_VARIABLE_VALUE, $this->model->value); + + if (!$this->model->hasLinkedAsset()) { + return; + } + + $asset = $this->model->resolveLinkedAsset(); + if (!$asset) { + Log::warning('EnvironmentVariableExporter: linked asset not found during export', [ + 'environment_variable' => $this->model->name, + 'asset_type' => $this->model->asset_type, + 'asset_uuid' => $this->model->asset_uuid, + ]); + + return; + } + + $exporterClass = ExporterMap::getExporterClassForModel($asset); + if (!$exporterClass) { + Log::warning('EnvironmentVariableExporter: no exporter registered for linked asset', [ + 'environment_variable' => $this->model->name, + 'asset_type' => $this->model->asset_type, + ]); + + return; + } + + $this->addDependent(DependentType::ENVIRONMENT_VARIABLE_ASSET, $asset, $exporterClass); } public function import() : bool { + foreach ($this->getDependents(DependentType::ENVIRONMENT_VARIABLE_ASSET, true) as $dependent) { + $asset = $dependent->model; + if (!$asset) { + continue; + } + + $this->model->asset_type = get_class($asset); + $this->model->asset_uuid = $asset->uuid; + $this->model->value = (string) $asset->id; + + return $this->model->save(); + } + + // Non-linked env vars (or discarded asset dependents): restore exported value. $this->model->value = $this->getReference(DependentType::ENVIRONMENT_VARIABLE_VALUE); return $this->model->save(); diff --git a/ProcessMaker/Models/EnvironmentVariable.php b/ProcessMaker/Models/EnvironmentVariable.php index 8aaea1cf0c..c8a1b4cc20 100644 --- a/ProcessMaker/Models/EnvironmentVariable.php +++ b/ProcessMaker/Models/EnvironmentVariable.php @@ -3,8 +3,10 @@ namespace ProcessMaker\Models; use Exception; +use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\Log; use Illuminate\Validation\Rule; +use ProcessMaker\Enums\ExporterMap; use ProcessMaker\Traits\Exportable; /** @@ -13,6 +15,8 @@ * @OA\Property(property="name", type="string"), * @OA\Property(property="description", type="string"), * @OA\Property(property="value", type="string"), + * @OA\Property(property="asset_type", type="string", nullable=true), + * @OA\Property(property="asset_uuid", type="string", format="uuid", nullable=true), * ), * @OA\Schema( * schema="EnvironmentVariable", @@ -36,12 +40,24 @@ class EnvironmentVariable extends ProcessMakerModel 'name', 'description', 'value', + 'asset_type', + 'asset_uuid', ]; protected $hidden = [ 'value', ]; + protected static function boot() + { + parent::boot(); + + static::saving(function (self $environmentVariable) { + $environmentVariable->normalizeAssetLink(); + $environmentVariable->syncLinkedAssetValue(); + }); + } + /** * Store the encrypted version of the variable value here */ @@ -74,21 +90,116 @@ public static function rules($existing = null) { $unique = Rule::unique('environment_variables')->ignore($existing); $validVariableName = '/^[a-zA-Z][a-zA-Z_$0-9]*$/'; + $allowedAssetTypes = self::allowedAssetTypes(); return [ 'description' => 'required', 'value' => 'nullable', 'name' => ['required', "regex:{$validVariableName}", $unique], + 'asset_type' => [ + 'nullable', + 'string', + Rule::in($allowedAssetTypes), + 'required_with:asset_uuid', + ], + 'asset_uuid' => [ + 'nullable', + 'uuid', + 'required_with:asset_type', + ], ]; } + /** + * Validate that a linked asset exists when asset_type and asset_uuid are provided. + */ + public static function validateLinkedAssetExists(array $data): void + { + $assetType = $data['asset_type'] ?? null; + $assetUuid = $data['asset_uuid'] ?? null; + + if (!$assetType && !$assetUuid) { + return; + } + + if (!$assetType || !$assetUuid) { + return; + } + + if (!class_exists($assetType)) { + throw \Illuminate\Validation\ValidationException::withMessages([ + 'asset_type' => [__('The selected asset type is not available.')], + ]); + } + + if (!$assetType::where('uuid', $assetUuid)->exists()) { + throw \Illuminate\Validation\ValidationException::withMessages([ + 'asset_uuid' => [__('The selected asset could not be found.')], + ]); + } + } + public static function messages() { return [ 'name.regex' => trans('environmentVariables.validation.name.invalid_variable_name'), + 'asset_type.required_with' => trans('environmentVariables.validation.asset_type.required_with'), + 'asset_uuid.required_with' => trans('environmentVariables.validation.asset_uuid.required_with'), ]; } + /** + * Exportable model classes that may be linked to an environment variable. + */ + public static function allowedAssetTypes(): array + { + return collect(ExporterMap::TYPES) + ->map(fn (array $type) => $type[0]) + ->filter(fn (string $class) => class_exists($class)) + ->values() + ->all(); + } + + public function resolveLinkedAsset(): ?Model + { + if (!$this->asset_type || !$this->asset_uuid || !class_exists($this->asset_type)) { + return null; + } + + return $this->asset_type::where('uuid', $this->asset_uuid)->first(); + } + + public function hasLinkedAsset(): bool + { + return !empty($this->asset_type) && !empty($this->asset_uuid); + } + + protected function normalizeAssetLink(): void + { + if ($this->asset_type === '') { + $this->asset_type = null; + } + + if ($this->asset_uuid === '') { + $this->asset_uuid = null; + } + } + + /** + * Keep value in sync with the linked asset's numeric ID on this instance. + */ + public function syncLinkedAssetValue(): void + { + if (!$this->hasLinkedAsset()) { + return; + } + + $asset = $this->resolveLinkedAsset(); + if ($asset) { + $this->value = (string) $asset->id; + } + } + public static function getMetricsApiEndpoint() { $variable = self::where('name', 'METRICS_API_ENDPOINT')->first(); diff --git a/database/migrations/2026_07_20_000000_add_asset_link_to_environment_variables_table.php b/database/migrations/2026_07_20_000000_add_asset_link_to_environment_variables_table.php new file mode 100644 index 0000000000..564ea65a09 --- /dev/null +++ b/database/migrations/2026_07_20_000000_add_asset_link_to_environment_variables_table.php @@ -0,0 +1,29 @@ +string('asset_type')->nullable()->after('value'); + $table->uuid('asset_uuid')->nullable()->after('asset_type'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('environment_variables', function (Blueprint $table) { + $table->dropColumn(['asset_type', 'asset_uuid']); + }); + } +}; diff --git a/resources/js/processes/environment-variables/assetTypes.js b/resources/js/processes/environment-variables/assetTypes.js new file mode 100644 index 0000000000..de5b18201e --- /dev/null +++ b/resources/js/processes/environment-variables/assetTypes.js @@ -0,0 +1,54 @@ +/** + * Exportable asset types that can be linked to an environment variable. + * apiPath is the local API listing endpoint used by the asset picker. + */ +export default [ + { + class: "ProcessMaker\\Models\\Process", + label: "Process", + apiPath: "processes", + nameField: "name", + }, + { + class: "ProcessMaker\\Models\\Screen", + label: "Screen", + apiPath: "screens", + nameField: "title", + }, + { + class: "ProcessMaker\\Models\\Script", + label: "Script", + apiPath: "scripts", + nameField: "title", + }, + { + class: "ProcessMaker\\Plugins\\Collections\\Models\\Collection", + label: "Collection", + apiPath: "collections", + nameField: "name", + }, + { + class: "ProcessMaker\\Packages\\Connectors\\DataSources\\Models\\DataSource", + label: "Data Connector", + apiPath: "data_sources", + nameField: "name", + }, + { + class: "ProcessMaker\\Package\\PackageDecisionEngine\\Models\\DecisionTable", + label: "Decision Table", + apiPath: "decision-tables", + nameField: "title", + }, + { + class: "ProcessMaker\\Package\\PackageAi\\Models\\FlowGenie", + label: "FlowGenie", + apiPath: "flow-genies", + nameField: "name", + }, + { + class: "ProcessMaker\\Package\\PackagePmBlocks\\Models\\PmBlock", + label: "PM Block", + apiPath: "pm-blocks", + nameField: "name", + }, +]; diff --git a/resources/js/processes/environment-variables/components/AssetLinkFields.vue b/resources/js/processes/environment-variables/components/AssetLinkFields.vue new file mode 100644 index 0000000000..e2fe423587 --- /dev/null +++ b/resources/js/processes/environment-variables/components/AssetLinkFields.vue @@ -0,0 +1,239 @@ + + + diff --git a/resources/js/processes/environment-variables/components/CreateEnvironmentVariableModal.vue b/resources/js/processes/environment-variables/components/CreateEnvironmentVariableModal.vue index b8ec2f81df..776b711ed2 100644 --- a/resources/js/processes/environment-variables/components/CreateEnvironmentVariableModal.vue +++ b/resources/js/processes/environment-variables/components/CreateEnvironmentVariableModal.vue @@ -36,28 +36,22 @@ name="description" > - - - + + @endsection diff --git a/tests/Feature/Api/EnvironmentVariablesTest.php b/tests/Feature/Api/EnvironmentVariablesTest.php index eb6b17f28f..91f3aed813 100644 --- a/tests/Feature/Api/EnvironmentVariablesTest.php +++ b/tests/Feature/Api/EnvironmentVariablesTest.php @@ -6,6 +6,7 @@ use Illuminate\Support\Facades\Artisan; use Illuminate\Support\Facades\Hash; use ProcessMaker\Models\EnvironmentVariable; +use ProcessMaker\Models\Screen; use ProcessMaker\Models\User; use Tests\Feature\Shared\RequestHelper; use Tests\TestCase; @@ -255,4 +256,66 @@ public function testCreationOfMetricVariable() 'name' => 'METRICS_API_ENDPOINT', ]); } + + /** @test */ + public function test_it_should_create_an_environment_variable_linked_to_an_asset() + { + $screen = Screen::factory()->create(); + + $response = $this->apiCall('POST', self::API_TEST_VARIABLES, [ + 'name' => 'MY_SCREEN_ID', + 'description' => 'Linked screen id', + 'asset_type' => Screen::class, + 'asset_uuid' => $screen->uuid, + ]); + + $response->assertStatus(201); + $response->assertJsonFragment([ + 'name' => 'MY_SCREEN_ID', + 'asset_type' => Screen::class, + 'asset_uuid' => $screen->uuid, + ]); + + $variable = EnvironmentVariable::where('name', 'MY_SCREEN_ID')->firstOrFail(); + $this->assertEquals((string) $screen->id, $variable->value); + $this->assertEquals(Screen::class, $variable->asset_type); + $this->assertEquals($screen->uuid, $variable->asset_uuid); + } + + /** @test */ + public function test_it_should_reject_invalid_asset_link() + { + $response = $this->apiCall('POST', self::API_TEST_VARIABLES, [ + 'name' => 'MY_SCREEN_ID', + 'description' => 'Linked screen id', + 'asset_type' => Screen::class, + 'asset_uuid' => '00000000-0000-0000-0000-000000000000', + ]); + + $response->assertStatus(422); + $response->assertJsonValidationErrors(['asset_uuid']); + } + + /** @test */ + public function test_it_should_update_environment_variable_asset_link() + { + $screen = Screen::factory()->create(); + $variable = EnvironmentVariable::factory()->create([ + 'name' => 'MY_SCREEN_ID', + 'value' => 'old-value', + ]); + + $response = $this->apiCall('PUT', self::API_TEST_VARIABLES . '/' . $variable->id, [ + 'name' => 'MY_SCREEN_ID', + 'description' => 'Linked screen id', + 'asset_type' => Screen::class, + 'asset_uuid' => $screen->uuid, + ]); + + $response->assertStatus(200); + $variable->refresh(); + $this->assertEquals((string) $screen->id, $variable->value); + $this->assertEquals(Screen::class, $variable->asset_type); + $this->assertEquals($screen->uuid, $variable->asset_uuid); + } } diff --git a/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php b/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php index 5dcbb6b564..e0d39725b4 100644 --- a/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php +++ b/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php @@ -6,7 +6,9 @@ use Illuminate\Support\Facades\DB; use ProcessMaker\ImportExport\Exporters\ScriptExporter; use ProcessMaker\ImportExport\Options; +use ProcessMaker\ImportExport\DependentType; use ProcessMaker\Models\EnvironmentVariable; +use ProcessMaker\Models\Screen; use ProcessMaker\Models\Script; use ProcessMaker\Models\ScriptCategory; use ProcessMaker\Models\User; @@ -191,4 +193,100 @@ public function testWithDuplicatedEnvVariable() $this->assertDatabaseHas('environment_variables', ['name' => $environmentVariable1->name . '_2']); $this->assertDatabaseHas('environment_variables', ['name' => $environmentVariable2->name . '_2']); $this->assertDatabaseMissing('environment_variables', ['name' => $environmentVariable3->name . '_2']); - }} + } + + public function testLinkedAssetRemappedOnImportUpdateMode() + { + $screen = Screen::factory()->create(['title' => 'Linked Screen']); + $environmentVariable = EnvironmentVariable::factory()->create([ + 'name' => 'MY_SCREEN_ID', + 'description' => 'Screen id for scripts', + 'asset_type' => Screen::class, + 'asset_uuid' => $screen->uuid, + 'value' => (string) $screen->id, + ]); + $script = Script::factory()->create([ + 'title' => 'script with linked env var', + 'code' => 'export($script, ScriptExporter::class); + + $envVarPayload = collect($payload['export'])->first(function ($asset) { + return ($asset['model'] ?? null) === EnvironmentVariable::class; + }); + $this->assertNotNull($envVarPayload); + $this->assertTrue( + collect($envVarPayload['dependents'])->pluck('type')->contains(DependentType::ENVIRONMENT_VARIABLE_ASSET) + ); + + // Simulate a stale numeric ID on the target instance before update import. + // Bypass model sync (saving hook would rewrite value from the linked asset). + DB::table('environment_variables') + ->where('id', $environmentVariable->id) + ->update(['value' => encrypt('999999')]); + $this->assertEquals('999999', $environmentVariable->fresh()->value); + + $options = new Options([ + $script->uuid => ['mode' => 'update'], + $environmentVariable->uuid => ['mode' => 'update'], + $screen->uuid => ['mode' => 'update'], + ]); + $this->import($payload, $options); + + $screen->refresh(); + $environmentVariable->refresh(); + + $this->assertEquals(1, Screen::where('title', 'Linked Screen')->count()); + $this->assertEquals(1, EnvironmentVariable::where('name', 'MY_SCREEN_ID')->count()); + $this->assertEquals(Screen::class, $environmentVariable->asset_type); + $this->assertEquals($screen->uuid, $environmentVariable->asset_uuid); + $this->assertEquals((string) $screen->id, $environmentVariable->value); + } + + public function testLinkedAssetRemappedOnImportCopyMode() + { + $screen = Screen::factory()->create(['title' => 'Linked Screen']); + $environmentVariable = EnvironmentVariable::factory()->create([ + 'name' => 'MY_SCREEN_ID', + 'description' => 'Screen id for scripts', + 'asset_type' => Screen::class, + 'asset_uuid' => $screen->uuid, + 'value' => (string) $screen->id, + ]); + $script = Script::factory()->create([ + 'title' => 'script with linked env var', + 'code' => 'uuid; + $originalScreenId = $screen->id; + $originalEnvUuid = $environmentVariable->uuid; + + $payload = $this->export($script, ScriptExporter::class); + + $options = new Options([ + $script->uuid => ['mode' => 'copy'], + $environmentVariable->uuid => ['mode' => 'copy'], + $screen->uuid => ['mode' => 'copy'], + ]); + $this->import($payload, $options); + + // Originals remain unchanged. + $screen->refresh(); + $environmentVariable->refresh(); + $this->assertEquals($originalScreenUuid, $screen->uuid); + $this->assertEquals((string) $originalScreenId, $environmentVariable->value); + $this->assertEquals($originalScreenUuid, $environmentVariable->asset_uuid); + $this->assertEquals($originalEnvUuid, $environmentVariable->uuid); + + $copiedScreen = Screen::where('title', 'Linked Screen 2')->firstOrFail(); + $copiedVariable = EnvironmentVariable::where('name', 'MY_SCREEN_ID_2')->firstOrFail(); + + $this->assertNotEquals($originalScreenUuid, $copiedScreen->uuid); + $this->assertNotEquals($originalEnvUuid, $copiedVariable->uuid); + $this->assertEquals(Screen::class, $copiedVariable->asset_type); + $this->assertEquals($copiedScreen->uuid, $copiedVariable->asset_uuid); + $this->assertEquals((string) $copiedScreen->id, $copiedVariable->value); + } +} From d935512929d37c24b50aa3221b48f4c93a822897 Mon Sep 17 00:00:00 2001 From: David Callizaya Date: Tue, 21 Jul 2026 08:49:25 -0400 Subject: [PATCH 2/6] feat: add edit.js for environment variables processing --- webpack.mix.js | 1 + 1 file changed, 1 insertion(+) diff --git a/webpack.mix.js b/webpack.mix.js index 39dfe16484..bb38ecedd0 100644 --- a/webpack.mix.js +++ b/webpack.mix.js @@ -122,6 +122,7 @@ mix .js("resources/js/processes/scripts/preview.js", "public/js/processes/scripts") .js("resources/js/processes/export/index.js", "public/js/processes/export") .js("resources/js/processes/environment-variables/index.js", "public/js/processes/environment-variables") + .js("resources/js/processes/environment-variables/edit.js", "public/js/processes/environment-variables") .js("resources/js/processes/import/index.js", "public/js/processes/import") .js("resources/js/processes/screens/index.js", "public/js/processes/screens") .js("resources/js/processes/screens/edit.js", "public/js/processes/screens") From f4cd15855fc918f7d70fa550f3c89b9137064344 Mon Sep 17 00:00:00 2001 From: David Callizaya Date: Wed, 22 Jul 2026 17:19:02 -0400 Subject: [PATCH 3/6] feat: enhance environment variable asset linking validation and update logic --- .../Api/EnvironmentVariablesController.php | 14 ++-- ProcessMaker/Models/EnvironmentVariable.php | 50 ++++++++++++-- resources/lang/en/environmentVariables.php | 5 ++ .../Feature/Api/EnvironmentVariablesTest.php | 65 +++++++++++++++++++ 4 files changed, 123 insertions(+), 11 deletions(-) diff --git a/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php b/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php index db05fad400..e2250cf71a 100644 --- a/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php +++ b/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php @@ -106,7 +106,10 @@ public function store(Request $request) { $data = $this->prepareAssetLinkInput($request->all()); validator($data, EnvironmentVariable::rules(), EnvironmentVariable::messages())->validate(); - EnvironmentVariable::validateLinkedAssetExists($data); + $linkedAsset = EnvironmentVariable::validateAssetLinkConsistency($data); + if ($linkedAsset) { + $data['value'] = (string) $linkedAsset->id; + } $environment_variable = EnvironmentVariable::create($data); // Register the Event EnvironmentVariablesCreated::dispatch($data); @@ -177,12 +180,13 @@ public function update(EnvironmentVariable $environment_variable, Request $reque $data = $this->prepareAssetLinkInput($request->all()); // Validate the request, passing in the existing variable to tweak unique rule on name validator($data, EnvironmentVariable::rules($environment_variable), EnvironmentVariable::messages())->validate(); - EnvironmentVariable::validateLinkedAssetExists($data); + $linkedAsset = EnvironmentVariable::validateAssetLinkConsistency($data); $fields = ['name', 'description', 'asset_type', 'asset_uuid']; - // Only accept an explicit value when the variable is not linked to an asset. - if (!empty($data['asset_type']) && !empty($data['asset_uuid'])) { - // value is synced from the linked asset on save + if ($linkedAsset) { + // Guarantee value is always the selected asset's ID on this instance. + $data['value'] = (string) $linkedAsset->id; + $fields[] = 'value'; } elseif (array_key_exists('value', $data) && $data['value'] !== null && $data['value'] !== '') { $fields[] = 'value'; } diff --git a/ProcessMaker/Models/EnvironmentVariable.php b/ProcessMaker/Models/EnvironmentVariable.php index c8a1b4cc20..838b2fc697 100644 --- a/ProcessMaker/Models/EnvironmentVariable.php +++ b/ProcessMaker/Models/EnvironmentVariable.php @@ -6,6 +6,7 @@ use Illuminate\Database\Eloquent\Model; use Illuminate\Support\Facades\Log; use Illuminate\Validation\Rule; +use Illuminate\Validation\ValidationException; use ProcessMaker\Enums\ExporterMap; use ProcessMaker\Traits\Exportable; @@ -112,31 +113,68 @@ public static function rules($existing = null) /** * Validate that a linked asset exists when asset_type and asset_uuid are provided. + * + * @deprecated Use validateAssetLinkConsistency() which also checks value consistency. */ public static function validateLinkedAssetExists(array $data): void + { + self::validateAssetLinkConsistency($data); + } + + /** + * Validate asset_type + asset_uuid form a consistent pair, and that value (when sent) + * matches the selected asset's numeric ID. + * + * @return Model|null The resolved asset when a link is present + */ + public static function validateAssetLinkConsistency(array $data): ?Model { $assetType = $data['asset_type'] ?? null; $assetUuid = $data['asset_uuid'] ?? null; if (!$assetType && !$assetUuid) { - return; + return null; } if (!$assetType || !$assetUuid) { - return; + return null; } if (!class_exists($assetType)) { - throw \Illuminate\Validation\ValidationException::withMessages([ + throw ValidationException::withMessages([ 'asset_type' => [__('The selected asset type is not available.')], ]); } - if (!$assetType::where('uuid', $assetUuid)->exists()) { - throw \Illuminate\Validation\ValidationException::withMessages([ - 'asset_uuid' => [__('The selected asset could not be found.')], + if (!in_array($assetType, self::allowedAssetTypes(), true)) { + throw ValidationException::withMessages([ + 'asset_type' => [__('The selected asset type is not allowed.')], ]); } + + $asset = $assetType::where('uuid', $assetUuid)->first(); + if (!$asset) { + throw ValidationException::withMessages([ + 'asset_uuid' => [trans('environmentVariables.validation.asset_uuid.not_found_for_type')], + ]); + } + + // Guard against class hierarchy mismatches (e.g. subclass stored under a parent type). + if (get_class($asset) !== $assetType) { + throw ValidationException::withMessages([ + 'asset_type' => [trans('environmentVariables.validation.asset_type.mismatch')], + ]); + } + + if (array_key_exists('value', $data) && $data['value'] !== null && $data['value'] !== '') { + if ((string) $data['value'] !== (string) $asset->id) { + throw ValidationException::withMessages([ + 'value' => [trans('environmentVariables.validation.value.must_match_asset_id')], + ]); + } + } + + return $asset; } public static function messages() diff --git a/resources/lang/en/environmentVariables.php b/resources/lang/en/environmentVariables.php index e88b47057f..74788f0f10 100644 --- a/resources/lang/en/environmentVariables.php +++ b/resources/lang/en/environmentVariables.php @@ -7,9 +7,14 @@ ], 'asset_type' => [ 'required_with' => 'Asset type is required when an asset is selected.', + 'mismatch' => 'The selected asset does not match the specified asset type.', ], 'asset_uuid' => [ 'required_with' => 'An asset must be selected when an asset type is set.', + 'not_found_for_type' => 'The selected asset could not be found for the specified asset type.', + ], + 'value' => [ + 'must_match_asset_id' => 'The value must match the ID of the selected asset.', ], ], ]; diff --git a/tests/Feature/Api/EnvironmentVariablesTest.php b/tests/Feature/Api/EnvironmentVariablesTest.php index 91f3aed813..d544ded025 100644 --- a/tests/Feature/Api/EnvironmentVariablesTest.php +++ b/tests/Feature/Api/EnvironmentVariablesTest.php @@ -7,6 +7,7 @@ use Illuminate\Support\Facades\Hash; use ProcessMaker\Models\EnvironmentVariable; use ProcessMaker\Models\Screen; +use ProcessMaker\Models\Script; use ProcessMaker\Models\User; use Tests\Feature\Shared\RequestHelper; use Tests\TestCase; @@ -318,4 +319,68 @@ public function test_it_should_update_environment_variable_asset_link() $this->assertEquals(Screen::class, $variable->asset_type); $this->assertEquals($screen->uuid, $variable->asset_uuid); } + + /** @test */ + public function test_update_rejects_value_that_does_not_match_selected_asset_id() + { + $screen = Screen::factory()->create(); + $variable = EnvironmentVariable::factory()->create([ + 'name' => 'MY_SCREEN_ID', + 'value' => 'old-value', + ]); + + $response = $this->apiCall('PUT', self::API_TEST_VARIABLES . '/' . $variable->id, [ + 'name' => 'MY_SCREEN_ID', + 'description' => 'Linked screen id', + 'asset_type' => Screen::class, + 'asset_uuid' => $screen->uuid, + 'value' => '999999', + ]); + + $response->assertStatus(422); + $response->assertJsonValidationErrors(['value']); + } + + /** @test */ + public function test_update_accepts_value_matching_selected_asset_id() + { + $screen = Screen::factory()->create(); + $variable = EnvironmentVariable::factory()->create([ + 'name' => 'MY_SCREEN_ID', + 'value' => 'old-value', + ]); + + $response = $this->apiCall('PUT', self::API_TEST_VARIABLES . '/' . $variable->id, [ + 'name' => 'MY_SCREEN_ID', + 'description' => 'Linked screen id', + 'asset_type' => Screen::class, + 'asset_uuid' => $screen->uuid, + 'value' => (string) $screen->id, + ]); + + $response->assertStatus(200); + $variable->refresh(); + $this->assertEquals((string) $screen->id, $variable->value); + } + + /** @test */ + public function test_update_rejects_asset_uuid_not_found_for_asset_type() + { + $script = Script::factory()->create(); + $variable = EnvironmentVariable::factory()->create([ + 'name' => 'MY_SCREEN_ID', + 'value' => 'old-value', + ]); + + // Script UUID with Screen type — pair is inconsistent. + $response = $this->apiCall('PUT', self::API_TEST_VARIABLES . '/' . $variable->id, [ + 'name' => 'MY_SCREEN_ID', + 'description' => 'Linked screen id', + 'asset_type' => Screen::class, + 'asset_uuid' => $script->uuid, + ]); + + $response->assertStatus(422); + $response->assertJsonValidationErrors(['asset_uuid']); + } } From 509fe803552ce458458d4d60960c250683a1ade2 Mon Sep 17 00:00:00 2001 From: David Callizaya Date: Wed, 22 Jul 2026 17:44:29 -0400 Subject: [PATCH 4/6] refactor: remove asset_uuid from environment variable model and update related logic to use only value for asset linking --- .../Api/EnvironmentVariablesController.php | 10 +-- .../Exporters/EnvironmentVariableExporter.php | 3 +- ProcessMaker/Models/EnvironmentVariable.php | 81 +++++-------------- ...et_link_to_environment_variables_table.php | 3 +- .../components/AssetLinkFields.vue | 38 ++++----- .../CreateEnvironmentVariableModal.vue | 6 +- .../processes/environment-variables/edit.js | 8 +- resources/lang/en/environmentVariables.php | 7 +- .../environment-variables/edit.blade.php | 2 - .../Feature/Api/EnvironmentVariablesTest.php | 24 +++--- .../Exporters/ScriptExporterTest.php | 7 +- 11 files changed, 57 insertions(+), 132 deletions(-) diff --git a/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php b/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php index e2250cf71a..8a127f3a5c 100644 --- a/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php +++ b/ProcessMaker/Http/Controllers/Api/EnvironmentVariablesController.php @@ -182,7 +182,7 @@ public function update(EnvironmentVariable $environment_variable, Request $reque validator($data, EnvironmentVariable::rules($environment_variable), EnvironmentVariable::messages())->validate(); $linkedAsset = EnvironmentVariable::validateAssetLinkConsistency($data); - $fields = ['name', 'description', 'asset_type', 'asset_uuid']; + $fields = ['name', 'description', 'asset_type']; if ($linkedAsset) { // Guarantee value is always the selected asset's ID on this instance. $data['value'] = (string) $linkedAsset->id; @@ -204,14 +204,12 @@ public function update(EnvironmentVariable $environment_variable, Request $reque } /** - * Normalize empty asset link fields to null so both-or-neither validation works. + * Normalize empty asset_type to null. */ private function prepareAssetLinkInput(array $data): array { - foreach (['asset_type', 'asset_uuid'] as $field) { - if (array_key_exists($field, $data) && $data[$field] === '') { - $data[$field] = null; - } + if (array_key_exists('asset_type', $data) && $data['asset_type'] === '') { + $data['asset_type'] = null; } return $data; diff --git a/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php b/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php index 41484992e8..28e9d5178a 100644 --- a/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php +++ b/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php @@ -29,7 +29,7 @@ public function export() : void Log::warning('EnvironmentVariableExporter: linked asset not found during export', [ 'environment_variable' => $this->model->name, 'asset_type' => $this->model->asset_type, - 'asset_uuid' => $this->model->asset_uuid, + 'value' => $this->model->value, ]); return; @@ -57,7 +57,6 @@ public function import() : bool } $this->model->asset_type = get_class($asset); - $this->model->asset_uuid = $asset->uuid; $this->model->value = (string) $asset->id; return $this->model->save(); diff --git a/ProcessMaker/Models/EnvironmentVariable.php b/ProcessMaker/Models/EnvironmentVariable.php index 838b2fc697..38f6e9342b 100644 --- a/ProcessMaker/Models/EnvironmentVariable.php +++ b/ProcessMaker/Models/EnvironmentVariable.php @@ -17,7 +17,6 @@ * @OA\Property(property="description", type="string"), * @OA\Property(property="value", type="string"), * @OA\Property(property="asset_type", type="string", nullable=true), - * @OA\Property(property="asset_uuid", type="string", format="uuid", nullable=true), * ), * @OA\Schema( * schema="EnvironmentVariable", @@ -42,7 +41,6 @@ class EnvironmentVariable extends ProcessMakerModel 'description', 'value', 'asset_type', - 'asset_uuid', ]; protected $hidden = [ @@ -55,7 +53,6 @@ protected static function boot() static::saving(function (self $environmentVariable) { $environmentVariable->normalizeAssetLink(); - $environmentVariable->syncLinkedAssetValue(); }); } @@ -95,49 +92,37 @@ public static function rules($existing = null) return [ 'description' => 'required', - 'value' => 'nullable', + 'value' => [ + 'nullable', + 'required_with:asset_type', + ], 'name' => ['required', "regex:{$validVariableName}", $unique], 'asset_type' => [ 'nullable', 'string', Rule::in($allowedAssetTypes), - 'required_with:asset_uuid', - ], - 'asset_uuid' => [ - 'nullable', - 'uuid', - 'required_with:asset_type', ], ]; } /** - * Validate that a linked asset exists when asset_type and asset_uuid are provided. - * - * @deprecated Use validateAssetLinkConsistency() which also checks value consistency. - */ - public static function validateLinkedAssetExists(array $data): void - { - self::validateAssetLinkConsistency($data); - } - - /** - * Validate asset_type + asset_uuid form a consistent pair, and that value (when sent) - * matches the selected asset's numeric ID. + * Validate that asset_type + value resolve to an existing asset of that type. * * @return Model|null The resolved asset when a link is present */ public static function validateAssetLinkConsistency(array $data): ?Model { $assetType = $data['asset_type'] ?? null; - $assetUuid = $data['asset_uuid'] ?? null; + $value = $data['value'] ?? null; - if (!$assetType && !$assetUuid) { + if (!$assetType) { return null; } - if (!$assetType || !$assetUuid) { - return null; + if ($value === null || $value === '') { + throw ValidationException::withMessages([ + 'value' => [trans('environmentVariables.validation.value.required_with_asset_type')], + ]); } if (!class_exists($assetType)) { @@ -152,10 +137,10 @@ public static function validateAssetLinkConsistency(array $data): ?Model ]); } - $asset = $assetType::where('uuid', $assetUuid)->first(); + $asset = $assetType::find($value); if (!$asset) { throw ValidationException::withMessages([ - 'asset_uuid' => [trans('environmentVariables.validation.asset_uuid.not_found_for_type')], + 'value' => [trans('environmentVariables.validation.value.not_found_for_type')], ]); } @@ -166,14 +151,6 @@ public static function validateAssetLinkConsistency(array $data): ?Model ]); } - if (array_key_exists('value', $data) && $data['value'] !== null && $data['value'] !== '') { - if ((string) $data['value'] !== (string) $asset->id) { - throw ValidationException::withMessages([ - 'value' => [trans('environmentVariables.validation.value.must_match_asset_id')], - ]); - } - } - return $asset; } @@ -181,8 +158,7 @@ public static function messages() { return [ 'name.regex' => trans('environmentVariables.validation.name.invalid_variable_name'), - 'asset_type.required_with' => trans('environmentVariables.validation.asset_type.required_with'), - 'asset_uuid.required_with' => trans('environmentVariables.validation.asset_uuid.required_with'), + 'value.required_with' => trans('environmentVariables.validation.value.required_with_asset_type'), ]; } @@ -200,16 +176,20 @@ public static function allowedAssetTypes(): array public function resolveLinkedAsset(): ?Model { - if (!$this->asset_type || !$this->asset_uuid || !class_exists($this->asset_type)) { + if (!$this->hasLinkedAsset() || !class_exists($this->asset_type)) { + return null; + } + + if ($this->value === null || $this->value === '') { return null; } - return $this->asset_type::where('uuid', $this->asset_uuid)->first(); + return $this->asset_type::find($this->value); } public function hasLinkedAsset(): bool { - return !empty($this->asset_type) && !empty($this->asset_uuid); + return !empty($this->asset_type); } protected function normalizeAssetLink(): void @@ -217,25 +197,6 @@ protected function normalizeAssetLink(): void if ($this->asset_type === '') { $this->asset_type = null; } - - if ($this->asset_uuid === '') { - $this->asset_uuid = null; - } - } - - /** - * Keep value in sync with the linked asset's numeric ID on this instance. - */ - public function syncLinkedAssetValue(): void - { - if (!$this->hasLinkedAsset()) { - return; - } - - $asset = $this->resolveLinkedAsset(); - if ($asset) { - $this->value = (string) $asset->id; - } } public static function getMetricsApiEndpoint() diff --git a/database/migrations/2026_07_20_000000_add_asset_link_to_environment_variables_table.php b/database/migrations/2026_07_20_000000_add_asset_link_to_environment_variables_table.php index 564ea65a09..370df325dd 100644 --- a/database/migrations/2026_07_20_000000_add_asset_link_to_environment_variables_table.php +++ b/database/migrations/2026_07_20_000000_add_asset_link_to_environment_variables_table.php @@ -13,7 +13,6 @@ public function up(): void { Schema::table('environment_variables', function (Blueprint $table) { $table->string('asset_type')->nullable()->after('value'); - $table->uuid('asset_uuid')->nullable()->after('asset_type'); }); } @@ -23,7 +22,7 @@ public function up(): void public function down(): void { Schema::table('environment_variables', function (Blueprint $table) { - $table->dropColumn(['asset_type', 'asset_uuid']); + $table->dropColumn(['asset_type']); }); } }; diff --git a/resources/js/processes/environment-variables/components/AssetLinkFields.vue b/resources/js/processes/environment-variables/components/AssetLinkFields.vue index e2fe423587..4164373a7c 100644 --- a/resources/js/processes/environment-variables/components/AssetLinkFields.vue +++ b/resources/js/processes/environment-variables/components/AssetLinkFields.vue @@ -18,8 +18,8 @@ { const items = response.data.data || []; this.assetOptions = items; - this.selectedAsset = items.find((item) => item.uuid === this.assetUuid) || null; + this.selectedAsset = items.find((item) => String(item.id) === assetId) || null; if (!this.selectedAsset) { - // Fallback: keep a stub so the UUID remains visible until search finds it this.selectedAsset = { - uuid: this.assetUuid, - id: this.value || "", - [this.nameField]: this.assetUuid, + id: assetId, + [this.nameField]: assetId, }; this.assetOptions = [this.selectedAsset, ...items]; } }) .catch(() => { this.selectedAsset = { - uuid: this.assetUuid, - id: this.value || "", - [this.nameField]: this.assetUuid, + id: assetId, + [this.nameField]: assetId, }; this.assetOptions = [this.selectedAsset]; }) diff --git a/resources/js/processes/environment-variables/components/CreateEnvironmentVariableModal.vue b/resources/js/processes/environment-variables/components/CreateEnvironmentVariableModal.vue index 776b711ed2..77b52b5a73 100644 --- a/resources/js/processes/environment-variables/components/CreateEnvironmentVariableModal.vue +++ b/resources/js/processes/environment-variables/components/CreateEnvironmentVariableModal.vue @@ -38,7 +38,6 @@ @@ -60,7 +59,6 @@ description: '', value: '', assetType: null, - assetUuid: null, disabled: false, } }, @@ -70,7 +68,6 @@ this.description = ''; this.value = ''; this.assetType = null; - this.assetUuid = null; this.errors = {}; this.disabled = false; }, @@ -84,9 +81,8 @@ ProcessMaker.apiClient.post('environment_variables', { name: this.name, description: this.description, - value: this.assetType ? null : this.value, + value: this.value, asset_type: this.assetType, - asset_uuid: this.assetUuid, }) .then(response => { ProcessMaker.alert(this.$t('The environment variable was created.'), 'success'); diff --git a/resources/js/processes/environment-variables/edit.js b/resources/js/processes/environment-variables/edit.js index 802fb9936f..47d64ae5d4 100644 --- a/resources/js/processes/environment-variables/edit.js +++ b/resources/js/processes/environment-variables/edit.js @@ -16,14 +16,12 @@ new Vue({ description: initial.description, value: initial.value || null, asset_type: initial.asset_type || null, - asset_uuid: initial.asset_uuid || null, }, errors: { name: null, description: null, value: null, asset_type: null, - asset_uuid: null, }, }; }, @@ -34,7 +32,6 @@ new Vue({ description: null, value: null, asset_type: null, - asset_uuid: null, }; }, onClose() { @@ -46,11 +43,8 @@ new Vue({ name: this.formData.name, description: this.formData.description, asset_type: this.formData.asset_type, - asset_uuid: this.formData.asset_uuid, + value: this.formData.value, }; - if (!this.formData.asset_type && this.formData.value) { - payload.value = this.formData.value; - } ProcessMaker.apiClient.put(`environment_variables/${this.formData.id}`, payload) .then(() => { ProcessMaker.alert(this.$t("The environment variable was saved."), "success"); diff --git a/resources/lang/en/environmentVariables.php b/resources/lang/en/environmentVariables.php index 74788f0f10..94f305b391 100644 --- a/resources/lang/en/environmentVariables.php +++ b/resources/lang/en/environmentVariables.php @@ -6,14 +6,11 @@ 'invalid_variable_name' => 'Name has to start with a letter and can contain only letters, numbers, and underscores (_).', ], 'asset_type' => [ - 'required_with' => 'Asset type is required when an asset is selected.', 'mismatch' => 'The selected asset does not match the specified asset type.', ], - 'asset_uuid' => [ - 'required_with' => 'An asset must be selected when an asset type is set.', - 'not_found_for_type' => 'The selected asset could not be found for the specified asset type.', - ], 'value' => [ + 'required_with_asset_type' => 'A value (asset ID) is required when an asset type is set.', + 'not_found_for_type' => 'The selected asset could not be found for the specified asset type.', 'must_match_asset_id' => 'The value must match the ID of the selected asset.', ], ], diff --git a/resources/views/processes/environment-variables/edit.blade.php b/resources/views/processes/environment-variables/edit.blade.php index 85a71b5f63..ec70e02e3e 100644 --- a/resources/views/processes/environment-variables/edit.blade.php +++ b/resources/views/processes/environment-variables/edit.blade.php @@ -34,7 +34,6 @@ name), description: @json($environmentVariable->description), asset_type: @json($environmentVariable->asset_type), - asset_uuid: @json($environmentVariable->asset_uuid), // Safe to expose when linked: value is the asset numeric ID, not a secret. value: @json($environmentVariable->asset_type ? $environmentVariable->value : null), }; diff --git a/tests/Feature/Api/EnvironmentVariablesTest.php b/tests/Feature/Api/EnvironmentVariablesTest.php index d544ded025..2434a85ea3 100644 --- a/tests/Feature/Api/EnvironmentVariablesTest.php +++ b/tests/Feature/Api/EnvironmentVariablesTest.php @@ -267,20 +267,18 @@ public function test_it_should_create_an_environment_variable_linked_to_an_asset 'name' => 'MY_SCREEN_ID', 'description' => 'Linked screen id', 'asset_type' => Screen::class, - 'asset_uuid' => $screen->uuid, + 'value' => (string) $screen->id, ]); $response->assertStatus(201); $response->assertJsonFragment([ 'name' => 'MY_SCREEN_ID', 'asset_type' => Screen::class, - 'asset_uuid' => $screen->uuid, ]); $variable = EnvironmentVariable::where('name', 'MY_SCREEN_ID')->firstOrFail(); $this->assertEquals((string) $screen->id, $variable->value); $this->assertEquals(Screen::class, $variable->asset_type); - $this->assertEquals($screen->uuid, $variable->asset_uuid); } /** @test */ @@ -290,11 +288,11 @@ public function test_it_should_reject_invalid_asset_link() 'name' => 'MY_SCREEN_ID', 'description' => 'Linked screen id', 'asset_type' => Screen::class, - 'asset_uuid' => '00000000-0000-0000-0000-000000000000', + 'value' => '999999', ]); $response->assertStatus(422); - $response->assertJsonValidationErrors(['asset_uuid']); + $response->assertJsonValidationErrors(['value']); } /** @test */ @@ -310,20 +308,18 @@ public function test_it_should_update_environment_variable_asset_link() 'name' => 'MY_SCREEN_ID', 'description' => 'Linked screen id', 'asset_type' => Screen::class, - 'asset_uuid' => $screen->uuid, + 'value' => (string) $screen->id, ]); $response->assertStatus(200); $variable->refresh(); $this->assertEquals((string) $screen->id, $variable->value); $this->assertEquals(Screen::class, $variable->asset_type); - $this->assertEquals($screen->uuid, $variable->asset_uuid); } /** @test */ - public function test_update_rejects_value_that_does_not_match_selected_asset_id() + public function test_update_rejects_value_not_found_for_asset_type() { - $screen = Screen::factory()->create(); $variable = EnvironmentVariable::factory()->create([ 'name' => 'MY_SCREEN_ID', 'value' => 'old-value', @@ -333,7 +329,6 @@ public function test_update_rejects_value_that_does_not_match_selected_asset_id( 'name' => 'MY_SCREEN_ID', 'description' => 'Linked screen id', 'asset_type' => Screen::class, - 'asset_uuid' => $screen->uuid, 'value' => '999999', ]); @@ -354,7 +349,6 @@ public function test_update_accepts_value_matching_selected_asset_id() 'name' => 'MY_SCREEN_ID', 'description' => 'Linked screen id', 'asset_type' => Screen::class, - 'asset_uuid' => $screen->uuid, 'value' => (string) $screen->id, ]); @@ -364,7 +358,7 @@ public function test_update_accepts_value_matching_selected_asset_id() } /** @test */ - public function test_update_rejects_asset_uuid_not_found_for_asset_type() + public function test_update_rejects_value_id_of_wrong_asset_type() { $script = Script::factory()->create(); $variable = EnvironmentVariable::factory()->create([ @@ -372,15 +366,15 @@ public function test_update_rejects_asset_uuid_not_found_for_asset_type() 'value' => 'old-value', ]); - // Script UUID with Screen type — pair is inconsistent. + // Script ID with Screen type — pair is inconsistent. $response = $this->apiCall('PUT', self::API_TEST_VARIABLES . '/' . $variable->id, [ 'name' => 'MY_SCREEN_ID', 'description' => 'Linked screen id', 'asset_type' => Screen::class, - 'asset_uuid' => $script->uuid, + 'value' => (string) $script->id, ]); $response->assertStatus(422); - $response->assertJsonValidationErrors(['asset_uuid']); + $response->assertJsonValidationErrors(['value']); } } diff --git a/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php b/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php index e0d39725b4..98fcbeef25 100644 --- a/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php +++ b/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php @@ -202,7 +202,6 @@ public function testLinkedAssetRemappedOnImportUpdateMode() 'name' => 'MY_SCREEN_ID', 'description' => 'Screen id for scripts', 'asset_type' => Screen::class, - 'asset_uuid' => $screen->uuid, 'value' => (string) $screen->id, ]); $script = Script::factory()->create([ @@ -221,7 +220,6 @@ public function testLinkedAssetRemappedOnImportUpdateMode() ); // Simulate a stale numeric ID on the target instance before update import. - // Bypass model sync (saving hook would rewrite value from the linked asset). DB::table('environment_variables') ->where('id', $environmentVariable->id) ->update(['value' => encrypt('999999')]); @@ -240,7 +238,6 @@ public function testLinkedAssetRemappedOnImportUpdateMode() $this->assertEquals(1, Screen::where('title', 'Linked Screen')->count()); $this->assertEquals(1, EnvironmentVariable::where('name', 'MY_SCREEN_ID')->count()); $this->assertEquals(Screen::class, $environmentVariable->asset_type); - $this->assertEquals($screen->uuid, $environmentVariable->asset_uuid); $this->assertEquals((string) $screen->id, $environmentVariable->value); } @@ -251,7 +248,6 @@ public function testLinkedAssetRemappedOnImportCopyMode() 'name' => 'MY_SCREEN_ID', 'description' => 'Screen id for scripts', 'asset_type' => Screen::class, - 'asset_uuid' => $screen->uuid, 'value' => (string) $screen->id, ]); $script = Script::factory()->create([ @@ -277,7 +273,7 @@ public function testLinkedAssetRemappedOnImportCopyMode() $environmentVariable->refresh(); $this->assertEquals($originalScreenUuid, $screen->uuid); $this->assertEquals((string) $originalScreenId, $environmentVariable->value); - $this->assertEquals($originalScreenUuid, $environmentVariable->asset_uuid); + $this->assertEquals(Screen::class, $environmentVariable->asset_type); $this->assertEquals($originalEnvUuid, $environmentVariable->uuid); $copiedScreen = Screen::where('title', 'Linked Screen 2')->firstOrFail(); @@ -286,7 +282,6 @@ public function testLinkedAssetRemappedOnImportCopyMode() $this->assertNotEquals($originalScreenUuid, $copiedScreen->uuid); $this->assertNotEquals($originalEnvUuid, $copiedVariable->uuid); $this->assertEquals(Screen::class, $copiedVariable->asset_type); - $this->assertEquals($copiedScreen->uuid, $copiedVariable->asset_uuid); $this->assertEquals((string) $copiedScreen->id, $copiedVariable->value); } } From a51465ca37741097fb2b1be85b665f70210d245b Mon Sep 17 00:00:00 2001 From: David Callizaya Date: Wed, 22 Jul 2026 18:36:17 -0400 Subject: [PATCH 5/6] feat: implement logic to handle custom linked assets during environment variable import, including warnings for missing assets --- .../Exporters/EnvironmentVariableExporter.php | 20 +++- .../Exporters/ScriptExporterTest.php | 101 +++++++++++++++++- tests/Feature/ImportExport/HelperTrait.php | 6 +- 3 files changed, 119 insertions(+), 8 deletions(-) diff --git a/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php b/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php index 28e9d5178a..2a54c70c3a 100644 --- a/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php +++ b/ProcessMaker/ImportExport/Exporters/EnvironmentVariableExporter.php @@ -52,17 +52,27 @@ public function import() : bool { foreach ($this->getDependents(DependentType::ENVIRONMENT_VARIABLE_ASSET, true) as $dependent) { $asset = $dependent->model; - if (!$asset) { - continue; + if ($asset && $asset->exists) { + $this->model->asset_type = get_class($asset); + $this->model->value = (string) $asset->id; + + return $this->model->save(); } + } - $this->model->asset_type = get_class($asset); - $this->model->value = (string) $asset->id; + if ($this->model->asset_type) { + // Linked in source, but asset was not imported and was not found on target. + $this->logger?->addWarning(__( + 'Asset linked to environment variable ":env_variable" was missing on import; link and value were cleared', + ['env_variable' => $this->model->name] + )); + $this->model->asset_type = null; + $this->model->value = ''; return $this->model->save(); } - // Non-linked env vars (or discarded asset dependents): restore exported value. + // Standard case (non-linked) env var: restore secret/value from export. $this->model->value = $this->getReference(DependentType::ENVIRONMENT_VARIABLE_VALUE); return $this->model->save(); diff --git a/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php b/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php index 98fcbeef25..a432e59ffc 100644 --- a/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php +++ b/tests/Feature/ImportExport/Exporters/ScriptExporterTest.php @@ -4,14 +4,16 @@ use Database\Seeders\CategorySystemSeeder; use Illuminate\Support\Facades\DB; +use ProcessMaker\ImportExport\DependentType; use ProcessMaker\ImportExport\Exporters\ScriptExporter; +use ProcessMaker\ImportExport\Logger; use ProcessMaker\ImportExport\Options; -use ProcessMaker\ImportExport\DependentType; use ProcessMaker\Models\EnvironmentVariable; use ProcessMaker\Models\Screen; use ProcessMaker\Models\Script; use ProcessMaker\Models\ScriptCategory; use ProcessMaker\Models\User; +use ReflectionClass; use Tests\Feature\ImportExport\HelperTrait; use Tests\TestCase; @@ -284,4 +286,101 @@ public function testLinkedAssetRemappedOnImportCopyMode() $this->assertEquals(Screen::class, $copiedVariable->asset_type); $this->assertEquals((string) $copiedScreen->id, $copiedVariable->value); } + + /** + * Scenario A: linked asset is discarded but already exists on target — remap value to that asset. + */ + public function testLinkedAssetRemappedWhenDiscardedButExistsOnTarget() + { + $screen = Screen::factory()->create(['title' => 'Existing Linked Screen']); + $environmentVariableName = 'MY_SCREEN_ID'; + $environmentVariable = EnvironmentVariable::factory()->create([ + 'name' => $environmentVariableName, + 'description' => 'Screen id for scripts', + 'asset_type' => Screen::class, + 'value' => (string) $screen->id, + ]); + $script = Script::factory()->create([ + 'title' => 'script with linked env var discard exists', + 'code' => 'export($script, ScriptExporter::class); + + // Stale value before import; asset itself stays and is discarded (not updated). + DB::table('environment_variables') + ->where('id', $environmentVariable->id) + ->update(['value' => encrypt('999999')]); + + $options = new Options([ + $script->uuid => ['mode' => 'update'], + $environmentVariable->uuid => ['mode' => 'update'], + $screen->uuid => ['mode' => 'discard'], + ]); + $this->import($payload, $options); + + $screen->refresh(); + // Get the environment variable by name after import + $environmentVariable = EnvironmentVariable::where('name', $environmentVariableName)->firstOrFail(); + + $this->assertEquals(1, Screen::whereLike('title', 'Existing Linked Screen%')->count()); + $this->assertEquals(Screen::class, $environmentVariable->asset_type); + $this->assertEquals((string) $screen->id, $environmentVariable->value); + } + + /** + * Scenario B: linked asset is missing on target — clear link and value, warn in import summary. + */ + public function testLinkedAssetClearedWhenMissingOnImport() + { + $screen = Screen::factory()->create(['title' => 'Missing Linked Screen']); + $environmentVariableName = 'MY_SCREEN_ID'; + $environmentVariable = EnvironmentVariable::factory()->create([ + 'name' => $environmentVariableName, + 'description' => 'Screen id for scripts', + 'asset_type' => Screen::class, + 'value' => (string) $screen->id, + ]); + $script = Script::factory()->create([ + 'title' => 'script with linked env var missing asset', + 'code' => 'export($script, ScriptExporter::class); + $screenUuid = $screen->uuid; + + // Remove the asset from the target instance before import. + $screen->delete(); + $this->assertNull(Screen::where('uuid', $screenUuid)->first()); + + $logger = new Logger(); + $options = new Options([ + $script->uuid => ['mode' => 'update'], + $environmentVariable->uuid => ['mode' => 'update'], + $screenUuid => ['mode' => 'discard'], + ]); + $this->import($payload, $options, $logger); + + // Get the environment variable by name after import + $environmentVariable = EnvironmentVariable::where('name', $environmentVariableName)->firstOrFail(); + + $this->assertNull($environmentVariable->asset_type); + $this->assertSame('', $environmentVariable->value); + + $warnings = $this->getLoggerWarnings($logger); + $expectedWarning = __( + 'Asset linked to environment variable ":env_variable" was missing on import; link and value were cleared', + ['env_variable' => 'MY_SCREEN_ID'] + ); + $this->assertContains($expectedWarning, $warnings); + } + + private function getLoggerWarnings(Logger $logger): array + { + $reflection = new ReflectionClass($logger); + $property = $reflection->getProperty('warnings'); + $property->setAccessible(true); + + return $property->getValue($logger); + } } diff --git a/tests/Feature/ImportExport/HelperTrait.php b/tests/Feature/ImportExport/HelperTrait.php index 2b985453e7..f10061833a 100644 --- a/tests/Feature/ImportExport/HelperTrait.php +++ b/tests/Feature/ImportExport/HelperTrait.php @@ -75,12 +75,14 @@ public function export($model, $exporterClass, $options = null, $ignoreExplicitE return $exporter->payload(); } - public function import($payload, $options = null) + public function import($payload, $options = null, $logger = null) { $options = $options ?: new Options([]); - $importer = new Importer($payload, $options); + $importer = new Importer($payload, $options, $logger); $importer->previewImport(); $importer->doImport(); + + return $importer; } public function makeOptions($options = []) From cf75c38323cb9fcb2d95a631e24f44c524c5c87b Mon Sep 17 00:00:00 2001 From: David Callizaya Date: Wed, 22 Jul 2026 22:35:53 -0400 Subject: [PATCH 6/6] fix: update right api paths for decision tables and flow genies --- resources/js/processes/environment-variables/assetTypes.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/resources/js/processes/environment-variables/assetTypes.js b/resources/js/processes/environment-variables/assetTypes.js index de5b18201e..e03a0892bf 100644 --- a/resources/js/processes/environment-variables/assetTypes.js +++ b/resources/js/processes/environment-variables/assetTypes.js @@ -36,13 +36,13 @@ export default [ { class: "ProcessMaker\\Package\\PackageDecisionEngine\\Models\\DecisionTable", label: "Decision Table", - apiPath: "decision-tables", + apiPath: "decision_tables", nameField: "title", }, { class: "ProcessMaker\\Package\\PackageAi\\Models\\FlowGenie", label: "FlowGenie", - apiPath: "flow-genies", + apiPath: "package-ai/flow_genies", nameField: "name", }, {