In-Portal Issue Tracker

Welcome to the In-Portal Open Source CMS Issue Tracker! This is a central management / tracking tool for all types of tasks / issues / bugs for the In-Portal Project. Before reporting any issues, please make sure to read the Guide into Issue Tracker and How to Properly Test and Report Bugs!

Viewing Issue Simple Details [ Jump to Notes ] [ Wiki ]

[ View Advanced ] [ Issue History ] [ Print ]

Tags

No tags attached.

Reference

Change Log Message

Estimate Points

Attached Files

FilenameFix.patch [^] (3,508 bytes) 2011-06-27 15:29 [Show Content]

Index: core/kernel/db/cat_dbitem.php
===================================================================
--- core/kernel/db/cat_dbitem.php	(revision 14393)
+++ core/kernel/db/cat_dbitem.php	(working copy)
@@ -59,10 +59,10 @@
 	function Create($force_id = false, $system_create = false)
 	{
 		// TODO: move to OnBeforeItemCreate method
-		if ($this->useFilenames) {
+		/*if ($this->useFilenames) {
 			$this->checkFilename();
 			$this->generateFilename();
-		}
+		}*/
 
 		$ret = parent::Create($force_id, $system_create);
 
@@ -104,7 +104,7 @@
 		$ret = parent::Update($id, $system_update);
 
 		if ($ret) {
-			$filename = $this->useFilenames ? $this->GetDBField('Filename') : '';
+			$filename = $this->useFilenames ? (string)$this->GetDBField('Filename') : '';
 			$sql = 'UPDATE '.$this->CategoryItemsTable().'
 					SET Filename = '.$this->Conn->qstr($filename).'
 					WHERE ItemResourceId = '.$this->GetDBField('ResourceId');
@@ -403,7 +403,7 @@
 				'ItemResourceId'	=>	$this->GetField('ResourceId'),
 				'PrimaryCat'		=>	$is_primary ? 1 : 0,
 				'ItemPrefix'		=>	$this->Prefix,
-				'Filename'			=>	$this->useFilenames ? $this->GetDBField('Filename') : '', // because some prefixes does not use filenames,
+				'Filename'			=>	$this->useFilenames ? (string)$this->GetDBField('Filename') : '', // because some prefixes does not use filenames,
 			);
 			$this->Conn->doInsert($fields_hash, $table);
 		}
Index: core/units/categories/categories_item.php
===================================================================
--- core/units/categories/categories_item.php	(revision 14393)
+++ core/units/categories/categories_item.php	(working copy)
@@ -18,6 +18,9 @@
 	{
 		function Create($force_id = false, $system_create = false)
 		{
+			$parent_category = $this->GetDBField('ParentId') > 0 ? $this->GetDBField('ParentId') : $this->Application->GetVar('m_cat_id');
+			$this->SetDBField('ParentId', $parent_category);
+
 			$this->checkFilename();
 			$this->generateFilename();
 
@@ -33,9 +36,6 @@
 				$this->SetDBField('CreatedById', $this->Application->RecallVar('user_id'));
 			}
 
-			$parent_category = $this->GetDBField('ParentId') > 0 ? $this->GetDBField('ParentId') : $this->Application->GetVar('m_cat_id');
-			$this->SetDBField('ParentId', $parent_category);
-
 			$ret = parent::Create($force_id, $system_create);
 
 			if ($ret) {
@@ -132,7 +132,7 @@
 			}
 
 			$unique_clause = '(Filename = %s) AND (ThemeId = ' . $item_theme . ' OR ThemeId = 0)';
-			$check_in_parent_cat_only = $item_id ? ' AND ParentId = ' . $this->GetDBField('ParentId') : '';
+			$check_in_parent_cat_only = ' AND ParentId = ' . $this->GetDBField('ParentId');
 
 			// check temp table
 			$sql_temp = '	SELECT ' . $this->IDField . '
Index: core/units/helpers/filenames_helper.php
===================================================================
--- core/units/helpers/filenames_helper.php	(revision 14393)
+++ core/units/helpers/filenames_helper.php	(working copy)
@@ -95,7 +95,7 @@
 
 		$item_id = !$item_id ? 0 : $item_id;
 
-		if ($table == TABLE_PREFIX.'CategoryItems') {
+		if ( $this->Application->GetLiveName($table) == TABLE_PREFIX.'CategoryItems' ) {
 			$item_categories_cur = $this->Conn->GetCol('SELECT CategoryId FROM '.$table.' WHERE ItemResourceId = '.$item_id);
 			$item_categories_live = $this->Application->IsTempTable($table) ? $this->Conn->GetCol('SELECT CategoryId FROM '.$this->Application->GetLiveName($table).' WHERE ItemResourceId = '.$item_id) : array();

filename_generation_v2.patch [^] (9,358 bytes) 2011-06-28 08:42 [Show Content]

Index: kernel/db/cat_dbitem.php
===================================================================
--- kernel/db/cat_dbitem.php	(revision 14318)
+++ kernel/db/cat_dbitem.php	(working copy)
@@ -58,18 +58,13 @@
 
 	function Create($force_id = false, $system_create = false)
 	{
-		// TODO: move to OnBeforeItemCreate method
-		if ($this->useFilenames) {
-			$this->checkFilename();
-			$this->generateFilename();
-		}
-
 		$ret = parent::Create($force_id, $system_create);
 
 		if ($ret) {
 			// TODO: move to OnAfterItemCreate method
 			$this->assignPrimaryCategory();
 		}
+
 		return $ret;
 	}
 
@@ -104,7 +99,7 @@
 		$ret = parent::Update($id, $system_update);
 
 		if ($ret) {
-			$filename = $this->useFilenames ? $this->GetDBField('Filename') : '';
+			$filename = $this->useFilenames ? (string)$this->GetDBField('Filename') : '';
 			$sql = 'UPDATE '.$this->CategoryItemsTable().'
 					SET Filename = '.$this->Conn->qstr($filename).'
 					WHERE ItemResourceId = '.$this->GetDBField('ResourceId');
@@ -403,7 +398,7 @@
 				'ItemResourceId'	=>	$this->GetField('ResourceId'),
 				'PrimaryCat'		=>	$is_primary ? 1 : 0,
 				'ItemPrefix'		=>	$this->Prefix,
-				'Filename'			=>	$this->useFilenames ? $this->GetDBField('Filename') : '', // because some prefixes does not use filenames,
+				'Filename'			=>	$this->useFilenames ? (string)$this->GetDBField('Filename') : '', // because some prefixes does not use filenames,
 			);
 			$this->Conn->doInsert($fields_hash, $table);
 		}
Index: units/categories/categories_item.php
===================================================================
--- units/categories/categories_item.php	(revision 14318)
+++ units/categories/categories_item.php	(working copy)
@@ -18,6 +18,10 @@
 	{
 		function Create($force_id = false, $system_create = false)
 		{
+			// set parent category first, so filename generation could use it
+			$parent_category = $this->GetDBField('ParentId') > 0 ? $this->GetDBField('ParentId') : $this->Application->GetVar('m_cat_id');
+			$this->SetDBField('ParentId', $parent_category);
+
 			$this->checkFilename();
 			$this->generateFilename();
 
@@ -33,9 +37,6 @@
 				$this->SetDBField('CreatedById', $this->Application->RecallVar('user_id'));
 			}
 
-			$parent_category = $this->GetDBField('ParentId') > 0 ? $this->GetDBField('ParentId') : $this->Application->GetVar('m_cat_id');
-			$this->SetDBField('ParentId', $parent_category);
-
 			$ret = parent::Create($force_id, $system_create);
 
 			if ($ret) {
@@ -131,26 +132,25 @@
 				$item_theme = $current_theme;
 			}
 
-			$unique_clause = '(Filename = %s) AND (ThemeId = ' . $item_theme . ' OR ThemeId = 0)';
-			$check_in_parent_cat_only = $item_id ? ' AND ParentId = ' . $this->GetDBField('ParentId') : '';
+			$unique_clause = '(Filename = %s) AND (ThemeId = ' . $item_theme . ' OR ThemeId = 0) AND (ParentId = ' . $this->GetDBField('ParentId') . ')';
 
+			$sql_mask = '	SELECT ' . $this->IDField . '
+							FROM %s
+							WHERE ' . sprintf($unique_clause, $this->Conn->qstr($filename));
+
 			// check temp table
-			$sql_temp = '	SELECT ' . $this->IDField . '
-							FROM ' . $this->TableName . '
-							WHERE ' . sprintf($unique_clause, $this->Conn->qstr($filename)) . $check_in_parent_cat_only;
+			$sql_temp = sprintf($sql_mask, $this->TableName);
 			$found_temp_ids = $this->Conn->GetCol($sql_temp);
 
 			// check live table
-			$sql_live = '	SELECT ' . $this->IDField . '
-							FROM ' . $this->Application->GetLiveName($this->TableName) . '
-							WHERE '  . sprintf($unique_clause, $this->Conn->qstr($filename)) . $check_in_parent_cat_only;
+			$sql_live = sprintf($sql_mask, $this->Application->GetLiveName($this->TableName));
 			$found_live_ids = $this->Conn->GetCol($sql_live);
 
 			$found_item_ids = array_unique( array_merge($found_temp_ids, $found_live_ids) );
 
 			$has_page = preg_match('/(.*)_([\d]+)([a-z]*)$/', $filename, $rets);
+			$duplicates_found = (count($found_item_ids) > 1) || ($found_item_ids && $found_item_ids[0] != $item_id);
 
-			$duplicates_found = (count($found_item_ids) > 1) || ($found_item_ids && $found_item_ids[0] != $item_id);
 			if ($duplicates_found || $has_page) {// other category has same filename as ours OR we have filename, that ends with _number
 				$append = $duplicates_found ? $escape_char . 'a' : '';
 				if ($has_page) {
@@ -162,6 +162,7 @@
 				$sql_temp = '	SELECT ' . $this->IDField . '
 								FROM ' . $this->TableName . '
 								WHERE ' . $unique_clause . ' AND (' . $this->IDField . ' != ' . $item_id . ')';
+
 				$sql_live = '	SELECT ' . $this->IDField . '
 								FROM ' . $this->Application->GetLiveName($this->TableName) . '
 								WHERE ' . $unique_clause . ' AND (' . $this->IDField . ' != ' . $item_id . ')';
Index: units/helpers/filenames_helper.php
===================================================================
--- units/helpers/filenames_helper.php	(revision 14339)
+++ units/helpers/filenames_helper.php	(working copy)
@@ -91,35 +91,50 @@
 
 	function checkAutoFilename($table, $id_field, $item_id, $filename)
 	{
-		if(!$filename) return $filename;
+		if (!$filename) {
+			return $filename;
+		}
 
 		$item_id = !$item_id ? 0 : $item_id;
 
-		if ($table == TABLE_PREFIX.'CategoryItems') {
-			$item_categories_cur = $this->Conn->GetCol('SELECT CategoryId FROM '.$table.' WHERE ItemResourceId = '.$item_id);
-			$item_categories_live = $this->Application->IsTempTable($table) ? $this->Conn->GetCol('SELECT CategoryId FROM '.$this->Application->GetLiveName($table).' WHERE ItemResourceId = '.$item_id) : array();
+		if ( $this->Application->GetLiveName($table) == TABLE_PREFIX.'CategoryItems' ) {
+			$sql_mask = '	SELECT CategoryId
+							FROM %1$s
+							WHERE ItemResourceId = %2$s';
 
+			$item_categories_cur = $this->Conn->GetCol( sprintf($sql_mask, $table, $item_id) );
+
+			if ( $this->Application->IsTempTable($table) ) {
+				$item_categories_live = $this->Conn->GetCol( sprintf($sql_mask, $this->Application->GetLiveName($table), $item_id) );
+			}
+			else {
+				$item_categories_live = Array ();
+			}
+
 			$item_categories = array_unique(array_merge($item_categories_cur, $item_categories_live));
+
 			if (!$item_categories) {
-				$item_categories = array($this->Application->GetVar('m_cat_id')); // this may happen when creating new item
+				$item_categories = Array( $this->Application->GetVar('m_cat_id') ); // this may happen when creating new item
 			}
-			$cat_filter = ' AND CategoryId IN ('.implode(',', $item_categories).')';
+
+			$cat_filter = ' AND CategoryId IN (' . implode(',', $item_categories) . ')';
 		}
 		else {
 			$cat_filter = '';
 		}
 
 		// check current table (temp or live)
-		$sql_temp = 'SELECT '.$id_field.' FROM '.$table.' WHERE Filename = '.$this->Conn->qstr($filename).$cat_filter;
-		$found_temp_ids = $this->Conn->GetCol($sql_temp);
+		$sql_mask = '	SELECT ' . $id_field . '
+						FROM %1$s
+						WHERE Filename = ' . $this->Conn->qstr($filename) . $cat_filter;
+		$found_temp_ids = $this->Conn->GetCol( sprintf($sql_mask, $table) );
 
 		// check live table if current is temp
 		if ( $this->Application->IsTempTable($table) ) {
-			$sql_live = 'SELECT '.$id_field.' FROM '.$this->Application->GetLiveName($table).' WHERE Filename = '.$this->Conn->qstr($filename).$cat_filter;
-			$found_live_ids = $this->Conn->GetCol($sql_live);
+			$found_live_ids = $this->Conn->GetCol( sprintf($sql_mask, $this->Application->GetLiveName($table)) );
 		}
 		else {
-			$found_live_ids = array();
+			$found_live_ids = Array ();
 		}
 
 		$found_item_ids = array_unique( array_merge($found_temp_ids, $found_live_ids) );
@@ -127,18 +142,28 @@
 		$has_page = preg_match('/(.*)_([\d]+)([a-z]*)$/', $filename, $rets);
 
 		$duplicates_found = (count($found_item_ids) > 1) || ($found_item_ids && $found_item_ids[0] != $item_id);
-		if ($duplicates_found || $has_page) // other category has same filename as ours OR we have filename, that ends with _number
-		{
-			$append = $duplicates_found ? '_a' : '';
-			if($has_page)
-			{
+		if ($duplicates_found || $has_page) {// other category has same filename as ours OR we have filename, that ends with _number
+			$append = $duplicates_found ? $this->_escapeChar . 'a' : '';
+
+			if ($has_page) {
 				$filename = $rets[1].'_'.$rets[2];
-				$append = $rets[3] ? $rets[3] : '_a';
+				$append = $rets[3] ? $rets[3] : $this->_escapeChar . 'a';
 			}
 
 			// check live & temp table
-			$sql_cur = 'SELECT '.$id_field.' FROM '.$table.' WHERE (Filename = %s) AND ('.$id_field.' != '.$item_id.')'.$cat_filter;
-			$sql_live = $this->Application->IsTempTable($table) ? 'SELECT '.$id_field.' FROM '.$this->Application->GetLiveName($table).' WHERE (Filename = %s) AND ('.$id_field.' != '.$item_id.')'.$cat_filter : false;
+			$sql_cur = 'SELECT ' . $id_field . '
+						FROM ' . $table . '
+						WHERE (Filename = %s) AND (' . $id_field . ' != ' . $item_id . ')' . $cat_filter;
+
+			if ( $this->Application->IsTempTable($table) ) {
+				$sql_live = '	SELECT ' . $id_field . '
+								FROM ' . $this->Application->GetLiveName($table) . '
+								WHERE (Filename = %s) AND (' . $id_field . ' != ' . $item_id . ')' . $cat_filter;
+			}
+			else {
+				$sql_live = false;
+			}
+
 			while ( $this->Conn->GetOne( sprintf($sql_cur, $this->Conn->qstr($filename.$append)) ) > 0 ||
 							(	$sql_live
 								&&

Relationships [ Relation Graph ] [ Dependency Graph ]

Notes
(0003554) Dmitry (manager) 2011-06-26 23:25 edited on: 2011-06-27 02:18	Reminder sent to: alex I have reviewed the current functionality and believe there is a bug in how it works now. Below, I'll describe how it should properly work. Please implement the changes and attach the patch: 1. Each CategoryItem MUST have UNIQUE Filename within one Category. 2. Each CategoryItem can be placed into multiple Categories so we should check for Filename uniqueness in all of it's Categories and not just Primary. 3. The general rule of thumb we can't have the SAME Filenames inside the SAME Category. As you can see COPY functionality seems to be working correctly while ADD/EDIT is not. Please adjust accordingly.

(0003561) alex (manager) 2011-06-27 02:54 edited on: 2011-06-27 02:55	Strange things happening in item creation process: 1. no filename at start 2. before item is assigned to a category check is made for unique filename 3. since no assigned category check is made across all database and wrong filename is generated 4. item is assigned to a category 5. check is made, that filename is unique in this category 6. all passes Problem happens, when item creation in temp table happens in same time, when it's copied to live table. For example, when you go to "Sections" tab on category item editing, then it's probably won't be a problem.

(0003568) Dmitry (manager) 2011-06-27 15:45	Please find attached patch with fixes for Category Items. TODO: 1. Make sure it works for Categories and ending is unique on one level. With new patch it uses Ending letter from across the DB. 2. Make sure separator for CategoryItem filenames is used from Configuration settings

(0003569) alex (manager) 2011-06-28 08:43	Tested by developer, since too close to release.

(0003570) alex (manager) 2011-06-28 08:46	Fix committed to 5.1.x branch. Commit Message: Fixes 0000385: Copy/paste doesn't make category item filename unique

(0003823) alex (manager) 2011-09-19 16:08	Closing, since 5.1.3 release has been released.

Related Changesets
In-Portal CMS: 5.1.x r14394 Timestamp: 2011-06-28 08:46:34 Author: alex [ Details ] [ Diff ]	Fixes 0000385: Copy/paste doesn't make category item filename unique
	mod - /in-portal/branches/5.1.x/core/kernel/db/cat_dbitem.php	[ Diff ] [ File ]
	mod - /in-portal/branches/5.1.x/core/units/categories/categories_item.php	[ Diff ] [ File ]
	mod - /in-portal/branches/5.1.x/core/units/helpers/filenames_helper.php	[ Diff ] [ File ]

Issue History
Date Modified	Username	Field	Change
2011-09-19 16:08	alex	Note Added: 0003823
2011-09-19 16:08	alex	Status	resolved => closed
2011-06-28 08:46	alex	Note Added: 0003570
2011-06-28 08:46	alex	Status	reviewed and tested => resolved
2011-06-28 08:46	alex	Fixed in Version	=> 5.1.3-B2
2011-06-28 08:46	alex	Resolution	open => fixed
2011-06-28 08:46	alex	Assigned To	!COMMUNITY => alex
2011-06-28 08:46	alex	Changeset attached	5.1.x r14394
2011-06-28 08:43	alex	Note Added: 0003569
2011-06-28 08:43	alex	Status	needs testing => reviewed and tested
2011-06-28 08:43	alex	Assigned To	alex => !COMMUNITY
2011-06-28 08:43	alex	Developer	Dmitry => alex
2011-06-28 08:43	alex	Status	needs work => needs testing
2011-06-28 08:42	alex	File Added: filename_generation_v2.patch
2011-06-27 15:45	Dmitry	Note Added: 0003568
2011-06-27 15:29	Dmitry	File Added: FilenameFix.patch
2011-06-27 02:55	alex	Note Edited: 0003561	View Revisions
2011-06-27 02:54	alex	Note Added: 0003561
2011-06-27 02:18	alex	Note Edited: 0003554	View Revisions
2011-06-26 23:25	Dmitry	Assigned To	Dmitry => alex
2011-06-26 23:25	Dmitry	Note Added: 0003554
2011-04-04 12:04	Dmitry	Target Version	Icebox => 5.1.3
2010-12-06 12:06	alex	Target Version	5.1.2 => Icebox
2010-09-28 14:03	Dmitry	Assigned To	=> Dmitry
2010-09-28 14:03	Dmitry	Status	active => needs work
2010-09-28 13:59	Dmitry	Fixed in Version	5.1.1 =>
2010-09-28 13:59	Dmitry	Target Version	Icebox => 5.1.2
2010-09-28 13:40	Dmitry	Developer	=> Dmitry
2010-09-28 13:40	Dmitry	Fixed in Version	=> 5.1.1
2009-10-13 04:07	alex	Target Version	=> Icebox
2009-10-13 04:06	alex	New Issue

Web Development by Intechnic

In-Portal Open Source CMS