[3873] trunk: Get setslug() out of the Tag class and into the Term class where it belongs.

0 views
Skip to first unread message

svn-N...@habariproject.org

unread,
Dec 9, 2009, 9:32:12 AM12/9/09
to habar...@googlegroups.com
Revision
3873
Author
rickc
Date
2009-12-09 14:32:11 +0000 (Wed, 09 Dec 2009)

Log Message

Get setslug() out of the Tag class and into the Term class where it belongs. The code was stolen from the Post class. 

Makes Term::term always be unique. Improve Term::get() to allow terms to be numeric and not assume a term is just a slugified version of the term display text as returned by Utils::slugify(). This should fix issues reported on irc.

Have Vocabulary::add_term() return false on failure.

Update the tag tests class to the current code. All tests pass. test writing is a unique skill unto itself, I'm learning.

Cross my fingers this doesn't break any updates people have done to their plugins.

Modified Paths

Diff

Modified: trunk/htdocs/system/classes/tag.php (3872 => 3873)


--- trunk/htdocs/system/classes/tag.php	2009-12-09 05:36:34 UTC (rev 3872)
+++ trunk/htdocs/system/classes/tag.php	2009-12-09 14:32:11 UTC (rev 3873)
@@ -136,19 +136,6 @@
 	}
 
 	/**
-	 * Generate a new slug for the tag.
-	 *
-	 * @return string The slug
-	 */
-	private function setslug()
-	{
-		// make sure our slug is unique
-		$this->tag_slug = Plugins::filter( 'tag_setslug', $value );
-		$this->tag_slug = Utils::slugify( $slug );
-		return $this->tag_slug;
-	}
-
-	/**
 	 * function insert
 	 * Saves a new tag's data into the terms table
 	 */
@@ -164,10 +151,15 @@
 		$term = new Term( array( 'term' => $this->tag_slug, 'term_display' => $this->tag_text ) );
 		$term = Tags::vocabulary()->add_term( $term );
 
-		EventLog::log( sprintf(_t('New tag %1$s (%2$s);  Slug: %3$s'), $this->id, $this->tag_text, $this->tag_slug), 'info', 'content', 'habari' );
-		Plugins::act( 'tag_insert_after', $this );
+		if ( $term ) {
+			EventLog::log( sprintf(_t('New tag %1$s (%2$s);  Slug: %3$s'), $this->id, $this->tag_text, $this->tag_slug), 'info', 'content', 'habari' );
+			Plugins::act( 'tag_insert_after', $this );
+			return new Tag( array( 'tag_text' => $term->term_display, 'tag_slug' => $term->term, 'id' => $term->id ) );
+		}
+		else {
+			return FALSE;
+		}
 
-		return new Tag( array( 'tag_text' => $term->term_display, 'tag_slug' => $term->term, 'id' => $term->id ) );
 	}
 
 	/**
@@ -183,16 +175,17 @@
 		}
 		Plugins::act( 'tag_update_before', $this );
 
-		// Call setslug() only when tag slug is changed
-		if ( isset( $this->tag_slug ) && $this->tag_slug != '' ) {
-			$this->setslug();
-		}
-
 		$term = Tags::vocabulary()->get_term( $this->id );
 		$term->term = $this->tag_slug;
 		$term->term_display = $this->tag_text;
 		$result = $term->update();
 
+		$term = Tags::vocabulary()->get_term( $this->id );
+		if( $result ) {
+			$this->tag_text = $term->term_display;
+			$this->tag_slug = $term->term;
+		}
+
 		Plugins::act( 'tag_update_after', $this );
 		return $result;
 	}

Modified: trunk/htdocs/system/classes/term.php (3872 => 3873)


--- trunk/htdocs/system/classes/term.php	2009-12-09 05:36:34 UTC (rev 3872)
+++ trunk/htdocs/system/classes/term.php	2009-12-09 14:32:11 UTC (rev 3873)
@@ -83,24 +83,65 @@
 			$query = 'SELECT * FROM {terms} WHERE vocabulary_id = ? AND mptt_left = ?';
 		}
 		else {
-			$term = Utils::slugify( $term );
 			$params[] = $term;
-			if ( is_numeric( $term ) ) {
-				$query = 'SELECT * FROM {terms} WHERE vocabulary_id = ? AND id = ?';
-			}
-			else {
-				$query = 'SELECT * FROM {terms} WHERE vocabulary_id = ? AND term = ?';
-			}
+			$params[] = $term;
+			$params[] = $term;
+			$query = 'SELECT * FROM {terms} WHERE vocabulary_id = ? AND (id = ? OR term = ? OR term_display = ?)';
 		}
 		return DB::get_row( $query, $params, 'Term' );
 	}
 
 	/**
+	 * Generate a new slug for the post.
+	 *
+	 * @return string The slug
+	 */
+	protected function setslug()
+	{
+		// determine the base value from:
+		// - the new slug
+		if ( isset( $this->newfields['term']) && $this->newfields['term'] != '' ) {
+			$value = $this->newfields['term'];
+		}
+		// - the existing slug
+		elseif ( $this->fields['term'] != '' ) {
+			$value = $this->fields['term'];
+		}
+		// - the new term display text
+		elseif ( isset( $this->newfields['term_display'] ) && $this->newfields['term_display'] != '' ) {
+			$value = $this->newfields['term_display'];
+		}
+		// - the existing term display text
+		elseif ( $this->fields['term_display'] != '' ) {
+			$value = $this->fields['term_display'];
+		}
+
+		// make sure our slug is unique
+		$slug = Plugins::filter( 'term_setslug', $value );
+		$slug = Utils::slugify( $slug );
+		$postfix = '';
+		$postfixcount = 0;
+		do {
+			if ( ! $slugcount = DB::get_row( 'SELECT COUNT(term) AS ct FROM {terms} WHERE term = ?;', array( $slug . $postfix ) ) ) {
+				Utils::debug( DB::get_errors() );
+				exit;
+			}
+			if ( $slugcount->ct != 0 ) {
+				$postfix = "-" . ( ++$postfixcount );
+			}
+		} while ( $slugcount->ct != 0 );
+
+		return $this->newfields['term'] = $slug . $postfix;
+	}
+
+	/**
 	 * function insert
 	 * Saves a new term to the terms table
 	 */
 	public function insert()
 	{
+		$this->setslug();
+
 		// Let plugins disallow and act before we write to the database
 		$allow = true;
 		$allow = Plugins::filter( 'term_insert_allow', $allow, $this );
@@ -142,6 +183,13 @@
 		}
 		Plugins::act( 'term_update_before', $this );
 
