Merge pull request #102 from robbieaverill/fix/psr-2

FIX Updates for coding standards, move second classes in files to their own files, fix comments

_config.php

@@ -8,12 +8,15 @@ DMSDocumentAddController::add_allowed_extensions(); //add an array of additional
 
 define('DMS_DIR', 'dms');
 
-if (!file_exists(BASE_PATH . DIRECTORY_SEPARATOR . DMS_DIR)) user_error("DMS directory named incorrectly. Please install the DMS module into a folder named: ".DMS_DIR);
+if (!file_exists(BASE_PATH . DIRECTORY_SEPARATOR . DMS_DIR)) {
+    user_error("DMS directory named incorrectly. Please install the DMS module into a folder named: ".DMS_DIR);
+}
 
 CMSMenu::remove_menu_item('DMSDocumentAddController');
 
 ShortcodeParser::get('default')->register(
-	'dms_document_link', array('DMSShortcodeHandler', 'handle')
+	'dms_document_link',
+    array('DMSShortcodeHandler', 'handle')
 );
 
 if ($config->get('DMSDocument_versions', 'enable_versions')) {

code/DMS.php

@@ -1,11 +1,20 @@
 <?php
 class DMS implements DMSInterface
 {
+    /**
+     * Folder to store the documents in
+     *
+     * @var string
+     */
+    public static $dmsFolder = 'dms-assets';
 
-    public static $dmsFolder = 'dms-assets';   //folder to store the documents in
+    /**
-
+     * How many documents to store in a single folder. The square of this number is the maximum number of documents.
-    //How many documents to store in a single folder. The square of this number is the maximum number of documents.
+     *
-    //The number should be a multiple of 10
+     * The number should be a multiple of 10
+     *
+     * @var int
+     */
     public static $dmsFolderSize = 1000;
 
 
@@ -24,13 +33,25 @@ class DMS implements DMSInterface
         }
 
         if (!file_exists($dmsPath . DIRECTORY_SEPARATOR . '.htaccess')) {
-            //restrict access to the storage folder
+            // Restrict access to the storage folder
-            copy(BASE_PATH . DIRECTORY_SEPARATOR . DMS_DIR . DIRECTORY_SEPARATOR . 'resources' . DIRECTORY_SEPARATOR . '.htaccess',  $dmsPath . DIRECTORY_SEPARATOR . '.htaccess');
+            copy(
-            copy(BASE_PATH . DIRECTORY_SEPARATOR . DMS_DIR . DIRECTORY_SEPARATOR . 'resources' . DIRECTORY_SEPARATOR . 'web.config', $dmsPath . DIRECTORY_SEPARATOR . 'web.config');
+                BASE_PATH . DIRECTORY_SEPARATOR . DMS_DIR . DIRECTORY_SEPARATOR
+                . 'resources' . DIRECTORY_SEPARATOR . '.htaccess',
+                $dmsPath . DIRECTORY_SEPARATOR . '.htaccess'
+            );
+
+            copy(
+                BASE_PATH . DIRECTORY_SEPARATOR . DMS_DIR . DIRECTORY_SEPARATOR
+                . 'resources' . DIRECTORY_SEPARATOR . 'web.config',
+                $dmsPath . DIRECTORY_SEPARATOR . 'web.config'
+            );
         }
         return $dms;
     }
 
