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
12 changes: 6 additions & 6 deletions app/Http/Controllers/API/MeasureController.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class MeasureController extends Controller
{
public function index()
{
abort_if(Auth::User()->role !== 4, Response::HTTP_FORBIDDEN, '403 Forbidden');
abort_if(Auth::User()->isAPI(), Response::HTTP_FORBIDDEN, '403 Forbidden');

$measures = Measure::all();

Expand All @@ -21,9 +21,9 @@ public function index()

public function store(Request $request)
{
abort_if(Auth::User()->role !== 4, Response::HTTP_FORBIDDEN, '403 Forbidden');
abort_if(Auth::User()->isAPI(), Response::HTTP_FORBIDDEN, '403 Forbidden');

$measure = Measure::create($request->all());
$measure = Measure::query()->create($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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "MeasureController.php" | head -5

Repository: dbarzin/deming

Length of output: 151


🏁 Script executed:

find . -type f -name "Measure.php" | grep -E "(Models|model)" | head -5

Repository: dbarzin/deming

Length of output: 82


🏁 Script executed:

cat -n app/Http/Controllers/API/MeasureController.php | sed -n '1,50p'

Repository: dbarzin/deming

Length of output: 1779


🏁 Script executed:

cat -n app/Models/Measure.php

Repository: dbarzin/deming

Length of output: 2427


🏁 Script executed:

cat -n app/Http/Controllers/MeasureController.php | sed -n '1,100p'

Repository: dbarzin/deming

Length of output: 4806


🏁 Script executed:

cat -n app/Http/Controllers/MeasureController.php | sed -n '50,150p'

Repository: dbarzin/deming

Length of output: 5414


🏁 Script executed:

grep -n "setAttribute\|Attribute\|mutate" app/Models/Measure.php

Repository: dbarzin/deming

Length of output: 40


🏁 Script executed:

fd -t f "migration.*measure" --exec grep -l "attributes" {} \; 2>/dev/null | head -3

Repository: dbarzin/deming

Length of output: 40


🏁 Script executed:

cat -n app/Http/Controllers/MeasureController.php | sed -n '150,250p'

Repository: dbarzin/deming

Length of output: 4052


🏁 Script executed:

find . -path "./database/migrations/*" -name "*.php" | xargs grep -l "measures\|Measure" | head -3

Repository: dbarzin/deming

Length of output: 242


🏁 Script executed:

cat -n app/Http/Controllers/MeasureController.php | sed -n '260,320p'

Repository: dbarzin/deming

Length of output: 2112


🏁 Script executed:

cat -n ./database/migrations/2019_08_09_084322_create_measures_table.php

Repository: dbarzin/deming

Length of output: 1591


🏁 Script executed:

cat -n app/Http/Controllers/MeasureController.php | sed -n '350,410p'

Repository: dbarzin/deming

Length of output: 2282


Normalize attributes before mass create and update in the API controller.

Both store() (line 26) and update() (line 45) pass $request->all() directly to create/update without normalizing the attributes field. The web controller explicitly converts attributes array to space-separated string using implode(' ', ...) before saving (see store at line 199 and update at line 393). The database stores this as TEXT, and the parsing logic uses explode(' ') expecting a string. If API clients send attributes as an array, an array object gets stored instead, breaking the parsing logic.

🔧 Suggested normalization
-        $measure = Measure::query()->create($request->all());
+        $payload = $request->all();
+        if ($request->has('attributes')) {
+            $attributes = $request->input('attributes');
+            $payload['attributes'] = is_array($attributes)
+                ? implode(' ', $attributes)
+                : $attributes;
+        }
+        $measure = Measure::query()->create($payload);

Apply the same normalization to the update() method at line 45.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$measure = Measure::query()->create($request->all());
$payload = $request->all();
if ($request->has('attributes')) {
$attributes = $request->input('attributes');
$payload['attributes'] = is_array($attributes)
? implode(' ', $attributes)
: $attributes;
}
$measure = Measure::query()->create($payload);
🤖 Prompt for AI Agents
In `@app/Http/Controllers/API/MeasureController.php` at line 26, In
MeasureController's store() and update() flows, don't pass $request->all()
directly to Measure::query()->create(...) or ->update(...); instead normalize
the "attributes" field first: if $request->input('attributes') is an array,
convert it to a space-separated string via implode(' ', $attributes) and put it
back into the payload before calling Measure::query()->create(...) or
$measure->update(...). Ensure both the store() and update() paths perform this
normalization so the DB always receives a string for attributes.

if ($request->has('controls')) {
$measure->controls()->sync($request->input('controls', []));
}
Expand All @@ -33,14 +33,14 @@ public function store(Request $request)

public function show(Measure $measure)
{
abort_if(Auth::User()->role !== 4, Response::HTTP_FORBIDDEN, '403 Forbidden');
abort_if(Auth::User()->isAPI(), Response::HTTP_FORBIDDEN, '403 Forbidden');

return response()->json($measure);
}

public function update(Request $request, Measure $measure)
{
abort_if(Auth::User()->role !== 4, Response::HTTP_FORBIDDEN, '403 Forbidden');
abort_if(Auth::User()->isAPI(), Response::HTTP_FORBIDDEN, '403 Forbidden');

$measure->update($request->all());
if ($request->has('controls')) {
Expand All @@ -52,7 +52,7 @@ public function update(Request $request, Measure $measure)

public function destroy(Measure $measure)
{
abort_if(Auth::User()->role !== 4, Response::HTTP_FORBIDDEN, '403 Forbidden');
abort_if(Auth::User()->isAPI(), Response::HTTP_FORBIDDEN, '403 Forbidden');

$measure->delete();

Expand Down
43 changes: 19 additions & 24 deletions app/Http/Controllers/MeasureController.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use Illuminate\Support\Facades\DB;
use Illuminate\View\View;
use Maatwebsite\Excel\Facades\Excel;
use Mercator\Core\Models\Task;

class MeasureController extends Controller
{
Expand Down Expand Up @@ -195,18 +196,21 @@ public function store(Request $request)
]
);

$measure = new Measure();
$measure->domain_id = request('domain_id');
$measure->clause = request('clause');
$measure->name = request('name');
$measure->attributes = request('attributes') !== null ? implode(' ', request('attributes')) : null;
$measure->objective = request('objective');
$measure->input = request('input');
$measure->model = request('model');
$measure->indicator = request('indicator');
$measure->action_plan = request('action_plan');

$measure->save();
$attributes = $request->get('attributes');
$request['attributes'] = $attributes !== null ? implode(' ', $attributes) : null;

$measure = Measure::query()->create($request->all());
// $measure = new Measure();
// $measure->domain_id = request('domain_id');
// $measure->clause = request('clause');
// $measure->name = request('name');
// $measure->attributes = request('attributes') !== null ? implode(' ', request('attributes')) : null;
// $measure->objective = request('objective');
// $measure->input = request('input');
// $measure->model = request('model');
// $measure->indicator = request('indicator');
// $measure->action_plan = request('action_plan');
// $measure->save();

$request->session()->put('domain', $measure->domain_id);

Expand Down Expand Up @@ -378,23 +382,14 @@ public function update(Request $request)
);

// find measure
$measure = Measure::find($request->id);
$measure = Measure::query()->find($request->id);

// not found
abort_if($measure === null, Response::HTTP_NOT_FOUND, '404 Not Found');

// update measure
$measure->domain_id = request('domain_id');
$measure->name = request('name');
$measure->clause = request('clause');
$measure->attributes = request('attributes') !== null ? implode(' ', request('attributes')) : null;
$measure->objective = request('objective');
$measure->input = request('input');
$measure->model = request('model');
$measure->indicator = request('indicator');
$measure->action_plan = request('action_plan');

$measure->update();
$request['attributes'] = implode(' ', $request->get('attributes') !== null ? $request->get('attributes') : []);
$measure->update($request->all());

// return to view measure
return redirect('/alice/show/'.$measure->id);
Expand Down
7 changes: 5 additions & 2 deletions app/Models/Measure.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,15 @@ class Measure extends Model
];

protected $fillable = [
'name',
'domain_id',
'clause',
'name',
'objective',
'input',
'attributes',
'input',
'model',
'indicator',
'action_plan',
];

// Return the domain associated to this measure
Expand Down