+		// Call setslug() only when term is changed
+		if ( isset( $this->newfields['term'] ) && $this->newfields['term'] != '' ) {
+			if ( $this->fields['term'] != $this->newfields['term'] ) {
+				$this->setslug();
+			}
+		}
+
 		$result = parent::updateRecord( '{terms}', array( 'id' => $this->id ) );
 
 		// Let plugins act after we write to the database

Modified: trunk/htdocs/system/classes/vocabulary.php (3872 => 3873)


--- trunk/htdocs/system/classes/vocabulary.php	2009-12-09 05:36:34 UTC (rev 3872)
+++ trunk/htdocs/system/classes/vocabulary.php	2009-12-09 14:32:11 UTC (rev 3873)
@@ -327,9 +327,14 @@
 		$new_term->mptt_right = $ref + 2;
 
 		// Insert the new node
-		$new_term->insert();
+		$result = $new_term->insert();
+		if ( $result ) {
+			return $new_term;
+		}
+		else {
+			return FALSE;
+		}
 
-		return $new_term;
 	}
 
 	/**

Modified: trunk/tests/system/classes/tagTest.php (3872 => 3873)


--- trunk/tests/system/classes/tagTest.php	2009-12-09 05:36:34 UTC (rev 3872)
+++ trunk/tests/system/classes/tagTest.php	2009-12-09 14:32:11 UTC (rev 3873)
@@ -6,6 +6,7 @@
 {
 	private $text = 'Test Tag';
 	private $slug;
+	private $tag;
 
 	public function setup()
 	{
@@ -47,12 +48,17 @@
 	{
 		$count = count( Tags::get() );
 		$res = $this->tag->insert();
-		$this->assertEquals( $res, TRUE );
-		$this->assertEquals( $count + 1, count( Tags::get() ) );
-		$t = Tag::get( $this->slug );
-		$this->assertType( 'Tag', $t );
-		$this->assertEquals( $t->tag, $this->text );
-		$this->tag->delete();
+		if ( $res ) {
+			$this->assertType( 'Tag', $res );
+			$this->assertEquals( $count + 1, count( Tags::get() ) );
+			$t = Tag::get( $this->text );
+			$this->assertType( 'Tag', $t );
+			$this->assertEquals( $t->tag_text, $this->text );
+			$t->delete();
+		}
+		else {
+			$this->assertEquals( $res, FALSE );
+		}
 
 	}
 
@@ -71,9 +77,9 @@
 	public function test_delete_tag()
 	{
 		$count = count( Tags::get() );
-		$this->tag->insert();
+		$t = $this->tag->insert();
 		$this->assertEquals( $count + 1, count( Tags::get() ) );
-		$this->tag->delete();
+		$t->delete();
 		$this->assertEquals( $count, count( Tags::get() ) );
 	}
 
@@ -81,8 +87,8 @@
 	{
 		$this->tag->insert();
 		$t = Tag::get( $this->text );
-		$this->assertEquals( $t->tag, $this->tag->tag_text );
-		$this->tag->delete();
+		$this->assertEquals( $t->tag_text, $this->tag->tag_text );
+		$t->delete();
 	}
 
 	public function test_attach_to_post_tag()
Reply all
Reply to author
Forward
0 new messages