Skip to content
This repository was archived by the owner on May 29, 2026. It is now read-only.

fix(FilesController): normalize 'file' field uploads in filesMultipart#1436

Merged
WilcoLouwerse merged 2 commits into
developmentfrom
fix/woo-tutorial-quality
May 7, 2026
Merged

fix(FilesController): normalize 'file' field uploads in filesMultipart#1436
WilcoLouwerse merged 2 commits into
developmentfrom
fix/woo-tutorial-quality

Conversation

@rubenvdlinde

@rubenvdlinde rubenvdlinde commented May 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • POST to `/api/objects/{register}/{schema}/{id}/filesMultipart` with a single file in the `file` field returned 500 because `extractUploadedFiles()` appended the raw `$_FILES` entry without running it through `normalizeSingleFile`. `processUploadedFiles` then dereferenced `$file['share']`, which was missing, throwing the error: `TypeError: FileService::addFile(): Argument finished register dialog #4 ($share) must be of type bool, null given`.
  • Route the `file` branch through `normalizeSingleFile`, the same normalizer that the `files[]` branch already uses. Both upload patterns now succeed.

Background

Surfaced while writing the new Conduction Academy tutorial on uploading files to a Woo publication. The tutorial walks through four upload modes; this was the only one that returned a 500.

Test plan

  • `curl -F "file=@./test.txt" -F "share=true" .../filesMultipart` returns 200 with a normalized file object
  • `curl -F "files[]=@./test.txt" -F "share=true" .../filesMultipart` continues to return 200
  • `curl -F "files[]=@./a.txt" -F "files[]=@./b.txt" .../filesMultipart` continues to return a two-element array
  • CI passes

POST /api/objects/{register}/{schema}/{id}/filesMultipart with a single
file in the 'file' field returned 500 because extractUploadedFiles()
appended the raw $_FILES entry to the upload list without running it
through normalizeSingleFile. processUploadedFiles then dereferenced
$file['share'] which was missing, causing:

  TypeError: FileService::addFile(): Argument #4 ($share) must be of
  type bool, null given

Run the 'file' branch through normalizeSingleFile, the same path the
'files[]' branch already uses. Both upload patterns now succeed.

Verified end-to-end against a Woo register import on the dev environment
while writing the new academy tutorial on bestand uploads.
Comment thread lib/Controller/FilesController.php
Comment thread lib/Controller/FilesController.php

@WilcoLouwerse WilcoLouwerse left a comment

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.

Strict-mode review: no 🔴 blockers, but two 🟡 concerns trigger REQUEST_CHANGES — the share field still only matches the literal string 'true', so the commit message's claim that 'share' and 'tags' are populated is only half-true; and no PHPUnit regression test guards the bug-fix path. The Newman CI failure is pre-existing (verified on development HEAD) and not caused by this PR. The core extractUploadedFiles change is correct.

Address PR #1436 review feedback:

- Replace `$data['share'] === 'true'` strict comparison with
  `$this->parseBool(value: $data['share'] ?? false)` in both
  `normalizeSingleFile` and `normalizeMultipleFiles`. This matches
  what `create()` already does and accepts `1`, `yes`, `on`, and a
  real boolean `true` instead of silently returning false for any
  non-`'true'` literal.
- Strengthen `testExtractUploadedFilesSingleFile` to assert the
  normalized shape (`share` is bool, `tags` is array) so a future
  refactor of `normalizeSingleFile` cannot silently re-introduce
  the original `TypeError: FileService::addFile(): Argument #4
  ($share) must be of type bool, null given`.
- Add `testExtractUploadedFilesShareParsing` data provider covering
  the truthy/falsy cases parseBool now accepts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@WilcoLouwerse WilcoLouwerse left a comment

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.

Both prior 🟡 concerns resolved in 2166e03parseBool used in both normalizers and regression test asserts the normalized shape plus a parseBool data provider. CI green (Newman), no new findings.

@WilcoLouwerse WilcoLouwerse merged commit 07bb11b into development May 7, 2026
1 check passed
@WilcoLouwerse WilcoLouwerse deleted the fix/woo-tutorial-quality branch May 7, 2026 11:08
@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 8bac2e0

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 147/147
npm ✅ 598/598
PHPUnit
Newman
Playwright ⏭️

Quality workflow — 2026-05-07 11:51 UTC

Download the full PDF report from the workflow artifacts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants