improved news action handling, restore the form fields if something went wrong when saving it to the database.

1 files changed, 53 insertions, 7 deletions

diff --git a/modules/news.inc.php b/modules/news.inc.php
index b5fad69b..c94331e0 100644
--- a/modules/news.inc.php
+++ b/modules/news.inc.php
@@ -2,11 +2,26 @@
 
 class Page_News extends Page
 {
+	/**
+	 * Member variables needed to represent a news entry.
+	 *
+	 * $newsId		int		ID of the news entry attributed by the database.
+	 * $newsTitle	string	Title of the entry.
+	 * $newsContent	string	Content as text. (TODO: html-Support?)
+	 * $newsDate	string	Unix epoch date of the news' creation.
+	 */
 	private $newsId = false;
 	private $newsTitle = false;
 	private $newsContent = false;
 	private $newsDate = false;
 	
+	/**
+	 * Implementation of the abstract doPreprocess function
+	 *
+	 * Checks if the user is logged in and processes any
+	 * action if one was specified in the request.
+	 *
+	 */
 	protected function doPreprocess()
 	{
 		// load user, we will need it later
@@ -22,6 +37,7 @@ class Page_News extends Page
 		$action = Request::any('action', 'show');
 		if ($action === 'clear') {
 			// clear news input fields
+			// TODO: is this the right way?
 			$this->newsId = false;
 			$this->newsTitle = false;
 			$this->newsContent = false;
@@ -33,19 +49,33 @@ class Page_News extends Page
 			}
 		} elseif ($action === 'save') {
 			// save to DB
-			$this->saveNews();
+			if (!$this->saveNews()) {
+				// re-set the fields we got
+				Request::post('news-title') ? $this->newsTitle = Request::post('news-title') : $this->newsTitle = false;
+				Request::post('news-content') ? $this->newsContent = Request::post('news-content') : $this->newsContent = false;
+			} else {
+				Message::addSuccess('news-set-success');
+				Util::redirect('?do=News');
+			}
 		} elseif ($action === 'delete') {
 			// delete it
 			$this->delNews(Request::post('newsid'));
 		} else {
+			// unknown action, redirect user
 			Message::addError('invalid-action', $action);
 			Util::redirect('?do=News');
 		}
 	}
 
+	/**
+	 * Implementation of the abstract doRender function
+	 *
+	 * Fetch the list of news from the database and paginate it.
+	 *
+	 */
 	protected function doRender()
 	{
-		// prepare the list of the older news
+		// fetch the list of the older news
 		$lines = array();
 		$paginate = new Paginate("SELECT newsid, dateline, title, content FROM news ORDER BY dateline DESC", 10);
 		$res = $paginate->exec();
@@ -55,7 +85,7 @@ class Page_News extends Page
 			if ($row['newsid'] == $this->newsId) $row['active'] = "active";
 			$lines[] = $row;
 		}
-		
+		// render each entry
 		$paginate->render('page-news', array(
 				'token' => Session::get('token'),
 				'latestDate' => ($this->newsDate ? date('d.m.Y H:i', $this->newsDate) : '--'),
@@ -65,7 +95,13 @@ class Page_News extends Page
 		));
 
 	}
-	
+	/**
+	 * Loads the news with the given ID into the form.
+	 *
+	 * @param int $newsId ID of the news to be shown.
+	 * @return boolean true if loading that news worked
+	 *
+	 */
 	private function loadNews($newsId)
 	{
 		// check to see if we need to request a specific newsid
@@ -87,6 +123,10 @@ class Page_News extends Page
 		return $row !== false;
 	}
 
+	/**
+	 * Save the given $newsTitle and $newsContent as POST'ed into the database.
+	 *
+	 */
 	private function saveNews()
 	{
 		// check if news content were set by the user
@@ -99,19 +139,25 @@ class Page_News extends Page
 				'title' => $newsTitle,
 				'content' => $newsContent
 			));
-			// all done, redirect to main news page
-			Message::addSuccess('news-set-success');
-			Util::redirect('?do=News');
+			return true;
 		} else {
 			Message::addError('empty-field');
+			return false;
 		}
 	}
 	
+	/**
+	 * Delete the news entry with ID $newsId
+	 *
+	 * @param int $newsId ID of the entry to be deleted.
+	 */
 	private function delNews($newsId)
 	{
+		// sanity check: is newsId even numeric?
 		if (!is_numeric($newsId)) {
 			Message::addError('value-invalid', 'newsid', $newsId);
 		} else {
+			// check passed - do delete
 			Database::exec("DELETE FROM news WHERE newsid = :newsid LIMIT 1", array(
 				'newsid' => $newsId
 			));