Problem/Motivation

\Drupal\auditfiles\Batch\AuditFilesMergeFileReferencesBatchProcess::auditfilesMergeFileReferencesBatchMergeProcessBatch type hints $files_being_merged as an array.
Similarly \Drupal\auditfiles\Batch\AuditFilesMergeFileReferencesBatchProcess::$fileIds is type-hinted as an array.

However
\Drupal\auditfiles\ServiceAuditFilesMergeFileReferences::auditfilesMergeFileReferencesBatchMergeCreateBatch sets up the batch task using each separate file ID - these are integers and hence this causes a fatal error because an int isn't an array.

Similarly in \Drupal\auditfiles\Batch\AuditFilesMergeFileReferencesBatchProcess::dispatch it calls to \Drupal\auditfiles\ServiceAuditFilesMergeFileReferences::auditfilesMergeFileReferencesBatchMergeProcessFile using \Drupal\auditfiles\Batch\AuditFilesMergeFileReferencesBatchProcess::$fileIds

Which is the reverse, passing an array instead of an integer

Steps to reproduce

Try to use the merge file reference function

Proposed resolution

Remove the array typehints from \Drupal\auditfiles\Batch\AuditFilesMergeFileReferencesBatchProcess::$fileIds and \Drupal\auditfiles\Batch\AuditFilesMergeFileReferencesBatchProcess::auditfilesMergeFileReferencesBatchMergeProcessBatch

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#2 3210844.patch3.19 KBlarowlan

Issue fork auditfiles-3210844

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

larowlan created an issue. See original summary.

larowlan’s picture

Status: Active » Needs review
StatusFileSize
new3.19 KB
acbramley’s picture

Feels like we're missing test coverage here?

kim.pepper’s picture

Issue tags: +#pnx-sprint
anybody’s picture

Status: Needs review » Reviewed & tested by the community

Just ran into the same issue and reviewed and tested #2 for that reason. #2 fixed the issue for me.
So as it's a clear code fix, I'll set this RTBC, but if additional test coverage is needed, feel free to reset to NW.

dpi made their first commit to this issue’s fork.

dpi’s picture

Version: 8.x-3.x-dev » 4.0.x-dev
Assigned: Unassigned » dpi

dpi’s picture

Manually tested this, seems to work well.

Added a few extra improvements in the update to D9 in 4.x

  • dpi committed 2fc65ee on 4.0.x
    Issue #3210844 by dpi, larowlan: `\Drupal\auditfiles\Batch\...
dpi’s picture

Assigned: dpi » Unassigned
Status: Reviewed & tested by the community » Fixed

  • dpi committed 2fc65ee on 4.1.x
    Issue #3210844 by dpi, larowlan: `\Drupal\auditfiles\Batch\...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.