Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,15 @@ 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();
$linkedAsset = EnvironmentVariable::validateAssetLinkConsistency($data);
if ($linkedAsset) {
$data['value'] = (string) $linkedAsset->id;
}
$environment_variable = EnvironmentVariable::create($data);
// Register the Event
EnvironmentVariablesCreated::dispatch($request->all());
EnvironmentVariablesCreated::dispatch($data);

return new EnvironmentVariableResource($environment_variable);
}
Expand Down Expand Up @@ -172,14 +177,22 @@ 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());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 If asset_uuid is retained, update validation does not actually guarantee both-or-neither. It validates only the incoming payload, so clearing one link field while omitting the other can persist an inconsistent pair. Validate the merged resulting state or update both fields atomically.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added the validation of assets linking:

Image

If the asset does not match a valid one returns an 422

// Validate the request, passing in the existing variable to tweak unique rule on name
validator($data, EnvironmentVariable::rules($environment_variable), EnvironmentVariable::messages())->validate();
$linkedAsset = EnvironmentVariable::validateAssetLinkConsistency($data);

$fields = ['name', 'description', 'asset_type'];
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';
}
// 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();
Expand All @@ -190,6 +203,18 @@ public function update(EnvironmentVariable $environment_variable, Request $reque
return new EnvironmentVariableResource($environment_variable);
}

/**
* Normalize empty asset_type to null.
*/
private function prepareAssetLinkInput(array $data): array
{
if (array_key_exists('asset_type', $data) && $data['asset_type'] === '') {
$data['asset_type'] = null;
}

return $data;
}

/**
* @OA\Delete(
* path="/environment_variables/{environment_variable_id}",
Expand Down
2 changes: 2 additions & 0 deletions ProcessMaker/ImportExport/DependentType.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -17,10 +19,50 @@ 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,
'value' => $this->model->value,
]);

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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔴 A discarded or unresolved asset (Custom Import) can be returned as an empty model. The truthiness check at line 55 passes, then the importer saves asset_uuid = null and an empty numeric value. The fallback at line 66 is therefore bypassed, leaving scripts with a broken environment variable after Import or DevLink. Only use a persisted dependent asset; otherwise fail clearly or restore a deliberately unlinked value without persisting a half-link.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This scenario is covered and tested in this PR: #8928

continue;
}

$this->model->asset_type = get_class($asset);
$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();
Expand Down
112 changes: 111 additions & 1 deletion ProcessMaker/Models/EnvironmentVariable.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@
namespace ProcessMaker\Models;

use Exception;
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;

/**
Expand All @@ -13,6 +16,7 @@
* @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\Schema(
* schema="EnvironmentVariable",
Expand All @@ -36,12 +40,22 @@ class EnvironmentVariable extends ProcessMakerModel
'name',
'description',
'value',
'asset_type',
];

protected $hidden = [
'value',
];

protected static function boot()
{
parent::boot();

static::saving(function (self $environmentVariable) {
$environmentVariable->normalizeAssetLink();
});
}

/**
* Store the encrypted version of the variable value here
*/
Expand Down Expand Up @@ -74,21 +88,117 @@ 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',
'value' => [
'nullable',
'required_with:asset_type',
],
'name' => ['required', "regex:{$validVariableName}", $unique],
'asset_type' => [
'nullable',
'string',
Rule::in($allowedAssetTypes),
],
];
}

/**
* 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;
$value = $data['value'] ?? null;

if (!$assetType) {
return null;
}

if ($value === null || $value === '') {
throw ValidationException::withMessages([
'value' => [trans('environmentVariables.validation.value.required_with_asset_type')],
]);
}

if (!class_exists($assetType)) {
throw ValidationException::withMessages([
'asset_type' => [__('The selected asset type is not available.')],
]);
}

if (!in_array($assetType, self::allowedAssetTypes(), true)) {
throw ValidationException::withMessages([
'asset_type' => [__('The selected asset type is not allowed.')],
]);
}

$asset = $assetType::find($value);
if (!$asset) {
throw ValidationException::withMessages([
'value' => [trans('environmentVariables.validation.value.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')],
]);
}

return $asset;
}

public static function messages()
{
return [
'name.regex' => trans('environmentVariables.validation.name.invalid_variable_name'),
'value.required_with' => trans('environmentVariables.validation.value.required_with_asset_type'),
];
}

/**
* 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->hasLinkedAsset() || !class_exists($this->asset_type)) {
return null;
}

if ($this->value === null || $this->value === '') {
return null;
}

return $this->asset_type::find($this->value);
}

public function hasLinkedAsset(): bool
{
return !empty($this->asset_type);
}

protected function normalizeAssetLink(): void
{
if ($this->asset_type === '') {
$this->asset_type = null;
}
}

public static function getMetricsApiEndpoint()
{
$variable = self::where('name', 'METRICS_API_ENDPOINT')->first();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\Schema;

return new class extends Migration
{
/**
* Run the migrations.
*/
public function up(): void
{
Schema::table('environment_variables', function (Blueprint $table) {
$table->string('asset_type')->nullable()->after('value');
});
}

/**
* Reverse the migrations.
*/
public function down(): void
{
Schema::table('environment_variables', function (Blueprint $table) {
$table->dropColumn(['asset_type']);
});
}
};
54 changes: 54 additions & 0 deletions resources/js/processes/environment-variables/assetTypes.js
Original file line number Diff line number Diff line change
@@ -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: "package-ai/flow_genies",
nameField: "name",
},
{
class: "ProcessMaker\\Package\\PackagePmBlocks\\Models\\PmBlock",
label: "PM Block",
apiPath: "pm-blocks",
nameField: "name",
},
];
Loading
Loading