elofgren: USABILITY: Rework the tabs in the Newsletter section to be less confusing as per issue #57 of the Usability Report

Includes the removal of all tabs and submit action buttons when 'Drafts' or 'Sent Items' are clicked and the addition of an 'add one' link on 'Drafts' folder page. 
For more information see: http://www.elijahlofgren.com/silverstripe/newletter-tree-and-tab-row-are-confusing/ 
Includes a few fixes for undefined variables.  (merged from branches/gsoc)


git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/cms/trunk@41772 467b73ca-7a2a-4603-9d3b-597d59a354a9

code/Newsletter/Newsletter.php

@@ -14,7 +14,7 @@ class Newsletter extends DataObject {
 		
 		$ret = new FieldSet(
 			new TabSet("Root",
-				$mailTab = new Tab("Mail",
+				$mailTab = new Tab("Newsletter",
 					new TextField("Subject", "Subject", $this->Subject),
 					new HtmlEditorField("Content", "Content")
 				)

code/Newsletter/NewsletterList.php

@@ -20,10 +20,11 @@ class NewsletterList extends FormField {
 	function setController($controller) {
 		$this->controller = $controller;
 	}
-    
+    /* No longer used
     function Newsletters() {
         return DataObject::get( 'Newsletter', "`ParentID`='{$this->mailType->ID}' AND `Status`='{$this->status}'" );   
     }
+    */
     
 	function DraftNewsletters() {
 		return $this->mailType->DraftNewsletters();
@@ -36,22 +37,9 @@ class NewsletterList extends FormField {
 	function Status() {
 		return $this->status;
 	}
-	
+	function mailTypeID() {
-
+		return $this->mailType->ID;
-/*	function AddRecordForm() {
+	}
-		return new TabularStyle(new Form($this->controller,'AddRecordForm',
-			new FieldSet(
-				new TextField("FirstName", "First Name"),
-				new TextField("Surname", "Surname"),
-				new TextField("Email", "Email"),
-				new TextField("Password", "Password"),
-				new HiddenField("GroupID", null, $this->group->ID)
-			),
-			new FieldSet(
-				new FormAction("addmember", "Add")
-			)
-		));
-	}*/
 }
 
 ?>

code/NewsletterAdmin.php

@@ -87,15 +87,62 @@ class NewsletterAdmin extends LeftAndMain {
 		$result = $this->renderWith($this->class . "_right");
 		return $this->getLastFormIn($result);
 	}
-    
+
-    public function shownewsletter($params) {
+	/**
-	    return $this->showWithEditForm( $params, $this->getNewsletterEditForm( $params['ID'] ) );	
+	* Called when a mailing list is clicked on the left menu
+	*/
+	public function showrecipients($params) {
+		return $this->showWithEditForm( $params, $this->getMailingListEditForm( $params['ID'] ) );
+	}
+
+	/**
+	* Called when a draft or sent newsletter is clicked on the left menu
+	*/
+	public function shownewsletter($params) {
+		return $this->showWithEditForm( $params, $this->getNewsletterEditForm( $params['ID'] ) );	
+	}
+
+	/**
+	* Called when a newsletter type is clicked on the left menu
+	*/
+	public function showmailtype($params) {
+		return $this->showWithEditForm( $params, $this->getNewsletterTypeEditForm( $params['ID'] ) );	
+	}
+
+	/**
+	* Called when a 'Drafts' folder is clicked on the left menu
+	*/
+	public function showdrafts($params) {
+		return $this->ShowNewsletterFolder($params, 'Draft');
+	}
+
+	/**
+	* Called when a 'Sent Items' folder is clicked on the left menu
+	*/
+	public function showsent($params) {
+		return $this->ShowNewsletterFolder($params, 'Sent');					
 	}
 	
-    public function showtype($params) {
+	/**
-	    return $this->showWithEditForm( $params, $this->getNewsletterTypeEditForm( $params['ID'] ) );	
+	* Shows either the 'Sent' or 'Drafts' folder using the NewsletterList template
+	*/
+	public function ShowNewsletterFolder($params, $type) {
+		$id = $params['ID'];
+		if(!is_numeric($id)) {
+			$id = Session::get('currentPage');
+		}
+		if( is_a( $id, 'NewsletterType' ) ) {
+				$mailType = $id;
+				$id = $mailType->ID;
+		} else {
+			if($id && is_numeric($id)) {
+				$mailType = DataObject::get_by_id( 'NewsletterType', $id );
+			}
+		}
+		$draftList = new NewsletterList($type, $mailType, $type);
+		return $draftList->renderWith("NewsletterList");	
 	}
-    
+
     public function removenewsletter($params) {
         if( !is_numeric( $params['ID'] ) )
             return '';
@@ -131,7 +178,12 @@ class NewsletterAdmin extends LeftAndMain {
     	if((isset($_REQUEST['ID']) && isset($_REQUEST['Type']) && $_REQUEST['Type'] == 'Newsletter') || isset($_REQUEST['action_savenewsletter'])) {
     		return $this->NewsletterEditForm();
     	} else {
-    		return $this->TypeEditForm();
+		// If a mailing list member is being added to a group, then call the Recipient form
+		if (isset($_REQUEST['fieldName']) && 'Recipients' == $_REQUEST['fieldName']) {
+			return $this->MailingListEditForm();
+		} else {
+			return $this->TypeEditForm();
+		}
     	}
     }
     
@@ -150,6 +202,10 @@ class NewsletterAdmin extends LeftAndMain {
     	}
     	return $this->getNewsletterTypeEditForm($id);
     }
+    public function MailingListEditForm() {
+    	$id = isset($_REQUEST['ID']) ? $_REQUEST['ID'] : $this->currentPageID();
+    	return $this->getMailingListEditForm($id);
+    }
 	
 	public function getNewsletterTypeEditForm($id) {
         if(!is_numeric($id)) {
@@ -164,10 +220,22 @@ class NewsletterAdmin extends LeftAndMain {
 	    	}
 	    }
         
-		if(isset($mailType) && $mailType) {
+		if(isset($mailType) && is_object($mailType) && $mailType->GroupID) {
-            $fields = $mailType->getCMSFields();
+			$group = DataObject::get_one("Group", "ID = $mailType->GroupID");
+		} 
 
-			// $actions = new FieldSet(new FormAction('adddraft', 'Add Draft'));
+		if(isset($mailType)) {
+			$fields = new FieldSet(
+				new TextField("Title", "Newsletter Type"),
+				new TextField("FromEmail", "Send newsletters from"),
+				$templates = new TemplateList("Template","Template", $mailType->Template, self::template_path())
+			);
+			
+			$templates->setController($this);
+			
+			$fields->push($idField = new HiddenField("ID"));
+			$fields->push( new HiddenField( "executeForm", "", "TypeEditForm" ) );
+			$idField->setValue($id);
 			
 			$actions = new FieldSet(new FormAction('save','Save'));
 			
@@ -183,6 +251,71 @@ class NewsletterAdmin extends LeftAndMain {
 		return $form;
 	}
 	
+	public function getMailingListEditForm($id) {
+        if(!is_numeric($id)) {
+        		$id = Session::get('currentPage');
+		}
+	    if( is_a( $id, 'NewsletterType' ) ) {
+	    		$mailType = $id;
+	    		$id = $mailType->ID;
+	    } else {
+	    	if($id && is_numeric($id)) {
+	    		$mailType = DataObject::get_by_id( 'NewsletterType', $id );
+	    	}
+	    }
+        
+		if(isset($mailType) && is_object($mailType) && $mailType->GroupID) {
+			$group = DataObject::get_one("Group", "ID = $mailType->GroupID");
+		} 
+
+		if(isset($mailType) && is_object($mailType)) {
+			$fields = new FieldSet(
+				new TabSet("Root",
+					new Tab( "Recipients",
+						$recipients = new MemberTableField(
+							$this,
+							"Recipients", 
+							$group
+							)
+					),
+					new Tab( "Import",
+						$importField = new RecipientImportField("ImportFile","Import from file", $group )
+					),
+					new Tab("Unsubscribers",
+					$unsubscribedList = new UnsubscribedList("Unsubscribed", $mailType)    
+					),
+					new Tab("Bounced",
+					$bouncedList = new BouncedList("Bounced", $mailType )
+					)
+				)
+			);
+			
+			$recipients->setController($this);
+			$importField->setController($this);
+	      		$unsubscribedList->setController($this);
+			$bouncedList->setController($this);
+			
+			$importField->setTypeID( $id );
+			
+			$fields->push($idField = new HiddenField("ID"));
+			$fields->push( new HiddenField( "executeForm", "", "MailingListEditForm" ) );
+			$idField->setValue($id);
+	
+			$actions = new FieldSet(new FormAction('save','Save'));
+			
+			$form = new Form($this, "MailingListEditForm", $fields, $actions);
+			$form->loadDataFrom(array(
+				'Title' => $mailType->Title,
+				'FromEmail' => $mailType->FromEmail
+			));
+		} else {
+			$form = false;
+		}
+		
+		return $form;
+
+	}
+	
 	/**
 	 * Reloads the list of recipients via ajax
 	 */
@@ -199,10 +332,12 @@ class NewsletterAdmin extends LeftAndMain {
 		else return self::$template_path = project() . '/templates/email';
 	}
     
-    public function showdraft( $params ) {
+	/* Does not seem to be used 
-        return $this->showWithEditForm( $params, $this->getNewsletterEditForm( $params['ID'] ) );   
+	public function showdraft( $params ) {
-    }
+        	return $this->showWithEditForm( $params, $this->getNewsletterEditForm( $params['ID'] ) );   
-	
+	}
+	*/
+
 	public function getNewsletterEditForm($myId){
 	
 		$email = DataObject::get_by_id("Newsletter", $myId);
@@ -246,7 +381,7 @@ class NewsletterAdmin extends LeftAndMain {
 	
 	public function sendnewsletter( /*$data, $form = null*/ ) {
 		
-		$id = $_REQUEST['ID'] ? $_REQUEST['ID'] : $_REQUEST['NewsletterID'];
+		$id = isset($_REQUEST['ID']) ? $_REQUEST['ID'] : $_REQUEST['NewsletterID'];
 		
 		if( !$id ) {
 			FormResponse::status_message('No newsletter specified','bad');
@@ -307,6 +442,9 @@ class NewsletterAdmin extends LeftAndMain {
 		if( isset($_REQUEST['Type']) && $_REQUEST['Type'] == 'Newsletter' )
 			return $this->savenewsletter( $urlParams, $form );
 
+		// @TODO: Find the real fix for this.
+		$className = '';
+
 		$id = $_REQUEST['ID'];
 		$record = DataObject::get_one('NewsletterType', "`NewsletterType`.ID = $id");
 		
@@ -325,7 +463,9 @@ class NewsletterAdmin extends LeftAndMain {
 	
 	public function savenewsletter($urlParams, $form) {
 		$id = $_REQUEST['ID'];
-		$record = DataObject::get_one('Newsletter', "`Newsletter`.ID = $id");
+		// @TODO: Find the real fix for this.
+		$className = '';
+		$record = DataObject::get_one('Newsletter', "`$className`.ID = $id");
 		
 		// Is the template attached to the type, or the newsletter itself?
 		$type = $record->getNewsletterType();

javascript/NewsletterAdmin_left.js

@@ -206,22 +206,7 @@ SiteTreeNode.prototype.getPageFromServer = function() {
     var otherID = null;
     var type = null;
     
-    var forceReload = $('Form_EditForm_Type') && $('Form_EditForm_Type').value == 'Newsletter';
-        
 		if( match ) {
-            
-      // open the appropriate tab
-      switch( match[1] ) {
-          case 'recipients':
-              openTabName = 'Root_Recipients_set_Recipients';
-              break;
-          case 'drafts':
-              openTabName = 'Root_Drafts';
-              break;
-          case 'sent':
-              openTabName = 'Root_Sent_set_Sent';
-              break;    
-      }
       
       newPageID = match[2];
       type = match[1];
@@ -229,13 +214,8 @@ SiteTreeNode.prototype.getPageFromServer = function() {
 			newPageID = RegExp.$2;
 			type = RegExp.$1;
 			otherID = RegExp.$3;
-			forceReload = true;
 		}
-		
+		$('Form_EditForm').getPageFromServer(newPageID, type, otherID, openTabName);
-		if(	forceReload || currentPageID != newPageID )
-			$('Form_EditForm').getPageFromServer(newPageID, type, otherID, openTabName);
-		else
-			openTab( openTabName );
 };
 
 function draft_sent_ok( newsletterID, draftID ) {

javascript/NewsletterAdmin_right.js

@@ -33,11 +33,11 @@ Behaviour.register({
 				if(treeNode) treeNode.addNodeClass('loading');
 				statusMessage("loading...");
                 
-                var requestURL = 'admin/newsletter/showtype/' + id;
+                		var requestURL = 'admin/newsletter/show' + type + '/' + id;
                 
-                if( otherid )
+                		if( otherid ) {
-                    requestURL = 'admin/newsletter/shownewsletter/' + otherid;
+					requestURL = 'admin/newsletter/shownewsletter/' + otherid;
-		
+				}
 				new Ajax.Request(requestURL, {
 					asynchronous : true,
 					method : 'post', 
@@ -47,6 +47,13 @@ Behaviour.register({
 						errorMessage('error loading page',response);
 					}
 				});
+				// Hide the action buttons if 'Drafts' or 'Sent Items' is clicked on
+				if ('drafts' == type || 'sent' == type)
+				{
+					Element.hide('form_actions');
+					Element.hide('form_actions_right');
+				}
+
 			} else {
 				throw("getPageFromServer: Bad page ID: " + id);
 			}
@@ -534,3 +541,27 @@ RecipientImportField.prototype = {
  		}
  	}
  });
+ 
+/**
+ * Handle 'add one' link action. Adds a new draft to the site tree and loads it up to edit.
+ * Adapted from NewsletterAdmin_left.js
+ */
+function addNewDraft(parentID) {
+	var type = 'draft';
+	var request = new Ajax.Request( 'admin/newsletter/addtype?ajax=1&PageType=' + type + '&ParentID=' + parentID, {
+		method: 'get',
+		asynchronous: true,
+		onSuccess : function( response ) {
+			$('Form_EditForm').loadNewPage(response.responseText);
+				
+			// create a new node and add it to the site tree
+			$('sitetree').addDraftNode('New draft newsletter', parentID, $('Form_EditForm_ID').value );
+			
+			statusMessage('Added new ' + type);
+		},
+		onFailure : function(response) {
+			alert(response.responseText);
+			statusMessage('Could not add new ' + type );
+		}
+	});
+}

templates/NewsletterList.ss

@@ -1,19 +1,6 @@
-<table id="{$Status}List" class="CMSList NewsletterList">
+<% if Status = Draft %>
-	<thead>
+	<p>Please choose a draft on the left, or <a href="javascript:addNewDraft({$mailTypeID})">add one</a>.</p>
-		<tr>
+<% end_if %>
-			<td class="Email">Subject</td>
+<% if Status = Sent %>
-			<td class="Surname">Content</td>
+	<p>Please choose a sent item on the left.</p>
-		</tr>
+<% end_if %>
-	</thead>
-	
-	<tbody>
-		<% control Newsletters %>
-		<tr id="letter-$ID">
-			<td>$Subject</td>
-			<td>$Content</td>
-		</tr>
-		<% end_control %>
-		
-		<!-- $AddRecordForm.AsTableRow -->
-	</tbody>
-</table>