Post by JNeuhoff » Thu Oct 30, 2008 6:33 pm

I'd like the entry at http://code.google.com/p/open-cart/issues/detail?id=100 to be addressed again, because:

I think your fix will break some contributions, espcially the Export/Import module.

An image file can be shared by more than one image_id in the database. In fact, each image (as per the auto-
increment of the image_id) can be described in the database by its filename and its title. For instance, several
categories can share the same physical image file name (e.g. folder.png), with different titles.

Hence, before you delete the physical image file, make sure that no other image_id uses the same name in
the database! Your proposed fix at http://code.google.com/p/open-cart/source/detail?r=185 should therefore be updated
to take this into account.

In file /admin/controller/image.php replace the old fix

Code: Select all

	...
	$result = $database->getRows("select * from image where image_id = '" . (int)$request->get('image_id') . "'");
	$result = array_shift($result);
	$image->delete($result['filename']);
	$database->query("delete from image where image_id = '" . (int)$request->get('image_id') . "'");
	...
with something like this:

Code: Select all

	...
	$result = $database->getRows("select * from image where image_id = '" . (int)$request->get('image_id') . "'");
	$result = array_shift($result);
	$filename = $result['filename'];
	$rows = $database->getRows("select filename from image where filename=$filename");
	if (count($rows) <= 1) {
		$image->delete($filename);
	}
	$database->query("delete from image where image_id = '" . (int)$request->get('image_id') . "'");
	...
Last edited by JNeuhoff on Thu Oct 30, 2008 10:38 pm, edited 1 time in total.

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by hm2k » Thu Oct 30, 2008 7:08 pm

Oh you're posting in two places again, no matter as this one appears to have more information.

Firstly I must ask do you "think" or "know"?

And as I said on the issue tracker:
I believe that's what ValidateDelete() does, but I could be wrong.

Can you test your theory and get back to me?
Thanks.

UK Web Hosting


User avatar
Global Moderator

Posts

Joined
Tue Mar 11, 2008 9:06 am
Location - UK

Post by JNeuhoff » Thu Oct 30, 2008 7:21 pm

I'll test my proposed fix and let you know.

What I am certain of is that the current fix doesn't take it account the scenario where multiple image_ids share the the same image filename.

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by hm2k » Thu Oct 30, 2008 8:46 pm

How does that scenario occur?

UK Web Hosting


User avatar
Global Moderator

Posts

Joined
Tue Mar 11, 2008 9:06 am
Location - UK

Post by JNeuhoff » Thu Oct 30, 2008 9:46 pm

This scenario most commonly occurs in conjunction with the Export/Import module.

Each image, with an automatically incremented image_id, is assumed to be uniquely described by the filename and title, during an import from a spreadsheet file.

I am testing my proposed fix right now ...

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by JNeuhoff » Thu Oct 30, 2008 10:14 pm

Below is a scenario where the categories share the same image file named 'folder.png'.

Attachments

???
screenshot2.png
???
screenshot1.png

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by Qphoria » Thu Oct 30, 2008 10:15 pm

is it also not possible to add it directly through the admin image area?

upload a file image1.jpg and title it "first"
upload another file image1.jpg and title it "second"

That would link 2 titles to the same filename, no? (haven't tested)

Image


User avatar
Administrator

Posts

Joined
Tue Jul 22, 2008 3:02 am

Post by JNeuhoff » Thu Oct 30, 2008 10:20 pm

is it also not possible to add it directly through the admin image area?
Good question, I need to test it, too. Never done it that way.

First though I am going to have my deserved lunch break :)

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by Qphoria » Thu Oct 30, 2008 10:41 pm

You eat when you finish! NO SOUP FOR YOU!

Image


User avatar
Administrator

Posts

Joined
Tue Jul 22, 2008 3:02 am

Post by bruce » Thu Oct 30, 2008 11:19 pm

Oh listen to you!! Mr "I don't code on weekends"  :D

Active Member

Posts

Joined
Wed Dec 12, 2007 2:26 pm

Post by Qphoria » Thu Oct 30, 2008 11:29 pm

That was before I was part of the team.... hm2k (I think the "h" stands for "hitler") never lets me rest!
Now you must all conform!

Image


User avatar
Administrator

Posts

Joined
Tue Jul 22, 2008 3:02 am

Post by JNeuhoff » Fri Oct 31, 2008 1:31 am

OK, I have tested it successfully.

Function Delete() in file /admin/controller/image.php should look like this:

Code: Select all

	function delete() {
		$request	=& $this->locator->get('request');
		$response	=& $this->locator->get('response');
		$database	=& $this->locator->get('database');
		$url		=& $this->locator->get('url');
		$language	=& $this->locator->get('language');
		$template	=& $this->locator->get('template');
		$session	=& $this->locator->get('session');
		$module		=& $this->locator->get('module');
		$cache		=& $this->locator->get('cache');
		$image		=& $this->locator->get('image');
 
		$language->load('controller/image.php');
		
		$template->set('title', $language->get('heading_title'));
		 
		if (($request->get('image_id')) && ($this->validateDelete())) {
			$result = $database->getRows("select * from image where image_id = '" . (int)$request->get('image_id') . "'");
			$result = array_shift($result);
			$filename = $result['filename'];
			$rows = $database->getRows("select filename from image where filename='$filename'");
			if (count($rows) <= 1) {
				$image->delete($filename);
			}
			$database->query("delete from image where image_id = '" . (int)$request->get('image_id') . "'");
			$database->query("delete from image_description where image_id = '" . (int)$request->get('image_id') . "'");
	  		$cache->delete('image');
			$session->set('message', $language->get('text_message'));
		  	$response->redirect($url->ssl('image'));
		}
    
		$template->set('content', $this->getList());
		$template->set($module->fetch());
	
		$response->set($template->fetch('layout.tpl'));
	}

Export/Import Tool * SpamBot Buster * Unused Images Manager * Instant Option Price Calculator * Number Option * Google Tag Manager * Survey Plus * OpenTwig


User avatar
Guru Member

Posts

Joined
Wed Dec 05, 2007 3:38 am


Post by hm2k » Fri Oct 31, 2008 2:30 am

Qphoria wrote: That was before I was part of the team.... hm2k (I think the "h" stands for "hitler") never lets me rest!
Now you must all conform!
Well, me and Hitler do share birthdays.

But on a more serious note, obviously there is some kind of issue here, i'm not 100% sure what yet as i've not investigated it myself as of yet.

However, this issue can be tracked here: http://code.google.com/p/open-cart/issues/detail?id=100

This thread will now be locked.

UK Web Hosting


User avatar
Global Moderator

Posts

Joined
Tue Mar 11, 2008 9:06 am
Location - UK
Who is online

Users browsing this forum: No registered users and 0 guests