From dd6ffbfaf8f07d6d6b1b60a2655879210480722b Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Tue, 7 May 2013 20:56:38 +1200 Subject: [PATCH 1/3] FIX: WidgetControllerTest failing due to 3.1 API. Update package information --- README.md | 2 +- .../WidgetContentControllerExtension.php | 43 ++-- code/controller/WidgetController.php | 121 ++++++++++ code/form/WidgetAreaEditor.php | 24 +- code/model/Widget.php | 208 +++++------------- code/model/WidgetArea.php | 27 +-- javascript/WidgetAreaEditor.js | 2 +- tests/WidgetControllerTest.php | 18 +- 8 files changed, 235 insertions(+), 210 deletions(-) create mode 100644 code/controller/WidgetController.php diff --git a/README.md b/README.md index babe675..dd21c8c 100644 --- a/README.md +++ b/README.md @@ -241,7 +241,7 @@ sure that your controller follows the usual naming conventions, and it will be a ); } - class MyWidget_Controller extends Widget_Controller { + class MyWidget_Controller extends WidgetController { public function MyFormName() { return new Form( $this, diff --git a/code/controller/WidgetContentControllerExtension.php b/code/controller/WidgetContentControllerExtension.php index 8e5d2b2..69e5cbb 100644 --- a/code/controller/WidgetContentControllerExtension.php +++ b/code/controller/WidgetContentControllerExtension.php @@ -1,6 +1,9 @@ + /widget/. * * @return RequestHandler @@ -27,7 +33,11 @@ class WidgetContentControllerExtension extends Extension { // find WidgetArea relations $widgetAreaRelations = array(); $hasOnes = $this->owner->data()->has_one(); - if(!$hasOnes) return false; + + if(!$hasOnes) { + return false; + } + foreach($hasOnes as $hasOneName => $hasOneClass) { if($hasOneClass == 'WidgetArea' || is_subclass_of($hasOneClass, 'WidgetArea')) { $widgetAreaRelations[] = $hasOneName; @@ -36,26 +46,21 @@ class WidgetContentControllerExtension extends Extension { // find widget $widget = null; + foreach($widgetAreaRelations as $widgetAreaRelation) { - if($widget) break; + if($widget) { + break; + } + $widget = $this->owner->data()->$widgetAreaRelation()->Widgets( sprintf('"Widget"."ID" = %d', $SQL_id) )->First(); } - if(!$widget) user_error('No widget found', E_USER_ERROR); - - // find controller - $controllerClass = ''; - foreach(array_reverse(ClassInfo::ancestry($widget->class)) as $widgetClass) { - $controllerClass = "{$widgetClass}_Controller"; - if(class_exists($controllerClass)) break; + + if(!$widget) { + user_error('No widget found', E_USER_ERROR); } - if(!$controllerClass) user_error( - sprintf('No controller available for %s', $widget->class), - E_USER_ERROR - ); - - return new $controllerClass($widget); + + return $widget->getController(); } - } \ No newline at end of file diff --git a/code/controller/WidgetController.php b/code/controller/WidgetController.php new file mode 100644 index 0000000..1896296 --- /dev/null +++ b/code/controller/WidgetController.php @@ -0,0 +1,121 @@ +controller->getWidget()` inside the form logic. + * + * Note: Widget controllers currently only work on {@link Page} objects, + * because the logic is implemented in {@link ContentController->handleWidget()}. + * Copy this logic and the URL rules to enable it for other controllers. + * + * @package widgets + */ +class WidgetController extends Controller { + + /** + * @var Widget + */ + protected $widget; + + /** + * @var array + */ + private static $allowed_actions = array( + 'editablesegment' + ); + + /** + * @param Widget $widget + */ + public function __construct($widget = null) { + if($widget) { + $this->widget = $widget; + $this->failover = $widget; + } + + parent::__construct(); + } + + /** + * @param string $action + * @return string + */ + public function Link($action = null) { + $id = ($this->widget) ? $this->widget->ID : null; + $segment = Controller::join_links('widget', $id, $action); + + if($page = Director::get_current_page()) { + return $page->Link($segment); + } + + return Controller::curr()->Link($segment); + } + + /** + * @return Widget + */ + public function getWidget() { + return $this->widget; + } + + /** + * Overloaded from {@link Widget->Content()} to allow for controller / form + * linking. + * + * @return string HTML + */ + public function Content() { + return $this->renderWith(array_reverse(ClassInfo::ancestry($this->widget->class))); + } + + /** + * Overloaded from {@link Widget->WidgetHolder()} to allow for controller/ + * form linking. + * + * @return string HTML + */ + public function WidgetHolder() { + return $this->renderWith("WidgetHolder"); + } + + /** + * Uses the `WidgetEditor.ss` template and {@link Widget->editablesegment()} + * to render a administrator-view of the widget. It is assumed that this + * view contains form elements which are submitted and saved through + * {@link WidgetAreaEditor} within the CMS interface. + * + * @return string HTML + */ + public function editablesegment() { + $className = $this->urlParams['ID']; + if (class_exists('Translatable') && Member::currentUserID()) { + // set current locale based on logged in user's locale + $locale = Member::currentUser()->Locale; + Translatable::set_current_locale($locale); + i18n::set_locale($locale); + } + if(class_exists($className) && is_subclass_of($className, 'Widget')) { + $obj = new $className(); + return $obj->EditableSegment(); + } else { + user_error("Bad widget class: $className", E_USER_WARNING); + return "Bad widget class name given"; + } + } +} + +/** + * @deprecated Use WidgetController + * @package widgets + */ +class Widget_Controller extends WidgetController { + +} diff --git a/code/form/WidgetAreaEditor.php b/code/form/WidgetAreaEditor.php index bc0cff7..8977e87 100644 --- a/code/form/WidgetAreaEditor.php +++ b/code/form/WidgetAreaEditor.php @@ -1,13 +1,13 @@ renderWith("WidgetAreaEditor"); } @@ -40,7 +41,11 @@ class WidgetAreaEditor extends FormField { foreach($this->widgetClasses as $widgetClass) { $classes = ClassInfo::subclassesFor($widgetClass); - if(count($classes) > 1) array_shift($classes); + + if(count($classes) > 1) { + array_shift($classes); + } + foreach($classes as $class) { $widgets->push(singleton($class)); } @@ -50,7 +55,6 @@ class WidgetAreaEditor extends FormField { } /** - * * @return HasManyList */ public function UsedWidgets() { @@ -59,11 +63,11 @@ class WidgetAreaEditor extends FormField { $relationName = $this->name; $widgets = $this->form->getRecord()->getComponent($relationName)->Items(); + return $widgets; } /** - * * @return string */ public function IdxField() { @@ -76,11 +80,11 @@ class WidgetAreaEditor extends FormField { */ public function Value() { $relationName = $this->name; + return $this->form->getRecord()->getComponent($relationName)->ID; } /** - * * @param DataObjectInterface $record */ public function saveInto(DataObjectInterface $record) { @@ -145,7 +149,7 @@ class WidgetAreaEditor extends FormField { if($widget->ParentID == 0) { $widget->ParentID = $record->$name()->ID; } - // echo "Saving $widget->ID into $name/$widget->ParentID\n
"; + $widget->populateFromPostData($newWidgetData); } } diff --git a/code/model/Widget.php b/code/model/Widget.php index 807a5ba..681b460 100644 --- a/code/model/Widget.php +++ b/code/model/Widget.php @@ -1,19 +1,18 @@ extend('updateCMSFields', $fields); + return $fields; } /** - * Note: Overloaded in {@link Widget_Controller}. + * Note: Overloaded in {@link WidgetController}. * * @return string HTML */ @@ -99,13 +95,13 @@ class Widget extends DataObject { } /** - * Renders the widget content in a custom template with the same name as the current class. - * This should be the main point of output customization. + * Renders the widget content in a custom template with the same name as the + * current class. This should be the main point of output customization. * - * Invoked from within WidgetHolder.ss, which contains - * the "framing" around the custom content, like a title. + * Invoked from within WidgetHolder.ss, which contains the "framing" around + * the custom content, like a title. * - * Note: Overloaded in {@link Widget_Controller}. + * Note: Overloaded in {@link WidgetController}. * * @return string HTML */ @@ -114,7 +110,6 @@ class Widget extends DataObject { } /** - * * @return string */ public function Title() { @@ -122,7 +117,6 @@ class Widget extends DataObject { } /** - * * @return string */ public function CMSTitle() { @@ -130,7 +124,6 @@ class Widget extends DataObject { } /** - * * @return string */ public function Description() { @@ -138,7 +131,6 @@ class Widget extends DataObject { } /** - * * @return string - HTML */ public function DescriptionSegment() { @@ -146,7 +138,8 @@ class Widget extends DataObject { } /** - * @see Widget_Controller->editablesegment() + * @see WidgetController::editablesegment() + * * @return string - HTML */ public function EditableSegment() { @@ -154,12 +147,12 @@ class Widget extends DataObject { } /** - * * @return FieldList */ public function CMSEditor() { $fields = $this->getCMSFields(); $outputFields = new FieldList(); + foreach($fields as $field) { $name = $field->getName(); $value = $this->getField($name); @@ -170,11 +163,11 @@ class Widget extends DataObject { $field->setName($name); $outputFields->push($field); } + return $outputFields; } /** - * * @return string */ public function ClassName() { @@ -182,7 +175,6 @@ class Widget extends DataObject { } /** - * * @return string */ public function Name() { @@ -190,7 +182,39 @@ class Widget extends DataObject { } /** + * @throws Exception * + * @return WidgetController + */ + public function getController() { + if($this->controller) { + return $this->controller; + } + + foreach(array_reverse(ClassInfo::ancestry($this->class)) as $widgetClass) { + $controllerClass = "{$widgetClass}_Controller"; + + if(class_exists($controllerClass)) { + break; + } + + $controllerClass = "{$widgetClass}Controller"; + + if(class_exists($controllerClass)) { + break; + } + } + + if(!class_exists($controllerClass)) { + throw new Exception("Could not find controller class for $this->classname"); + } + + $this->controller = Injector::inst()->create($controllerClass, $this); + + return $this->controller; + } + + /** * @param array $data */ public function populateFromPostData($data) { @@ -212,132 +236,6 @@ class Widget extends DataObject { // The field must be written to ensure a unique ID. $this->Name = $this->class.$this->ID; $this->write(); - } - -} - -/** - * Optional controller for every widget which has its own logic, - * e.g. in forms. It always handles a single widget, usually passed - * in as a database identifier through the controller URL. - * Needs to be constructed as a nested controller - * within a {@link ContentController}. - * - * ## Forms - * You can add forms like in any other SilverStripe controller. - * If you need access to the widget from within a form, - * you can use `$this->controller->getWidget()` inside the form logic. - * Note: Widget controllers currently only work on {@link Page} objects, - * because the logic is implemented in {@link ContentController->handleWidget()}. - * Copy this logic and the URL rules to enable it for other controllers. - * - * @package cms - * @subpackage widgets - */ -class Widget_Controller extends Controller { - - /** - * @var Widget - */ - protected $widget; - - /** - * - * @var array - */ - private static $allowed_actions = array( - 'editablesegment' - ); - - /** - * - * @param Widget $widget - */ - public function __construct($widget = null) { - // TODO This shouldn't be optional, is only necessary for editablesegment() - if($widget) { - $this->widget = $widget; - $this->failover = $widget; - } - - parent::__construct(); - } - - /** - * - * @param string $action - * @return string - */ - public function Link($action = null) { - $segment = Controller::join_links('widget', ($this->widget ? $this->widget->ID : null), $action); - - if(Director::get_current_page()) { - return Director::get_current_page()->Link($segment); - } else { - return Controller::curr()->Link($segment); - } - } - - /** - * @return Widget - */ - public function getWidget() { - return $this->widget; - } - - /** - * Overloaded from {@link Widget->Content()} - * to allow for controller/form linking. - * - * @return string HTML - */ - public function Content() { - return $this->renderWith(array_reverse(ClassInfo::ancestry($this->widget->class))); - } - - /** - * Overloaded from {@link Widget->WidgetHolder()} - * to allow for controller/form linking. - * - * @return string HTML - */ - public function WidgetHolder() { - return $this->renderWith("WidgetHolder"); - } - - /** - * Uses the `WidgetEditor.ss` template and {@link Widget->editablesegment()} - * to render a administrator-view of the widget. It is assumed that this - * view contains form elements which are submitted and saved through {@link WidgetAreaEditor} - * within the CMS interface. - * - * @return string HTML - */ - public function editablesegment() { - $className = $this->urlParams['ID']; - if (class_exists('Translatable') && Member::currentUserID()) { - // set current locale based on logged in user's locale - $locale = Member::currentUser()->Locale; - Translatable::set_current_locale($locale); - i18n::set_locale($locale); - } - if(class_exists($className) && is_subclass_of($className, 'Widget')) { - $obj = new $className(); - return $obj->EditableSegment(); - } else { - user_error("Bad widget class: $className", E_USER_WARNING); - return "Bad widget class name given"; - } } } -/** - * @package cms - * @subpackage widgets - */ -class Widget_TreeDropdownField extends TreeDropdownField { - - public function FieldHolder($properties = array()) {} - public function Field($properties = array()) {} -} - diff --git a/code/model/WidgetArea.php b/code/model/WidgetArea.php index 9d037f8..af3fa1d 100644 --- a/code/model/WidgetArea.php +++ b/code/model/WidgetArea.php @@ -1,13 +1,13 @@ ItemsToRender() as $widget) { - // find controller - $controllerClass = ''; - foreach(array_reverse(ClassInfo::ancestry($widget->class)) as $widgetClass) { - $controllerClass = "{$widgetClass}_Controller"; - if(class_exists($controllerClass)) break; - } - $controller = new $controllerClass($widget); + $controller = $widget->getController(); + $controller->init(); $controllers->push($controller); } @@ -47,7 +41,6 @@ class WidgetArea extends DataObject { } /** - * * @return HasManyList */ public function Items() { @@ -55,7 +48,6 @@ class WidgetArea extends DataObject { } /** - * * @return HasManyList */ public function ItemsToRender() { @@ -63,7 +55,6 @@ class WidgetArea extends DataObject { } /** - * * @return string - HTML */ public function forTemplate() { diff --git a/javascript/WidgetAreaEditor.js b/javascript/WidgetAreaEditor.js index a041cf1..840d5a2 100644 --- a/javascript/WidgetAreaEditor.js +++ b/javascript/WidgetAreaEditor.js @@ -105,7 +105,7 @@ var parentRef=$(this); $.ajax({ - 'url': 'Widget_Controller/EditableSegment/' + className, + 'url': 'WidgetController/EditableSegment/' + className, 'success' : function(response) {parentRef.insertWidgetEditor(response)} }); }, diff --git a/tests/WidgetControllerTest.php b/tests/WidgetControllerTest.php index d2d9644..d569791 100644 --- a/tests/WidgetControllerTest.php +++ b/tests/WidgetControllerTest.php @@ -1,9 +1,10 @@ objFromFixture('WidgetControllerTest_Widget', 'widget1'); - $this->get($page->URLSegment); + $response = $this->get($page->URLSegment); $response = $this->submitForm('Form_Form', null, array('TestValue'=>'Updated')); - + $this->assertContains( 'TestValue: Updated', $response->getBody(), @@ -50,7 +51,7 @@ class WidgetControllerTest extends FunctionalTest { } /** - * @package cms + * @package widgets * @subpackage tests */ class WidgetControllerTest_Widget extends Widget implements TestOnly { @@ -60,10 +61,15 @@ class WidgetControllerTest_Widget extends Widget implements TestOnly { } /** - * @package cms + * @package widgets * @subpackage tests */ -class WidgetControllerTest_Widget_Controller extends Widget_Controller implements TestOnly { +class WidgetControllerTest_WidgetController extends WidgetController implements TestOnly { + + private static $allowed_actions = array( + 'Form' + ); + function Form() { $widgetform = new Form( $this, From 0536ad31ce577e96290975ff07773378aa31c395 Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Tue, 7 May 2013 22:22:41 +1200 Subject: [PATCH 2/3] FIX: Ensure available widgets list is consistent between versions (#6292) --- code/form/WidgetAreaEditor.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/code/form/WidgetAreaEditor.php b/code/form/WidgetAreaEditor.php index 8977e87..0623f28 100644 --- a/code/form/WidgetAreaEditor.php +++ b/code/form/WidgetAreaEditor.php @@ -42,10 +42,13 @@ class WidgetAreaEditor extends FormField { foreach($this->widgetClasses as $widgetClass) { $classes = ClassInfo::subclassesFor($widgetClass); - if(count($classes) > 1) { - array_shift($classes); + if (isset($classes['Widget'])) { + unset($classes['Widget']); + } + else if (isset($classes[0]) && $classes[0] == 'Widget') { + unset($classes[0]); } - + foreach($classes as $class) { $widgets->push(singleton($class)); } From 67104f52447d7a8a8ab98e31ddf38c232f2d822f Mon Sep 17 00:00:00 2001 From: Will Rossiter Date: Tue, 7 May 2013 22:25:51 +1200 Subject: [PATCH 3/3] FIX: ensure cms treats editor as a field --- templates/WidgetAreaEditor.ss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/WidgetAreaEditor.ss b/templates/WidgetAreaEditor.ss index 4127e29..9fe4eeb 100644 --- a/templates/WidgetAreaEditor.ss +++ b/templates/WidgetAreaEditor.ss @@ -1,4 +1,4 @@ -
maxwidgets="$MaxWidgets"<% end_if %>> +
maxwidgets="$MaxWidgets"<% end_if %>>

<% _t('AVAILABLE', 'Available Widgets') %>