+    /**
+     * @return string
+     */
     public static function get_dms_path()
     {
         return BASE_PATH . DIRECTORY_SEPARATOR . self::$dmsFolder;
@@ -56,8 +77,9 @@ class DMS implements DMSInterface
     /**
      * Takes a File object or a String (path to a file) and copies it into the DMS. The original file remains unchanged.
      * When storing a document, sets the fields on the File has "tag" metadata.
-     * @param $file File object, or String that is path to a file to store, e.g. "assets/documents/industry/supplied-v1-0.pdf"
+     * @param $file File object, or String that is path to a file to store,
-
+     *              e.g. "assets/documents/industry/supplied-v1-0.pdf"
+     * @return DMSDocument
      */
     public function storeDocument($file)
     {
@@ -74,7 +96,14 @@ class DMS implements DMSInterface
     /**
      *
      * Returns a number of Document objects based on the a search by tags. You can search by category alone,
-     * by tag value alone, or by both. I.e: getByTag("fruits",null); getByTag(null,"banana"); getByTag("fruits","banana")
+     * by tag value alone, or by both. I.e:
+     *
+     * <code>
+     * getByTag("fruits", null);
+     * getByTag(null, "banana");
+     * getByTag("fruits", "banana");
+     * </code>
+     *
      * @param null $category The metadata category to search for
      * @param null $value The metadata value to search for
      * @param bool $showEmbargoed Boolean that specifies if embargoed documents should be included in results

code/DMSShortcodeHandler.php

@@ -10,7 +10,11 @@ class DMSShortcodeHandler
 {
 
     public static function handle(
-        $arguments, $content, ShortcodeParser $parser, $tag, array $extra = array()
+        $arguments,
+        $content,
+        ShortcodeParser $parser,
+        $tag,
+        array $extra = array()
     ) {
         if (!empty($arguments['id'])) {
             $document = DMSDocument::get()->byID($arguments['id']);
@@ -18,7 +22,9 @@ class DMSShortcodeHandler
             if ($document && !$document->isHidden()) {
                 if ($content) {
                     return sprintf(
-                        '<a href="%s">%s</a>', $document->Link(), $parser->parse($content)
+                        '<a href="%s">%s</a>',
+                        $document->Link(),
+                        $parser->parse($content)
                     );
                 } else {
                     if (isset($extra['element'])) {

code/cms/DMSDocumentAddController.php

@@ -3,7 +3,6 @@
 /**
  * @package dms
  */
-
 class DMSDocumentAddController extends LeftAndMain
 {
 
@@ -51,7 +50,8 @@ class DMSDocumentAddController extends LeftAndMain
 
         if ($id && is_numeric($id) && $id > 0) {
             return Versioned::get_by_stage('SiteTree', 'Stage', sprintf(
-                'ID = %s', (int) $id
+                'ID = %s',
+                (int) $id
             ))->first();
         } else {
             // ID is either '0' or 'root'
@@ -89,7 +89,9 @@ class DMSDocumentAddController extends LeftAndMain
         $uploadField->setTemplate('AssetUploadField');
         $uploadField->setRecord($page);
 
-        $uploadField->getValidator()->setAllowedExtensions(array_filter(array_merge(Config::inst()->get('File', 'allowed_extensions'), self::$allowed_extensions)));
+        $uploadField->getValidator()->setAllowedExtensions(
+            array_filter(array_merge(Config::inst()->get('File', 'allowed_extensions'), self::$allowed_extensions))
+        );
         $exts = $uploadField->getValidator()->getAllowedExtensions();
 
         asort($exts);
@@ -105,8 +107,10 @@ class DMSDocumentAddController extends LeftAndMain
             $this,
             'getEditForm',
             new FieldList(
-                new TabSet('Main',
+                new TabSet(
-                    new Tab('From your computer',
+                    'Main',
+                    new Tab(
+                        'From your computer',
                         new HiddenField('ID', false, $page->ID),
                         $uploadField,
                         new LiteralField(
@@ -118,7 +122,8 @@ class DMSDocumentAddController extends LeftAndMain
                             )
                         )
                     ),
-                    new Tab('From the CMS',
+                    new Tab(
+                        'From the CMS',
                         $addExistingField
                     )
                 )
@@ -183,7 +188,9 @@ class DMSDocumentAddController extends LeftAndMain
         $term = (isset($_GET['term'])) ? $_GET['term'] : '';
         $term_sql = Convert::raw2sql($term);
         $data = DMSDocument::get()
-        ->where("(\"ID\" LIKE '%".$term_sql."%' OR \"Filename\" LIKE '%".$term_sql."%' OR \"Title\" LIKE '%".$term_sql."%')")
+        ->where(
+            "(\"ID\" LIKE '%".$term_sql."%' OR \"Filename\" LIKE '%".$term_sql."%' OR \"Title\" LIKE '%".$term_sql."%')"
+        )
         ->sort('ID ASC')
         ->limit(20);
 
@@ -208,15 +215,17 @@ class DMSDocumentAddController extends LeftAndMain
             $document = DataObject::get_by_id('DMSDocument', (int) $_GET['documentID']);
             $document->addPage($page);
 
-            $buttonText = '<button class="ss-uploadfield-item-edit ss-ui-button ui-corner-all" title="Edit this document" data-icon="pencil">'.
+            $buttonText = '<button class="ss-uploadfield-item-edit ss-ui-button ui-corner-all"'
-                'Edit<span class="toggle-details"><span class="toggle-details-icon"></span></span></button>';
+                . ' title="Edit this document" data-icon="pencil">'
+                . 'Edit<span class="toggle-details"><span class="toggle-details-icon"></span></span></button>';
 
             // Collect all output data.
             $return = array(
                 'id' => $document->ID,
                 'name' => $document->getTitle(),
                 'thumbnail_url' => $document->Icon($document->getExtension()),
-                'edit_url' => $this->getEditForm()->Fields()->fieldByName('Main.From your computer.AssetUploadField')->getItemHandler($document->ID)->EditLink(),
+                'edit_url' => $this->getEditForm()->Fields()->fieldByName('Main.From your computer.AssetUploadField')
+                    ->getItemHandler($document->ID)->EditLink(),
                 'size' => $document->getFileSizeFormatted(),
                 'buttons' => $buttonText,
                 'showeditform' => true
@@ -250,7 +259,8 @@ class DMSDocumentAddController extends LeftAndMain
             return $list;
         }
 
-        return sprintf('<p>%s</p>',
+        return sprintf(
+            '<p>%s</p>',
             _t('DMSDocumentAddController.NODOCUMENTS', 'There are no documents attached to the selected page.')
         );
     }

code/cms/DMSDocumentAddExistingField.php

@@ -2,7 +2,6 @@
 
 class DMSDocumentAddExistingField extends CompositeField
 {
-
     public $useFieldContext = true;
 
     public function __construct($name, $title = null)
@@ -10,7 +9,15 @@ class DMSDocumentAddExistingField extends CompositeField
         $this->name = $name;
         $this->title = ($title === null) ? $name : $title;
 
-        parent::__construct(new TreeDropdownField('PageSelector', 'Add from another page', 'SiteTree', 'ID', 'TitleWithNumberOfDocuments'));
+        parent::__construct(
+            new TreeDropdownField(
+                'PageSelector',
+                'Add from another page',
+                'SiteTree',
+                'ID',
+                'TitleWithNumberOfDocuments'
+            )
+        );
     }
 
     /**
@@ -23,7 +30,8 @@ class DMSDocumentAddExistingField extends CompositeField
         return $this;
     }
     /**
-     * Get the record to use as "Parent" for uploaded Files (eg a Page with a has_one to File) If none is set, it will use Form->getRecord() or Form->Controller()->data()
+     * Get the record to use as "Parent" for uploaded Files (eg a Page with a has_one to File) If none is set, it
+     * will use Form->getRecord() or Form->Controller()->data()
      * @return DataObject
      */
     public function getRecord()

code/cms/DMSGridFieldDeleteAction.php

@@ -13,7 +13,9 @@
  * @package dms
  * @subpackage cms
  */
-class DMSGridFieldDeleteAction extends GridFieldDeleteAction implements GridField_ColumnProvider, GridField_ActionProvider
+class DMSGridFieldDeleteAction extends GridFieldDeleteAction implements
+    GridField_ColumnProvider,
+    GridField_ActionProvider
 {
 
     /**
@@ -26,35 +28,52 @@ class DMSGridFieldDeleteAction extends GridFieldDeleteAction implements GridFiel
     public function getColumnContent($gridField, $record, $columnName)
     {
         if ($this->removeRelation) {
-            $field = GridField_FormAction::create($gridField, 'UnlinkRelation'.$record->ID, false, "unlinkrelation", array('RecordID' => $record->ID))
+            $field = GridField_FormAction::create(
+                $gridField,
+                'UnlinkRelation' . $record->ID,
+                false,
+                "unlinkrelation",
+                array('RecordID' => $record->ID)
+            )
                 ->addExtraClass('gridfield-button-unlink')
                 ->setAttribute('title', _t('GridAction.UnlinkRelation', "Unlink"))
                 ->setAttribute('data-icon', 'chain--minus');
         } else {
             if (!$record->canDelete()) {
-                return;
+                return '';
             }
-            $field = GridField_FormAction::create($gridField,  'DeleteRecord'.$record->ID, false, "deleterecord", array('RecordID' => $record->ID))
+
+            $field = GridField_FormAction::create(
+                $gridField,
+                'DeleteRecord' . $record->ID,
+                false,
+                "deleterecord",
+                array('RecordID' => $record->ID)
+            )
                 ->addExtraClass('gridfield-button-delete')
                 ->setAttribute('title', _t('GridAction.Delete', "Delete"))
                 ->setAttribute('data-icon', 'cross-circle')
                 ->setDescription(_t('GridAction.DELETE_DESCRIPTION', 'Delete'));
         }
 
-        //add a class to the field to if it is the last gridfield in the list
+        // Add a class to the field to if it is the last gridfield in the list
         $numberOfRelations = $record->Pages()->Count();
-        $field->addExtraClass('dms-delete') //add a new class for custom JS to handle the delete action
+        $field
-                ->setAttribute('data-pages-count', $numberOfRelations)  //add the number of pages attached to this field as a data-attribute
+            // Add a new class for custom JS to handle the delete action
-                ->removeExtraClass('gridfield-button-delete');  //remove the base gridfield behaviour
+            ->addExtraClass('dms-delete')
+            // Add the number of pages attached to this field as a data-attribute
+            ->setAttribute('data-pages-count', $numberOfRelations)
+            // Remove the base gridfield behaviour
+            ->removeExtraClass('gridfield-button-delete');
 
-        //set a class telling JS what kind of warning to display when clicking the delete button
+        // Set a class telling JS what kind of warning to display when clicking the delete button
         if ($numberOfRelations > 1) {
             $field->addExtraClass('dms-delete-link-only');
         } else {
             $field->addExtraClass('dms-delete-last-warning');
         }
 
-        //set a class to show if the document is hidden
+        // Set a class to show if the document is hidden
         if ($record->isHidden()) {
             $field->addExtraClass('dms-document-hidden');
         }
@@ -70,6 +89,7 @@ class DMSGridFieldDeleteAction extends GridFieldDeleteAction implements GridFiel
      * @param mixed $arguments
      * @param array $data - form data
      * @return void
+     * @throws ValidationException If the current user doesn't have permission to delete the record
      */
     public function handleAction(GridField $gridField, $actionName, $arguments, $data)
     {
@@ -79,7 +99,10 @@ class DMSGridFieldDeleteAction extends GridFieldDeleteAction implements GridFiel
                 return;
             }
             if ($actionName == 'deleterecord' && !$item->canDelete()) {
-                throw new ValidationException(_t('GridFieldAction_Delete.DeletePermissionsFailure', "No delete permissions"), 0);
+                throw new ValidationException(
+                    _t('GridFieldAction_Delete.DeletePermissionsFailure', "No delete permissions"),
+                    0
+                );
             }
 
             $delete = false;
@@ -87,10 +110,12 @@ class DMSGridFieldDeleteAction extends GridFieldDeleteAction implements GridFiel
                 $delete = true;
             }
 
-            $gridField->getList()->remove($item);   //remove the relation
+            // Remove the relation
+            $gridField->getList()->remove($item);
             if ($delete) {
+                // Delete the document
                 $item->delete();
-            }   //delete the DMSDocument
+            }
         }
     }
 }

code/cms/DMSUploadField.php

@@ -237,7 +237,9 @@ class DMSUploadField extends UploadField
         }
 
         // Fields instance
-        if ($this->fileEditFields instanceof FieldList) return $this->fileEditFields;
+        if ($this->fileEditFields instanceof FieldList) {
+            return $this->fileEditFields;
+        }
 
         // Method to call on the given file
         if ($file->hasMethod($this->fileEditFields)) {
@@ -265,7 +267,9 @@ class DMSUploadField extends UploadField
         }
 
         // Actions instance
-        if ($this->fileEditActions instanceof FieldList) return $this->fileEditActions;
+        if ($this->fileEditActions instanceof FieldList) {
+            return $this->fileEditActions;
+        }
 
         // Method to call on the given file
         if ($file->hasMethod($this->fileEditActions)) {
@@ -285,10 +289,14 @@ class DMSUploadField extends UploadField
     public function getDMSFileEditValidator($file)
     {
         // Empty validator
-        if(empty($this->fileEditValidator)) return null;
+        if (empty($this->fileEditValidator)) {
+            return null;
+        }
 
         // Validator instance
-        if($this->fileEditValidator instanceof Validator) return $this->fileEditValidator;
+        if ($this->fileEditValidator instanceof Validator) {
+            return $this->fileEditValidator;
+        }
 
         // Method to call on the given file
         if ($file->hasMethod($this->fileEditValidator)) {
@@ -298,42 +306,3 @@ class DMSUploadField extends UploadField
         user_error("Invalid value for UploadField::fileEditValidator", E_USER_ERROR);
     }
 }
-
-class DMSUploadField_ItemHandler extends UploadField_ItemHandler
-{
-	
-    private static $allowed_actions = array(
-        'delete',
-        'edit',
-        'EditForm',
-    );
-	
-    public function getItem()
-    {
-        return DataObject::get_by_id('DMSDocument', $this->itemID);
-    }
-	
-    /**
-     * @return Form
-     */
-    public function EditForm() {
-    	$file = $this->getItem();
-
-    	// Get form components
-    	$fields = $this->parent->getDMSFileEditFields($file);
-    	$actions = $this->parent->getDMSFileEditActions($file);
-    	$validator = $this->parent->getDMSFileEditValidator($file);
-    	$form = new Form(
-    		$this,
-    		__FUNCTION__,
-    		$fields,
-    		$actions,
-    		$validator
-    	);
-    	$form->loadDataFrom($file);
-    	$form->addExtraClass('small');
-
-    	return $form;
-    }
-    
-}

code/cms/DMSUploadField_ItemHandler.php

@@ -0,0 +1,39 @@
+<?php
+
+class DMSUploadField_ItemHandler extends UploadField_ItemHandler
+{
+    private static $allowed_actions = array(
+        'delete',
+        'edit',
+        'EditForm',
+    );
+
+    public function getItem()
+    {
+        return DataObject::get_by_id('DMSDocument', $this->itemID);
+    }
+
+    /**
+     * @return Form
+     */
+    public function EditForm()
+    {
+        $file = $this->getItem();
+
+        // Get form components
+        $fields = $this->parent->getDMSFileEditFields($file);
+        $actions = $this->parent->getDMSFileEditActions($file);
+        $validator = $this->parent->getDMSFileEditValidator($file);
+        $form = new Form(
+            $this,
+            __FUNCTION__,
+            $fields,
+            $actions,
+            $validator
+        );
+        $form->loadDataFrom($file);
+        $form->addExtraClass('small');
+
+        return $form;
+    }
+}

code/cms/DocumentHtmlEditorFieldToolbar.php

@@ -2,11 +2,8 @@
 /**
  * Extends the original toolbar with document picking capability - modified lines are commented.
  */
-
-
 class DocumentHtmlEditorFieldToolbar extends Extension
 {
-
     public function updateLinkForm(Form $form)
     {
         $linkType = null;

code/extensions/DMSSiteTreeExtension.php

@@ -33,7 +33,8 @@ class DMSSiteTreeExtension extends DataExtension
 
     /**
      * Only show the documents tab on the list of pages set here. Any pages set in the no_documents_tab array will
-     * still not be shown. If this isn't called, or if it is called with an empty array, all pages will get Document tabs.
+     * still not be shown. If this isn't called, or if it is called with an empty array, all pages will get
+     * Document tabs.
      * @static
      * @param $array Array of page types to show the Documents tab on
      */
@@ -51,7 +52,7 @@ class DMSSiteTreeExtension extends DataExtension
 
     public function updateCMSFields(FieldList $fields)
     {
-        //prevent certain pages from having a Document tab in the CMS
+        // Prevent certain pages from having a Document tab in the CMS
         if (in_array($this->owner->ClassName, self::$noDocumentsList)) {
             return;
         }
@@ -59,11 +60,12 @@ class DMSSiteTreeExtension extends DataExtension
             return;
         }
 
-        //javascript to customize the grid field for the DMS document (overriding entwine in FRAMEWORK_DIR.'/javascript/GridField.js'
+        // Javascript to customize the grid field for the DMS document (overriding entwine
+        // in FRAMEWORK_DIR.'/javascript/GridField.js'
         Requirements::javascript(DMS_DIR.'/javascript/DMSGridField.js');
         Requirements::css(DMS_DIR.'/css/DMSMainCMS.css');
 
-        //javascript for the link editor pop-up in TinyMCE
+        // Javascript for the link editor pop-up in TinyMCE
         Requirements::javascript(DMS_DIR."/javascript/DocumentHtmlEditorFieldToolbar.js");
 
         // Document listing
@@ -97,12 +99,18 @@ class DMSSiteTreeExtension extends DataExtension
 
         // HACK: Create a singleton of DMSDocument to ensure extensions are applied before we try to get display fields.
         singleton('DMSDocument');
-        $gridFieldConfig->getComponentByType('GridFieldDataColumns')->setDisplayFields(Config::inst()->get('DMSDocument', 'display_fields'))
+        $gridFieldConfig->getComponentByType('GridFieldDataColumns')
+            ->setDisplayFields(Config::inst()->get('DMSDocument', 'display_fields'))
             ->setFieldCasting(array('LastChanged'=>"Datetime->Ago"))
-            ->setFieldFormatting(array('FilenameWithoutID'=>'<a target=\'_blank\' class=\'file-url\' href=\'$Link\'>$FilenameWithoutID</a>'));
+            ->setFieldFormatting(
+                array(
+                    'FilenameWithoutID'=>'<a target=\'_blank\' class=\'file-url\' href=\'$Link\'>$FilenameWithoutID</a>'
+                )
+            );
 
         //override delete functionality with this class
-        $gridFieldConfig->getComponentByType('GridFieldDetailForm')->setItemRequestClass('DMSGridFieldDetailForm_ItemRequest');
+        $gridFieldConfig->getComponentByType('GridFieldDetailForm')
+            ->setItemRequestClass('DMSGridFieldDetailForm_ItemRequest');
 
         $gridField = GridField::create(
             'Documents',
@@ -115,7 +123,8 @@ class DMSSiteTreeExtension extends DataExtension
         $uploadBtn = new LiteralField(
             'UploadButton',
             sprintf(
-                '<a class="ss-ui-button ss-ui-action-constructive cms-panel-link" data-pjax-target="Content" data-icon="add" href="%s">%s</a>',
+                '<a class="ss-ui-button ss-ui-action-constructive cms-panel-link" data-pjax-target="Content"'
+                . ' data-icon="add" href="%s">%s</a>',
                 Controller::join_links(singleton('DMSDocumentAddController')->Link(), '?ID=' . $this->owner->ID),
                 "Add Documents"
             )

code/interface/DMSDocumentInterface.php

@@ -1,12 +1,11 @@
 <?php
 /**
- * Interface for a DMSDocument used in the Document Management System. A DMSDocument is create by storing a File object in an
+ * Interface for a DMSDocument used in the Document Management System. A DMSDocument is create by storing a File
- * instance of the DMSInterface. All write operations on the DMSDocument create a new relation, so we never need to
+ * object in an instance of the DMSInterface. All write operations on the DMSDocument create a new relation, so we
- * explicitly call the write() method on the DMSDocument DataObject
+ * never need to explicitly call the write() method on the DMSDocument DataObject
  */
 interface DMSDocumentInterface
 {
-
     /**
      * Deletes the DMSDocument, its underlying file, as well as any tags related to this DMSDocument.
      *
@@ -27,7 +26,8 @@ interface DMSDocumentInterface
     public function addPage($pageObject);
 
     /**
-     * Associates this DMSDocument with a set of Pages. This method loops through a set of page ids, and then associates this
+     * Associates this DMSDocument with a set of Pages. This method loops through a set of page ids, and then
+     * associates this
      * DMSDocument with the individual Page with the each page id in the set
      * @abstract
      * @param $pageIDs array of page ids used for the page objects associate this DMSDocument with
@@ -36,7 +36,8 @@ interface DMSDocumentInterface
     public function addPages($pageIDs);
 
     /**
-     * Removes the association between this DMSDocument and a Page. This method does nothing if the association does not exist.
+     * Removes the association between this DMSDocument and a Page. This method does nothing if the association does
+     * not exist.
      * @abstract
      * @param $pageObject Page object to remove the association to
      * @return mixed
@@ -59,8 +60,10 @@ interface DMSDocumentInterface
 
     /**
      * Adds a metadata tag to the DMSDocument. The tag has a category and a value.
-     * Each category can have multiple values by default. So: addTag("fruit","banana") addTag("fruit", "apple") will add two items.
+     * Each category can have multiple values by default. So: addTag("fruit","banana") addTag("fruit", "apple") will
-     * However, if the third parameter $multiValue is set to 'false', then all updates to a category only ever update a single value. So:
+     * add two items.
+     * However, if the third parameter $multiValue is set to 'false', then all updates to a category only ever update
+     * a single value. So:
      * addTag("fruit","banana") addTag("fruit", "apple") would result in a single metadata tag: fruit->apple.
      * Can could be implemented as a key/value store table (although it is more like category/value, because the
      * same category can occur multiple times)

code/interface/DMSInterface.php

@@ -10,7 +10,6 @@
  */
 interface DMSInterface
 {
-
     /**
      * Factory method that returns an instance of the DMS. This could be any class that implements the DMSInterface.
      * @static
@@ -31,7 +30,13 @@ interface DMSInterface
     /**
      *
      * Returns a number of Document objects based on the a search by tags. You can search by category alone,
-     * by tag value alone, or by both. I.e: getByTag("fruits",null); getByTag(null,"banana"); getByTag("fruits","banana")
+     * by tag value alone, or by both. I.e:
+     *
+     * <code>
+     * getByTag("fruits", null);
+     * getByTag(null, "banana");
+     * getByTag("fruits", "banana");
+     * </code>
      * @abstract
      * @param null $category The metadata category to search for
      * @param null $value The metadata value to search for

code/model/DMSDocument.php

@@ -5,14 +5,14 @@
  */
 class DMSDocument extends DataObject implements DMSDocumentInterface
 {
-
     private static $db = array(
         "Filename" => "Varchar(255)", // eg. 3469~2011-energysaving-report.pdf
         "Folder" => "Varchar(255)",    // eg.	0
         "Title" => 'Varchar(1024)', // eg. "Energy Saving Report for Year 2011, New Zealand LandCorp"
         "Description" => 'Text',
         "ViewCount" => 'Int',
-        "LastChanged" => 'SS_DateTime', //when this document's file was created or last replaced (small changes like updating title don't count)
+        // When this document's file was created or last replaced (small changes like updating title don't count)
+        "LastChanged" => 'SS_DateTime',
 
         "EmbargoedIndefinitely" => 'Boolean(false)',
         "EmbargoedUntilPublished" => 'Boolean(false)',
@@ -169,7 +169,10 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
     {
         $this->Pages()->add($pageObject);
 
-        DB::query("UPDATE \"DMSDocument_Pages\" SET \"DocumentSort\"=\"DocumentSort\"+1 WHERE \"SiteTreeID\" = $pageObject->ID");
+        DB::query(
+            "UPDATE \"DMSDocument_Pages\" SET \"DocumentSort\"=\"DocumentSort\"+1"
+            . " WHERE \"SiteTreeID\" = $pageObject->ID"
+        );
 
         return $this;
     }
@@ -926,8 +929,7 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
         $defaultDownloadBehaviour = Config::inst()->get('DMSDocument', 'default_download_behaviour');
         if (!isset($downloadBehaviorSource[$defaultDownloadBehaviour])) {
             user_error('Default download behaviour "' . $defaultDownloadBehaviour . '" not supported.', E_USER_WARNING);
-        }
+        } else {
-        else {
             $downloadBehaviorSource[$defaultDownloadBehaviour] .= ' (' . _t('DMSDocument.DEFAULT', 'default') . ')';
         }
 
@@ -938,7 +940,10 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
                 $downloadBehaviorSource,
                 $defaultDownloadBehaviour
             )
-            ->setDescription('How the visitor will view this file. <strong>Open in browser</strong> allows files to be opened in a new tab.')
+            ->setDescription(
+                'How the visitor will view this file. <strong>Open in browser</strong> '
+                . 'allows files to be opened in a new tab.'
+            )
         );
 
         //create upload field to replace document
@@ -990,9 +995,15 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
                 new GridFieldDataColumns(),
                 new GridFieldPaginator(30)
             );
-            $versionsGridFieldConfig->getComponentByType('GridFieldDataColumns')->setDisplayFields(Config::inst()->get('DMSDocument_versions', 'display_fields'))
+            $versionsGridFieldConfig->getComponentByType('GridFieldDataColumns')
+                ->setDisplayFields(Config::inst()->get('DMSDocument_versions', 'display_fields'))
                 ->setFieldCasting(array('LastChanged'=>"Datetime->Ago"))
-                                    ->setFieldFormatting(array('FilenameWithoutID'=>'<a target=\'_blank\' class=\'file-url\' href=\'$Link\'>$FilenameWithoutID</a>'));
+                ->setFieldFormatting(
+                    array(
+                        'FilenameWithoutID' => '<a target=\'_blank\' class=\'file-url\' href=\'$Link\'>'
+                            . '$FilenameWithoutID</a>'
+                    )
+                );
 
             $versionsGrid =  GridField::create(
                 'Versions',
@@ -1004,7 +1015,8 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
             //$extraFields = $versionsGrid->addExtraClass('find-versions');
         }
 
-        $fields->add(new LiteralField('BottomTaskSelection',
+        $fields->add(new LiteralField(
+            'BottomTaskSelection',
             '<div id="Actions" class="field actions"><label class="left">Actions</label><ul>'.
             '<li class="ss-ui-button" data-panel="embargo">Embargo</li>'.
             '<li class="ss-ui-button" data-panel="expiry">Expiry</li>'.
@@ -1023,17 +1035,41 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
         } elseif (!empty($this->EmbargoedUntilDate)) {
             $embargoValue = 'Date';
         }
-        $embargo = new OptionsetField('Embargo', 'Embargo', array('None'=>'None', 'Published'=>'Hide document until page is published', 'Indefinitely'=>'Hide document indefinitely', 'Date'=>'Hide until set date'), $embargoValue);
+        $embargo = new OptionsetField(
+            'Embargo',
+            'Embargo',
+            array(
+                'None' => 'None',
+                'Published' => 'Hide document until page is published',
+                'Indefinitely' => 'Hide document indefinitely',
+                'Date' => 'Hide until set date'
+            ),
+            $embargoValue
+        );
         $embargoDatetime = DatetimeField::create('EmbargoedUntilDate', '');
-        $embargoDatetime->getDateField()->setConfig('showcalendar', true)->setConfig('dateformat', 'dd-MM-yyyy')->setConfig('datavalueformat', 'dd-MM-yyyy');
+        $embargoDatetime->getDateField()
+            ->setConfig('showcalendar', true)
+            ->setConfig('dateformat', 'dd-MM-yyyy')
+            ->setConfig('datavalueformat', 'dd-MM-yyyy');
 
         $expiryValue = 'None';
         if (!empty($this->ExpireAtDate)) {
             $expiryValue = 'Date';
         }
-        $expiry = new OptionsetField('Expiry', 'Expiry', array('None'=>'None', 'Date'=>'Set document to expire on'), $expiryValue);
+        $expiry = new OptionsetField(
+            'Expiry',
+            'Expiry',
+            array(
+                'None' => 'None',
+                'Date' => 'Set document to expire on'
+            ),
+            $expiryValue
+        );
         $expiryDatetime = DatetimeField::create('ExpireAtDate', '');
-        $expiryDatetime->getDateField()->setConfig('showcalendar', true)->setConfig('dateformat', 'dd-MM-yyyy')->setConfig('datavalueformat', 'dd-MM-yyyy');
+        $expiryDatetime->getDateField()
+            ->setConfig('showcalendar', true)
+            ->setConfig('dateformat', 'dd-MM-yyyy')
+            ->setConfig('datavalueformat', 'dd-MM-yyyy');
 
         // This adds all the actions details into a group.
         // Embargo, History, etc to go in here
@@ -1045,28 +1081,22 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
                 $embargo,
                 $embargoDatetime
             )->addExtraClass('embargo'),
-
             FieldGroup::create(
                 $expiry,
                 $expiryDatetime
             )->addExtraClass('expiry'),
-
             FieldGroup::create(
                 $uploadField
             )->addExtraClass('replace'),
-
             FieldGroup::create(
                 $pagesGrid
             )->addExtraClass('find-usage'),
-
             FieldGroup::create(
                 $referencesGrid
             )->addExtraClass('find-references'),
-
             FieldGroup::create(
                 $versionsGrid
             )->addExtraClass('find-versions')
-
         );
 
         $actionsPanel->setName("ActionsPanel");
@@ -1193,8 +1223,10 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
     {
         $extension = $this->getExtension();
 
-        $previewField = new LiteralField("ImageFull",
+        $previewField = new LiteralField(
-            "<img id='thumbnailImage' class='thumbnail-preview' src='{$this->Icon($extension)}?r=" . rand(1, 100000)  . "' alt='{$this->Title}' />\n"
+            "ImageFull",
+            "<img id='thumbnailImage' class='thumbnail-preview' src='{$this->Icon($extension)}?r="
+            . rand(1, 100000) . "' alt='{$this->Title}' />\n"
         );
 
         //count the number of pages this document is published on
@@ -1217,15 +1249,41 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
                 CompositeField::create(
                     CompositeField::create(
                         new ReadonlyField("ID", "ID number". ':', $this->ID),
-                        new ReadonlyField("FileType", _t('AssetTableField.TYPE', 'File type') . ':', self::get_file_type($extension)),
+                        new ReadonlyField(
-                        new ReadonlyField("Size", _t('AssetTableField.SIZE', 'File size') . ':', $this->getFileSizeFormatted()),
+                            "FileType",
-                        $urlField = new ReadonlyField('ClickableURL', _t('AssetTableField.URL', 'URL'),
+                            _t('AssetTableField.TYPE', 'File type') . ':',
-                            sprintf('<a href="%s" target="_blank" class="file-url">%s</a>', $this->getLink(), $this->getLink())
+                            self::get_file_type($extension)
+                        ),
+                        new ReadonlyField(
+                            "Size",
+                            _t('AssetTableField.SIZE', 'File size') . ':',
+                            $this->getFileSizeFormatted()
+                        ),
+                        $urlField = new ReadonlyField(
+                            'ClickableURL',
+                            _t('AssetTableField.URL', 'URL'),
+                            sprintf(
+                                '<a href="%s" target="_blank" class="file-url">%s</a>',
+                                $this->getLink(),
+                                $this->getLink()
+                            )
                         ),
                         new ReadonlyField("FilenameWithoutIDField", "Filename". ':', $this->getFilenameWithoutID()),
-                        new DateField_Disabled("Created", _t('AssetTableField.CREATED', 'First uploaded') . ':', $this->Created),
+                        new DateField_Disabled(
-                        new DateField_Disabled("LastEdited", _t('AssetTableField.LASTEDIT', 'Last changed') . ':', $this->LastEdited),
+                            "Created",
-                        new DateField_Disabled("LastChanged", _t('AssetTableField.LASTCHANGED', 'Last replaced') . ':', $this->LastChanged),
+                            _t('AssetTableField.CREATED', 'First uploaded') . ':',
+                            $this->Created
+                        ),
+                        new DateField_Disabled(
+                            "LastEdited",
+                            _t('AssetTableField.LASTEDIT', 'Last changed') . ':',
+                            $this->LastEdited
+                        ),
+                        new DateField_Disabled(
+                            "LastChanged",
+                            _t('AssetTableField.LASTCHANGED', 'Last replaced') . ':',
+                            $this->LastChanged
+                        ),
                         new ReadonlyField("PublishedOn", "Published on". ':', $publishedOnValue),
                         new ReadonlyField("ReferencedOn", "Referenced on". ':', $relationListCountValue),
                         new ReadonlyField("ViewCount", "View count". ':', $this->ViewCount)
@@ -1246,7 +1304,7 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
      *
      * @param File $file
      *
-     * @return DMSDocument
+     * @return $this
      */
     public function ingestFile($file)
     {
@@ -1256,161 +1314,3 @@ class DMSDocument extends DataObject implements DMSDocumentInterface
         return $this;
     }
 }
-
-/**
- * @package dms
- */
-class DMSDocument_Controller extends Controller
-{
-
-    public static $testMode = false;   //mode to switch for testing. Does not return document download, just document URL
-
-    private static $allowed_actions = array(
-        'index'
-    );
-
-    public function init()
-    {
-        Versioned::choose_site_stage();
-        parent::init();
-    }
-
-    /**
-     * Returns the document object from the request object's ID parameter.
-     * Returns null, if no document found
-     * @return DMSDocument|null
-     */
-    protected function getDocumentFromID($request)
-    {
-        $doc = null;
-
-        $id = Convert::raw2sql($request->param('ID'));
-
-        if (strpos($id, 'version') === 0) { //versioned document
-            $id = str_replace('version', '', $id);
-            $doc = DataObject::get_by_id('DMSDocument_versions', $id);
-            $this->extend('updateVersionFromID', $doc, $request);
-        } else {    //normal document
-            $doc = DataObject::get_by_id('DMSDocument', $id);
-            $this->extend('updateDocumentFromID', $doc, $request);
-        }
-
-        return $doc;
-    }
-
-    /**
-     * Access the file download without redirecting user, so we can block direct
-     * access to documents.
-     */
-    public function index(SS_HTTPRequest $request)
-    {
-        $doc = $this->getDocumentFromID($request);
-
-        if (!empty($doc)) {
-            $canView = false;
-
-            // Runs through all pages that this page links to and sets canView
-            // to true if the user can view ONE of these pages
-            if (method_exists($doc, 'Pages')) {
-                $pages = $doc->Pages();
-                if ($pages->Count() > 0) {
-                    foreach ($pages as $page) {
-                        if ($page->CanView()) {
-                            // just one canView is enough to know that we can
-                            // view the file
-                            $canView = true;
-                            break;
-                        }
-                    }
-                } else {
-                    // if the document isn't on any page, then allow viewing of
-                    // the document (because there is no canView() to consult)
-                    $canView = true;
-                }
-            }
-
-            // check for embargo or expiry
-            if ($doc->isHidden()) {
-                $canView = false;
-            }
-
-            //admins can always download any document, even if otherwise hidden
-            $member = Member::currentUser();
-            if ($member && Permission::checkMember($member, 'ADMIN')) {
-                $canView = true;
-            }
-
-            if ($canView) {
-                $path = $doc->getFullPath();
-                if (is_file($path)) {
-                    $fileBin = trim(`whereis file`);
-                    if (function_exists('finfo_file')) {
-                        // discover the mime type properly
-                        $finfo = finfo_open(FILEINFO_MIME_TYPE);
-                        $mime = finfo_file($finfo, $path);
-                    } elseif (is_executable($fileBin)) {
-                        // try to use the system tool
-                        $mime = `$fileBin -i -b $path`;
-                        $mime = explode(';', $mime);
-                        $mime = trim($mime[0]);
-                    } else {
-                        // make do with what we have
-                        $ext = $doc->getExtension();
-                        if ($ext =='pdf') {
-                            $mime = 'application/pdf';
-                        } elseif ($ext == 'html' || $ext =='htm') {
-                            $mime = 'text/html';
-                        } else {
-                            $mime = 'application/octet-stream';
-                        }
-                    }
-
-                    if (self::$testMode) {
-                        return $path;
-                    }
-
-                    // set fallback if no config nor file-specific value
-                    $disposition = 'attachment';
-
-                    // file-specific setting
-                    if ($doc->DownloadBehavior == 'open') {
-                        $disposition = 'inline';
-                    }
-
-                    //if a DMSDocument can be downloaded and all the permissions/privileges has passed,
-                    //its ViewCount should be increased by 1 just before the browser sending the file to front.
-                    $doc->trackView();
-
-                    $this->sendFile($path, $mime, $doc->getFilenameWithoutID(), $disposition);
-                    return;
-                }
-            }
-        }
-
-        if (self::$testMode) {
-            return 'This asset does not exist.';
-        }
-        $this->httpError(404, 'This asset does not exist.');
-    }
-
-    /**
-     * @param string $path File path
-     * @param string $mime File mime type
-     * @param string $name File name
-     * @param string $disposition Content dispositon
-     */
-    protected function sendFile($path, $mime, $name, $disposition) {
-        header('Content-Type: ' . $mime);
-        header('Content-Length: ' . filesize($path), null);
-        if (!empty($mime) && $mime != "text/html") {
-            header('Content-Disposition: '.$disposition.'; filename="'.addslashes($name).'"');
-        }
-        header('Content-transfer-encoding: 8bit');
-        header('Expires: 0');
-        header('Pragma: cache');
-        header('Cache-Control: private');
-        flush();
-        readfile($path);
-        exit;
-    }
-}

code/model/DMSDocument_Controller.php

@@ -0,0 +1,165 @@
+<?php
+
+class DMSDocument_Controller extends Controller
+{
+    /**
+     * Mode to switch for testing. Does not return document download, just document URL.
+     *
+     * @var boolean
+     */
+    public static $testMode = false;
+
+    private static $allowed_actions = array(
+        'index'
+    );
+
+    public function init()
+    {
+        Versioned::choose_site_stage();
+        parent::init();
+    }
+
+    /**
+     * Returns the document object from the request object's ID parameter.
+     * Returns null, if no document found
+     *
+     * @param  SS_HTTPRequest $request
+     * @return DMSDocument|null
+     */
+    protected function getDocumentFromID($request)
+    {
+        $doc = null;
+
+        $id = Convert::raw2sql($request->param('ID'));
+
+        if (strpos($id, 'version') === 0) {
+            // Versioned document
+            $id = str_replace('version', '', $id);
+            $doc = DataObject::get_by_id('DMSDocument_versions', $id);
+            $this->extend('updateVersionFromID', $doc, $request);
+        } else {
+            // Normal document
+            $doc = DataObject::get_by_id('DMSDocument', $id);
+            $this->extend('updateDocumentFromID', $doc, $request);
+        }
+
+        return $doc;
+    }
+
+    /**
+     * Access the file download without redirecting user, so we can block direct
+     * access to documents.
+     */
+    public function index(SS_HTTPRequest $request)
+    {
+        $doc = $this->getDocumentFromID($request);
+
+        if (!empty($doc)) {
+            $canView = false;
+
+            // Runs through all pages that this page links to and sets canView
+            // to true if the user can view ONE of these pages
+            if (method_exists($doc, 'Pages')) {
+                $pages = $doc->Pages();
+                if ($pages->Count() > 0) {
+                    foreach ($pages as $page) {
+                        if ($page->CanView()) {
+                            // just one canView is enough to know that we can
+                            // view the file
+                            $canView = true;
+                            break;
+                        }
+                    }
+                } else {
+                    // if the document isn't on any page, then allow viewing of
+                    // the document (because there is no canView() to consult)
+                    $canView = true;
+                }
+            }
+
+            // check for embargo or expiry
+            if ($doc->isHidden()) {
+                $canView = false;
+            }
+
+            //admins can always download any document, even if otherwise hidden
+            $member = Member::currentUser();
+            if ($member && Permission::checkMember($member, 'ADMIN')) {
+                $canView = true;
+            }
+
+            if ($canView) {
+                $path = $doc->getFullPath();
+                if (is_file($path)) {
+                    $fileBin = trim(`whereis file`);
+                    if (function_exists('finfo_file')) {
+                        // discover the mime type properly
+                        $finfo = finfo_open(FILEINFO_MIME_TYPE);
+                        $mime = finfo_file($finfo, $path);
+                    } elseif (is_executable($fileBin)) {
+                        // try to use the system tool
+                        $mime = `$fileBin -i -b $path`;
+                        $mime = explode(';', $mime);
+                        $mime = trim($mime[0]);
+                    } else {
+                        // make do with what we have
+                        $ext = $doc->getExtension();
+                        if ($ext =='pdf') {
+                            $mime = 'application/pdf';
+                        } elseif ($ext == 'html' || $ext =='htm') {
+                            $mime = 'text/html';
+                        } else {
+                            $mime = 'application/octet-stream';
+                        }
+                    }
+
+                    if (self::$testMode) {
+                        return $path;
+                    }
+
+                    // set fallback if no config nor file-specific value
+                    $disposition = 'attachment';
+
+                    // file-specific setting
+                    if ($doc->DownloadBehavior == 'open') {
+                        $disposition = 'inline';
+                    }
+
+                    //if a DMSDocument can be downloaded and all the permissions/privileges has passed,
+                    //its ViewCount should be increased by 1 just before the browser sending the file to front.
+                    $doc->trackView();
+
+                    $this->sendFile($path, $mime, $doc->getFilenameWithoutID(), $disposition);
+                    return;
+                }
+            }
+        }
+
+        if (self::$testMode) {
+            return 'This asset does not exist.';
+        }
+        $this->httpError(404, 'This asset does not exist.');
+    }
+
+    /**
+     * @param string $path File path
+     * @param string $mime File mime type
+     * @param string $name File name
+     * @param string $disposition Content dispositon
+     */
+    protected function sendFile($path, $mime, $name, $disposition)
+    {
+        header('Content-Type: ' . $mime);
+        header('Content-Length: ' . filesize($path), null);
+        if (!empty($mime) && $mime != "text/html") {
+            header('Content-Disposition: '.$disposition.'; filename="'.addslashes($name).'"');
+        }
+        header('Content-transfer-encoding: 8bit');
+        header('Expires: 0');
+        header('Pragma: cache');
+        header('Cache-Control: private');
+        flush();
+        readfile($path);
+        exit;
+    }
+}

code/model/DMSDocument_versions.php

@@ -206,14 +206,17 @@ class DMSDocument_versions extends DataObject
         $filename = $doc->Filename;
 
         do {
-            $versionPaddingString = str_pad($versionCounter, 4, '0', STR_PAD_LEFT); //add leading zeros to make sorting accurate up to 10,000 documents
+            // Add leading zeros to make sorting accurate up to 10,000 documents
+            $versionPaddingString = str_pad($versionCounter, 4, '0', STR_PAD_LEFT);
             $newVersionFilename = preg_replace('/([0-9]+~)(.*?)/', '$1~'.$versionPaddingString.'~$2', $filename);
 
-            if ($newVersionFilename == $filename || empty($newVersionFilename)) {   //sanity check for crazy document names
+            // Sanity check for crazy document names
+            if ($newVersionFilename == $filename || empty($newVersionFilename)) {
                 user_error('Cannot generate new document filename for file: '.$filename, E_USER_ERROR);
             }
 
-            $versionCounter++;  //increase the counter for the next loop run, if necessary
+            // Increase the counter for the next loop run, if necessary
+            $versionCounter++;
         } while (file_exists($this->getFullPath($newVersionFilename)));
 
         return $newVersionFilename;

code/model/DMSTag.php

@@ -1,5 +1,4 @@
 <?php
-
 /**
  * Hold a set of metadata category/value tags associated with a DMSDocument
  *
@@ -7,7 +6,6 @@
  */
 class DMSTag extends DataObject
 {
-
     private static $db = array(
         'Category' => 'Varchar(1024)',
         'Value' => 'Varchar(1024)',

javascript/DMSDocumentAddExistingField.js

@@ -15,9 +15,15 @@
 							var fnout = fn({
 								files: [data],
                                 formatFileSize: function (bytes) {
-									if (typeof bytes !== 'number') return '';
+									if (typeof bytes !== 'number') {
-									if (bytes >= 1000000000) return (bytes / 1000000000).toFixed(2) + ' GB';
+                                        return '';
-									if (bytes >= 1000000) return (bytes / 1000000).toFixed(2) + ' MB';
+                                    }
+									if (bytes >= 1000000000) {
+                                        return (bytes / 1000000000).toFixed(2) + ' GB';
+                                    }
+									if (bytes >= 1000000) {
+                                        return (bytes / 1000000).toFixed(2) + ' MB';
+                                    }
 									return (bytes / 1000).toFixed(2) + ' KB';
 								}
 		                    });

javascript/DMSDocumentCMSFields.js

@@ -7,7 +7,9 @@
 			onadd: function () {
 				this.addClass('ui-button ss-ui-button ui-corner-all ui-state-default ui-widget ui-button-text-only');
 				this.parents('ul').removeClass('ui-tabs-nav');
-				if(this.find('input').is(':checked')) this.addClass('selected');
+				if (this.find('input').is(':checked')) {
+                    this.addClass('selected');
+                }
 			},
 			onclick: function (e) {
 				$('#DocumentTypeID').find('li.selected').removeClass('selected');

javascript/DocumentHtmlEditorFieldToolbar.js

@@ -36,7 +36,9 @@
 					href = '[dms_document_link,id=' + this.find('.selected-document').data('document-id') + ']';
 
 					// Determine target
-					if(this.find(':input[name=TargetBlank]').is(':checked')) target = '_blank';
+					if (this.find(':input[name=TargetBlank]').is(':checked')) {
+                        target = '_blank';
+                    }
 
 					var attributes = {
 						href : href,
@@ -65,12 +67,16 @@
 						linkDataSource = selectedEl = selectedEl.parents('a:first');
 					}
 				}
-				if(linkDataSource && linkDataSource.length) this.modifySelection(function(ed){
+				if (linkDataSource && linkDataSource.length) {
+                    this.modifySelection(function (ed) {
                                         ed.selectNode(linkDataSource[0]);
                     });
+                }
 
 				// Is anchor not a link
-				if (!linkDataSource.attr('href')) linkDataSource = null;
+				if (!linkDataSource.attr('href')) {
+                    linkDataSource = null;
+                }
 
 				if (linkDataSource) {
 					href = linkDataSource.attr('href');

tests/DMSDocumentControllerTest.php

@@ -3,25 +3,40 @@
 /**
  * Class DMSDocumentControllerTest
  */
-class DMSDocumentControllerTest extends SapphireTest {
+class DMSDocumentControllerTest extends SapphireTest
-
+{
     protected static $fixture_file = "dmstest.yml";
 
-    public function testDownloadBehaviourOpen() {
+    /**
+     * Test that the download behaviour is either "open" or "download"
+     *
+     * @param string $behaviour
+     * @param string $expectedDisposition
+     * @dataProvider behaviourProvider
+     */
+    public function testDownloadBehaviourOpen($behaviour, $expectedDisposition)
+    {
         DMS::$dmsFolder = DMS_DIR;    //sneakily setting the DMS folder to the folder where the test file lives
 
         $this->logInWithPermission('ADMIN');
+
         /** @var DMSDocument_Controller $controller */
         $controller = $this->getMockBuilder('DMSDocument_Controller')
             ->setMethods(array('sendFile'))->getMock();
+
         $self = $this;
-        $controller->expects($this->once())->method('sendFile')->will($this->returnCallback(function($path, $mime, $name, $disposition) use($self) {
+        $controller->expects($this->once())
-            $self->assertEquals('inline', $disposition);
+            ->method('sendFile')
-        }));
+            ->will(
+                $this->returnCallback(function ($path, $mime, $name, $disposition) use ($self, $expectedDisposition) {
+                    $self->assertEquals($expectedDisposition, $disposition);
+                })
+            );
+
         $openDoc = new DMSDocument();
         $openDoc->Filename = "DMS-test-lorum-file.pdf";
         $openDoc->Folder = "tests";
-        $openDoc->DownloadBehavior = 'open';
+        $openDoc->DownloadBehavior = $behaviour;
         $openDoc->clearEmbargo(false);
         $openDoc->write();
         $request = new SS_HTTPRequest('GET', 'index/' . $openDoc->ID);
@@ -29,26 +44,14 @@ class DMSDocumentControllerTest extends SapphireTest {
         $controller->index($request);
     }
 
-    public function testDownloadBehaviourDownload() {
+    /**
-        DMS::$dmsFolder = DMS_DIR;    //sneakily setting the DMS folder to the folder where the test file lives
+     * @return array[]
-
+     */
-        $this->logInWithPermission('ADMIN');
+    public function behaviourProvider()
-        /** @var DMSDocument_Controller $controller */
+    {
-        $controller = $this->getMockBuilder('DMSDocument_Controller')
+        return array(
-            ->setMethods(array('sendFile'))->getMock();
+            array('open', 'inline'),
-        $self = $this;
+            array('download', 'attachment')
-        $controller->expects($this->once())->method('sendFile')->will($this->returnCallback(function($path, $mime, $name, $disposition) use($self) {
+        );
-            $self->assertEquals('attachment', $disposition);
-        }));
-        $openDoc = new DMSDocument();
-        $openDoc->Filename = "DMS-test-lorum-file.pdf";
-        $openDoc->Folder = "tests";
-        $openDoc->DownloadBehavior = 'download';
-        $openDoc->clearEmbargo(false);
-        $openDoc->write();
-        $request = new SS_HTTPRequest('GET', 'index/' . $openDoc->ID);
-        $request->match('index/$ID');
-        $controller->index($request);
     }
-
 }

tests/DMSDocumentTest.php

@@ -1,8 +1,7 @@
 <?php
 class DMSDocumentTest extends SapphireTest
 {
-
+    protected static $fixture_file = "dmstest.yml";
-    protected static $fixture_file = "dms/tests/dmstest.yml";
 
     public function tearDownOnce()
     {
@@ -64,13 +63,19 @@ class DMSDocumentTest extends SapphireTest
 
         $doc->removePage($s1);
         $pages = $doc->Pages();
-        $pagesArray = $pages->toArray();    //page 1 is missing
+        $pagesArray = $pages->toArray();    // Page 1 is missing
         $this->assertEquals($pagesArray[0]->ID, $s2->ID, "Page 2 still associated correctly");
         $this->assertEquals($pagesArray[1]->ID, $s3->ID, "Page 3 still associated correctly");
 
         $documents = $s2->Documents();
         $documentsArray = $documents->toArray();
-        $this->assertDOSContains(array(array('Filename'=>$doc->Filename)), $documentsArray, "Document associated with page");
+        $this->assertDOSContains(
+            array(
+                array('Filename' => $doc->Filename)
+            ),
+            $documentsArray,
+            "Document associated with page"
+        );
 
         $doc->removeAllPages();
         $pages = $doc->Pages();
@@ -158,7 +163,8 @@ class DMSDocumentTest extends SapphireTest
         );
     }
 
-    public function testDefaultDownloadBehabiourCMSFields() {
+    public function testDefaultDownloadBehabiourCMSFields()
+    {
         $document = singleton('DMSDocument');
         Config::inst()->update('DMSDocument', 'default_download_behaviour', 'open');
         $cmsFields = $document->getCMSFields();

tests/DMSEmbargoTest.php

@@ -1,8 +1,7 @@
 <?php
 class DMSEmbargoTest extends SapphireTest
 {
-
+    public static $fixture_file = "dmsembargotest.yml";
-    public static $fixture_file = "dms/tests/dmsembargotest.yml";
 
     public function tearDownOnce()
     {
@@ -52,7 +51,11 @@ class DMSEmbargoTest extends SapphireTest
 
         $this->logInWithPermission('random-user-group');
         $result = $controller->index($this->createFakeHTTPRequest($docID));
-        $this->assertNotEquals($doc->getFullPath(), $result, "File no longer returned (in test mode) when switching to other user group");
+        $this->assertNotEquals(
+            $doc->getFullPath(),
+            $result,
+            "File no longer returned (in test mode) when switching to other user group"
+        );
 
         DMS::$dmsFolder = $oldDMSFolder;
         DMSDocument_Controller::$testMode = $oldTestMode;
@@ -192,7 +195,10 @@ class DMSEmbargoTest extends SapphireTest
         $s1->publish('Stage', 'Live');
         $s1->doPublish();
         $doc = DataObject::get_by_id("DMSDocument", $dID);
-        $this->assertTrue($doc->isHidden(), "Document is still hidden because although the untilPublish flag is cleared, the indefinitely flag is still there");
+        $this->assertTrue(
+            $doc->isHidden(),
+            "Document is still hidden because although the untilPublish flag is cleared, the indefinitely flag is there"
+        );
         $this->assertTrue($doc->isEmbargoed(), "Document is embargoed");
         $this->assertFalse($doc->isExpired(), "Document is not expired");
 

tests/DMSShortcodeTest.php

@@ -7,14 +7,14 @@
  */
 class DMSShortcodeTest extends SapphireTest
 {
-
     public function testShortcodeOperation()
     {
         $file = 'dms/tests/DMS-test-lorum-file.pdf';
         $document = DMS::inst()->storeDocument($file);
 
         $result = ShortcodeParser::get('default')->parse(sprintf(
-            '<p><a href="[dms_document_link id=\'%d\']">Document</a></p>', $document->ID
+            '<p><a href="[dms_document_link id=\'%d\']">Document</a></p>',
+            $document->ID
         ));
 
         $value = Injector::inst()->create('HTMLValue', $result);

tests/DMSTagTest.php

@@ -1,6 +1,7 @@
 <?php
 class DMSTagTest extends SapphireTest
 {
+    protected $usesDatabase = true;
 
     public function tearDownOnce()
     {

tests/DMSTest.php

@@ -1,11 +1,18 @@
 <?php
 class DMSTest extends FunctionalTest
 {
+    protected $usesDatabase = true;
 
+    /**
+     * Stub PDF files for testing
+     * @var string
+     */
     public static $testFile = 'dms/tests/DMS-test-lorum-file.pdf';
     public static $testFile2 = 'dms/tests/DMS-test-document-2.pdf';
 
-    //store values to reset back to after this test runs
+    /**
+     * Store values to reset back to after this test runs
+     */
     public static $dmsFolderOld;
     public static $dmsFolderSizeOld;
 
@@ -48,6 +55,11 @@ class DMSTest extends FunctionalTest
         self::$is_running_test = $this->originalIsRunningTest;
     }
 
+    /**
+     * Delete a file that was created during a unit test
+     *
+     * @param string $path
+     */
     public function delete($path)
     {
         if (file_exists($path) || is_dir($path)) {
@@ -68,7 +80,6 @@ class DMSTest extends FunctionalTest
         }
     }
 
-
     public function testDMSStorage()
     {
         $dms = DMS::inst();
@@ -77,9 +88,13 @@ class DMSTest extends FunctionalTest
         $document = $dms->storeDocument($file);
 
         $this->assertNotNull($document, "Document object created");
-        $this->assertTrue(file_exists(DMS::get_dms_path() . DIRECTORY_SEPARATOR . $document->Folder . DIRECTORY_SEPARATOR . $document->Filename), "Document file copied into DMS folder");
+        $this->assertTrue(
-
+            file_exists(
-        //$title = $document->getTag('title');
+                DMS::get_dms_path() . DIRECTORY_SEPARATOR . $document->Folder
+                . DIRECTORY_SEPARATOR . $document->Filename
+            ),
+            "Document file copied into DMS folder"
+        );
     }
 
     public function testDMSFolderSpanning()
@@ -97,15 +112,18 @@ class DMSTest extends FunctionalTest
             $documents[] = $document;
         }
 
-        //test document objects have their folders set
+        // Test document objects have their folders set
         $folders = array();
         for ($i = 0; $i <= 16; $i++) {
             $folderName = $documents[$i]->Folder;
-            $this->assertTrue(strpos($documents[$i]->getFullPath(), DIRECTORY_SEPARATOR . $folderName . DIRECTORY_SEPARATOR) !== false, "Correct folder name for the documents. Document path contains reference to folder name '$folderName'");
+            $this->assertTrue(
+                strpos($documents[$i]->getFullPath(), DIRECTORY_SEPARATOR . $folderName . DIRECTORY_SEPARATOR) !== false,
+                "Correct folder name for the documents. Document path contains reference to folder name '$folderName'"
+            );
             $folders[] = $folderName;
         }
 
-        //test we created 4 folder to contain the 17 files
+        // Test we created 4 folder to contain the 17 files
         foreach ($folders as $f) {
             $this->assertTrue(is_dir(DMS::get_dms_path() . DIRECTORY_SEPARATOR . $f), "Document folder '$f' exists");
         }
@@ -115,34 +133,29 @@ class DMSTest extends FunctionalTest
     {
         $dms = DMS::inst();
 
-        //store the first document
+        // Store the first document
         $document = $dms->storeDocument(self::$testFile);
         $document->Title = "My custom title";
         $document->Description = "My custom description";
         $document->write();
 
-        //then overwrite with a second document
+        // Then overwrite with a second document
         $document = $document->replaceDocument(self::$testFile2);
 
         $this->assertNotNull($document, "Document object created");
-        $this->assertTrue(file_exists(DMS::get_dms_path() . DIRECTORY_SEPARATOR . $document->Folder . DIRECTORY_SEPARATOR . $document->Filename), "Document file copied into DMS folder");
+        $this->assertTrue(
-        $this->assertContains("DMS-test-document-2", $document->Filename, "Original document filename is contain in the new filename");
+            file_exists(
+                DMS::get_dms_path() . DIRECTORY_SEPARATOR . $document->Folder
+                . DIRECTORY_SEPARATOR . $document->Filename
+            ),
+            "Document file copied into DMS folder"
+        );
+        $this->assertContains(
+            "DMS-test-document-2",
+            $document->Filename,
+            "Original document filename is contain in the new filename"
+        );
         $this->assertEquals("My custom title", $document->Title, "Custom title not modified");
         $this->assertEquals("My custom description", $document->Description, "Custom description not modified");
     }
-
-    public function testDownloadDocument()
-    {
-        //		$dms = DMS::inst();
-//
-//		//store the first document
-//		$document = $dms->storeDocument(self::$testFile);
-//		$link = $document->getLink();
-//
-//		Debug::Show($link);
-//		$d=new DMSDocument_Controller();
-//		$response = $d->index(new SS_HTTPRequest('GET',$link,array("ID"=>$document->ID)));
-//		//$response = $this->get($link);
-//		Debug::show($response);
-    }
 }

tests/DMSVersioningTest.php

@@ -1,11 +1,18 @@
 <?php
 class DMSVersioningTest extends SapphireTest
 {
+    protected $usesDatabase = true;
 
+    /**
+     * Stub PDF files for testing
+     * @var string
+     */
     public static $testFile = 'dms/tests/DMS-test-lorum-file.pdf';
     public static $testFile2 = 'dms/tests/DMS-test-document-2.pdf';
 
-    //store values to reset back to after this test runs
+    /**
+     * Store values to reset back to after this test runs
+     */
     public static $dmsFolderOld;
     public static $dmsFolderSizeOld;
     public static $dmsEnableVersionsOld;
@@ -37,17 +44,22 @@ class DMSVersioningTest extends SapphireTest
             $t1->delete();
         }
 
-        //delete the test folder after the test runs
+        // Delete the test folder after the test runs
         $this->delete(BASE_PATH . DIRECTORY_SEPARATOR . 'dms-assets-test-versions');
 
         parent::tearDown();
 
-        //set the old DMS folder back again
+        // Set the old DMS folder back again
         DMS::$dmsFolder = self::$dmsFolderOld;
         DMS::$dmsFolderSize = self::$dmsFolderSizeOld;
         DMSDocument_versions::$enable_versions = self::$dmsEnableVersionsOld;
     }
 
+    /**
+     * Delete a file that was created during a unit test
+     *
+     * @param string $path
+     */
     public function delete($path)
     {
         if (file_exists($path) || is_dir($path)) {
@@ -68,7 +80,6 @@ class DMSVersioningTest extends SapphireTest
         }
     }
 
-
     public function testDMSVersionStorage()
     {
         $dms = DMS::inst();
@@ -76,7 +87,13 @@ class DMSVersioningTest extends SapphireTest
         $document = $dms->storeDocument(self::$testFile);
 
         $this->assertNotNull($document, "Document object created");
-        $this->assertTrue(file_exists(DMS::get_dms_path() . DIRECTORY_SEPARATOR . $document->Folder . DIRECTORY_SEPARATOR . $document->Filename), "Document file copied into DMS folder");
+        $this->assertTrue(
+            file_exists(
+                DMS::get_dms_path() . DIRECTORY_SEPARATOR . $document->Folder
+                . DIRECTORY_SEPARATOR . $document->Filename
+            ),
+            "Document file copied into DMS folder"
+        );
 
         $document->replaceDocument(self::$testFile2);
         $document->replaceDocument(self::$testFile);

tests/ShortCodeRelationFinderTest.php

@@ -1,10 +1,7 @@
 <?php
 class ShortCodeRelationFinderTest extends SapphireTest
 {
-
+    protected static $fixture_file = 'dmstest.yml';
-    public static $fixture_file = array(
-        'dms/tests/dmstest.yml'
-    );
 
     public function testFindInRate()
     {