Completed
Pull Request — master (#306)
by Will
01:42
created

AssetAdminFile::humanizedChanges()   C

Complexity

Conditions 10
Paths 27

Size

Total Lines 47
Code Lines 29

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
c 0
b 0
f 0
dl 0
loc 47
rs 5.1578
cc 10
eloc 29
nc 27
nop 2

How to fix   Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
namespace SilverStripe\AssetAdmin\Controller;
4
5
use SilverStripe\Control\Director;
6
use SilverStripe\ORM\DataExtension;
7
use SilverStripe\ORM\Versioning\Versioned;
8
use SilverStripe\ORM\Versioning\DataDifferencer;
9
10
/**
11
 * Update File dataobjects to be editable in this asset admin
12
 */
13
class AssetAdminFile extends DataExtension
14
{
15
    public function updateCMSEditLink(&$link)
16
    {
17
        // Update edit link for this file to point to the new asset admin
18
        $controller = AssetAdmin::singleton();
19
        $link = Director::absoluteURL($controller->getFileEditLink($this->owner));
0 ignored issues
show
Compatibility introduced by
$this->owner of type object<SilverStripe\ORM\DataObject> is not a sub-type of object<SilverStripe\Assets\File>. It seems like you assume a child class of the class SilverStripe\ORM\DataObject to be always present.

This check looks for parameters that are defined as one type in their type hint or doc comment but seem to be used as a narrower type, i.e an implementation of an interface or a subclass.

Consider changing the type of the parameter or doing an instanceof check before assuming your parameter is of the expected type.

Loading history...
20
    }
21
22
    /**
23
     * @param int $from
24
     * @param int $to
25
     *
26
     * @return string
27
     */
28
    public function humanizedChanges($from, $to) {
29
        if(!$from) {
30
            return _t('SilverStripe\\AssetAdmin\\Controller\\AssetAdmin.UPLOADEDFILE', "Uploaded file");
31
        }
32
33
        $fromRecord = Versioned::get_version($this->owner->class, $this->owner->ID, $from);
34
        $toRecord = Versioned::get_version($this->owner->class, $this->owner->ID, $to);
35
36
        $diff = new DataDifferencer($fromRecord, $toRecord);
37
        $changes = $diff->changedFieldNames();
38
39
        $k = array_search('LastEdited', $changes);
40
41
        if($k !== false) {
42
            unset($changes[$k]);
43
        }
44
45
        $output = array();
46
47
        foreach($changes as $change) {
48
            $human = $change;
0 ignored issues
show
Unused Code introduced by
$human is not used, you could remove the assignment.

This check looks for variable assignements that are either overwritten by other assignments or where the variable is not used subsequently.

$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}

Both the $myVar assignment in line 1 and the $higher assignment in line 2 are dead. The first because $myVar is never used and the second because $higher is always overwritten for every possible time line.

Loading history...
49
50
            if($change == "ParentID") {
51
                // updated folder ID
52
                $human = _t('SilverStripe\\AssetAdmin\\Controller\\AssetAdminFile.MOVEDFOLDER', "Moved file");
53
            } else if($change == 'Title') {
54
                $human = _t('SilverStripe\\AssetAdmin\\Controller\\AssetAdminFile.RENAMEDTITLE', "Updated title to ") . $fromRecord->Title;
55
            } else if($change == 'Name') {
56
                $human = _t('SilverStripe\\AssetAdmin\\Controller\\AssetAdminFile.RENAMEDFILE', "Renamed file to ") . $fromRecord->Filename;
57
            } else if($change == 'File') {
58
                // check to make sure the files are actually different
59
                if($fromRecord->getHash() != $toRecord->getHash()) {
60
                    $human = _t('SilverStripe\\AssetAdmin\\Controller\\AssetAdminFile.RENAMEDFILE', "Replaced file");
61
                } else {
62
                    $human = false;
63
                }
64
            } else {
65
                $human = false;
66
            }
67
68
            if($human) {
0 ignored issues
show
Bug Best Practice introduced by
The expression $human of type string|false is loosely compared to true; this is ambiguous if the string can be empty. You might want to explicitly use !== false instead.

In PHP, under loose comparison (like ==, or !=, or switch conditions), values of different types might be equal.

For string values, the empty string '' is a special case, in particular the following results might be unexpected:

''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
Loading history...
69
                $output[] = $human;
70
            }
71
        }
72
73
        return implode(", ", $output);
74
    }
75
}
76