Problem/Motivation

file_unmanaged_move() logs an error if file copy failed. When doing so it sends wrong arguments to LoggerInterface::error().

Proposed resolution

Remove unneeded argument.

Remaining tasks

- prepare patch
- review patch

CommentFileSizeAuthor
#10 2393455-wrong-argument-sent-to-logger.patch795 bytesRavindraSingh
#1 2393455_1.patch790 bytesslashrsm

Comments

slashrsm’s picture

slashrsm commented
Status: Active » Needs review
StatusFileSize
new 2393455_1.patch790 bytes

Status: Needs review » Needs work

The last submitted patch, 1: 2393455_1.patch, failed testing.

Status: Needs work » Needs review

slashrsm queued 1: 2393455_1.patch for re-testing.

primsi’s picture

primsi commented
Status: Needs review » Reviewed & tested by the community

Looks ok.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: 2393455_1.patch, failed testing.

Status: Needs work » Needs review

slashrsm queued 1: 2393455_1.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2393455_1.patch, failed testing.

Status: Needs work » Needs review

slashrsm queued 1: 2393455_1.patch for re-testing.

slashrsm’s picture

slashrsm commented
Status: Needs review » Reviewed & tested by the community

Still works. It seems test fail was just a temporary hick-up.

RavindraSingh’s picture

RavindraSingh commented
StatusFileSize
new 2393455-wrong-argument-sent-to-logger.patch795 bytes

I think lets try to fix the testing fail also. Here I am uploading updated patch.

slashrsm’s picture

slashrsm commented

I don't see any changes compared to #1 (except wrong indentation). Can you explain?

RavindraSingh’s picture

RavindraSingh commented

Its almost same except $logger->error(t('The specified file %file could not be copied to %destination.'), array('%file' => $source, '%destination' => $destination));
I have used t() function.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 10: 2393455-wrong-argument-sent-to-logger.patch, failed testing.

Status: Needs work » Needs review

RavindraSingh queued 10: 2393455-wrong-argument-sent-to-logger.patch for re-testing.

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/includes/file.inc
@@ -756,7 +756,7 @@ function file_unmanaged_copy($source, $destination = NULL, $replace = FILE_EXIST
-      $logger->error('file', 'The specified file %file could not be copied to %destination.', array('%file' => $source, '%destination' => $destination));
+        $logger->error(t('The specified file %file could not be copied to %destination.'), array('%file' => $source, '%destination' => $destination));

Wrong intentation. Not: We also don't use t() for log messages, so just remove 'file' and be done with it :)

slashrsm’s picture

slashrsm commented
Status: Needs review » Reviewed & tested by the community

It is exactly what #1 did, which was already RTBC.

dawehner’s picture

dawehner
Primary language German
commented

@slashrsm
Sorry, I haven't seen that. +1 to your old path

slashrsm’s picture

slashrsm commented

NP :) Thank you.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed #1 to 8.0.x, thanks!

  • catch committed bb56d4c on 8.0.x 
    Issue #2393455 by slashrsm, RavindraSingh: Wrong arguments sent to...

Status: Fixed » Closed (fixed)

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