Startartikel Eigenschaft wird gelöscht

emergence
Beiträge: 10645
Registriert: Mo 28. Jul 2003, 12:49
Wohnort: Austria
Kontaktdaten:

Beitrag von emergence » Di 4. Jul 2006, 09:51

hmm... ich hab mir das jetzt ein wenig überlegt...
bin mir nicht sicher ob die änderung so korrekt ist...

Code: Alles auswählen

                        $tmp_startidartlang = $db->f('startidartlang'); 
                        if ($idartlang != $tmp_startidartlang) 
                        { 
                            conMakeStart($tmp_idcatart, 0); 
                        }
heisst falls $idartlang nicht $db->f('startidartlang') ist soll dieser artikel nicht startartikel sein...
-> aber dann ist er es sowieso nicht...

der orginal code ist meiner meinung nach korrekt...

denn alles in der if schleife

Code: Alles auswählen

			if (!isset($is_start))
			{
....
			}
dient ja nur dazu folgendes zu machen *

Code: Alles auswählen

			if (!isset($is_start))
			{
				conMakeStart($tmp_idcatart, 0);
			}
falls der artikel ein startartikel war...

* vermutlich würde das als code stück dort auch reichen... (anstelle der komplizierten konstruktion die dort momentan zu finden ist)
*** make your own tools (wishlist :: thx)

xmurrix
Beiträge: 3149
Registriert: Do 21. Okt 2004, 11:08
Wohnort: Augsburg
Kontaktdaten:

Beitrag von xmurrix » Di 4. Jul 2006, 11:09

heisst falls $idartlang nicht $db->f('startidartlang') ist soll dieser artikel nicht startartikel sein...
-> aber dann ist er es sowieso nicht...
Stimmt, wenn die beiden IDs nicht übereinstimmen, dann ist es auch kein Startartikel, aber falls sie überinstimmen, dann ist es Eins. Und im aktuellen Code wird ja die Eigenschaft trotzdem zurückgesetzt wenn $is_start nicht gesetzt ist.

der orginal code ist meiner meinung nach korrekt...

denn alles in der if schleife

Code: Alles auswählen

			if (!isset($is_start))
			{
....
			}
dient ja nur dazu folgendes zu machen *

Code: Alles auswählen

			if (!isset($is_start))
			{
				conMakeStart($tmp_idcatart, 0);
			}
falls der artikel ein startartikel war...

* vermutlich würde das als code stück dort auch reichen... (anstelle der komplizierten konstruktion die dort momentan zu finden ist)
Falls der Artikel ein Startartikel ist/war, und der Parameter $is_start nicht über das Formular übertragen wurde, das ist der Fall, wenn der User kein Recht auf Änderung der Startartikel-Eigenschaft hat, sollte dies nicht zurückgesetzt werden. Das passiert ja in diesem Fall. Um das zu umgehen sollte der Code korrigiert werden.

Code: Alles auswählen

//startartikel wird zurückgesetzt, wenn $is_start nicht gesetzt ist und 
//aktueller artikel der startartikel der aktuellen kategorie ist
$tmp_startidartlang = $db->f('startidartlang');
if ($idartlang == $tmp_startidartlang)
{
    conMakeStart($tmp_idcatart, 0);
}
Kann sein, dass ich mich hier komplett irre, glaube ich aber nicht...

emergence
Beiträge: 10645
Registriert: Mo 28. Jul 2003, 12:49
Wohnort: Austria
Kontaktdaten:

Beitrag von emergence » Di 4. Jul 2006, 11:53

Stimmt, wenn die beiden IDs nicht übereinstimmen, dann ist es auch kein Startartikel, aber falls sie überinstimmen, dann ist es Eins. Und im aktuellen Code wird ja die Eigenschaft trotzdem zurückgesetzt wenn $is_start nicht gesetzt ist.
ähm sobald eine checkbox nicht gesetzt ist wird nie ein wert übergeben...
es ist somit egal ob man das recht hat oder nicht... das recht ob es geändert werden darf muss zusätzlich berücksichtigt werden...

zusätzlich
die funktionen conEditArt und conEditFirstArt enthalten ja auch code den startartikel status zu setzen... (was sie zeitweise ja auch tun)
keine der beiden funktionen macht es meiner meinung nach richtig..
die action in der con_saveart versucht diesen sachverhalt einfach wieder zu korrigieren...

der ablauf stimmt einfach nicht...
*** make your own tools (wishlist :: thx)

emergence
Beiträge: 10645
Registriert: Mo 28. Jul 2003, 12:49
Wohnort: Austria
Kontaktdaten:

Beitrag von emergence » Sa 29. Jul 2006, 10:12

das ist jetzt ne größere sache, die nur jemand einbauen sollte der auch ahnung von der materie hat... getestet bei einer 4.6.8
*high risk*
wer keine ahnung hat was er genau tun soll -> finger davon lassen...
wenn sich jemand sein system damit zusammen legt, selbst schuld -> nicht mein problem...

behebt eigentlich(also meiner meinung nach) alle erwähnten probleme...
hab lange überlegt ob ich das überhaupt veröffentlichen soll... insgesamt stecken in dem umbau an die 30 stunden arbeit...

zusammenfassung von conEditArt und conEditFirstArt in eine funktion
have_perm wurde aus der funktion entfernt
startartikel funktionalität wurde direkt in die funktion verlegt
bereinigung und vereinfachung der abläufe (doppelte code passagen wurden entfernt)
update und insert statements werden nun anders generiert (so das es auch nachvollziehbar ist)
zusätzliche berücksichtigung der free_use felder möglich...

ein feature das auch nie richtig funktioniert hat, wurde entfernt (autostart)
erledigt das formular selbst, wenn es der einzige artikel in einer kategorie ist...

änderung in der db -> con_actions - con_saveart

neuer code

Code: Alles auswählen

if (!isset($artspec))
{
  $artspec = "";
}

if (isset($title))
{
	// this variable is needed not to update all fields
	// if exclude is false, all fields will be updated correctly
	// otherwise given fields will be removed on both insert/update statement

	$exclude = false;
    
	if (!$perm->have_perm_area_action_item("con","con_makeonline", $idcat))
	{
		$exclude = Array("online","datestart","dateend","time_move_cat","time_target_cat","time_online_move");
	}

	if (!$perm->have_perm_area_action_item("con","con_makestart", $idcat))
	{
		if (!$exclude) {
			$exclude = Array("is_start");
		} else {
			$exclude[] = "is_start";
		}
	}

	if (1 == $tmp_firstedit)
	{
		// set idart to false so a new article will be generated
		$idart = conEditArt($idcat, $idcatnew, false, $is_start, $idartlang, $lang, $title, $summary, $created, $lastmodified, $author, $online, $datestart, $dateend, $artsort, $exclude);
	} else {
		conEditArt($idcat, $idcatnew, $idart, $is_start, $idartlang, $lang, $title, $summary, $created, $lastmodified, $author, $online, $datestart, $dateend, $artsort, $exclude);
	}

	$tmp_notification = $notification->returnNotification("info", i18n("Changes saved"));

}
functions.con.php funktionen conEditArt und conEditFirstArt werden ersetzt durch folgendes

Code: Alles auswählen

/**
 * Edit an existing article / Create a new Article
 *
 * @param mixed many
 * @return int idart of the article
 *
 * @author Olaf Niemann <Olaf.Niemann@4fb.de>
 *		   Martin Horwath <horwath@dayside.net>
 *
 * @copyright four for business AG <http://www.4fb.de>
 *            dayside.net <http://dayside.net>
 *
 */
function conEditArt($idcat, $idcatnew, $idart, $is_start, $idartlang,
					$idlang, $title, $summary, $created, $lastmodified, $author,
					$online, $datestart, $dateend, $artsort, $exclude = false)
{

		global $db, $client, $lang, $cfg;
		global $auth;

		// Some stuff for the redirect
		global $redirect, $redirect_url, $external_redirect;

		// Some stuff for the time management
		global $time_move_cat; // Used to indicate "move to cat"
		global $time_target_cat; // Used to indicate the target category
		global $time_online_move; // Used to indicate if the moved article should be online
		global $timemgmt;

		// Page title
		global $page_title;

		// Article specification
		global $artspec;

		// Free use fields
		global $free_use_01, $free_use_02, $free_use_03;

		// Needed for metatags
		global $_POST;

		$is_start			= ($is_start == 1) ? 1 : 0;
		$online				= ($online == 1) ? 1 : 0;

		$usetimemgmt		= ($timemgmt == '1' && $is_start == 0) ? '1' : '0';
		$movetocat			= ($time_move_cat == '1') ? '1' : '0';
		$onlineaftermove	= ($time_online_move == '1') ? '1' : '0';

		$redirect			= ($redirect == '1') ? '1' : '0';
		$external_redirect	= ($external_redirect == '1') ? '1' : '0';
		$redirect_url		= ($redirect_url == 'http://' || $redirect_url == '') ? '0' : $redirect_url;

		$free_use_01		= is_numeric($free_use_01) ? $free_use_01 : '0';
		$free_use_02		= is_numeric($free_use_02) ? $free_use_02 : '0';
		$free_use_03		= is_numeric($free_use_03) ? $free_use_03 : '0';

		$idlang 			= is_numeric($idlang) ? $idlang : $lang;

		if ( !$title ) $title = "--- ".i18n("Default title")." ---";

		$title = htmlspecialchars($title, ENT_QUOTES);

		/* Add slashes because single quotes will crash the db */
		$page_title = addslashes($page_title);

		// decide if a new article should be created
		if ($idart === false) {
			$createArticle = true;
		} else {
			$createArticle = false;
		}

		// create new article, first insert minimal database values
		if ($createArticle)
		{
			$idart = $db->nextid($cfg["tab"]["art"]);

			# Table 'con_art'
			$sql = "INSERT INTO ".$cfg["tab"]["art"]." (idart, idclient) VALUES ('$idart', '$client')";
			$db->query($sql);

			# Table 'con_art_lang'
			$idartlang = $db->nextid($cfg["tab"]["art_lang"]);

		}

		if ( !is_numeric($idartlang) )
		{
			$sql = "SELECT idartlang FROM ".$cfg["tab"]["art_lang"]." WHERE idart = '$idart' AND idlang = '$idlang'";
			$db->query($sql);
			$db->next_record();
			$idartlang = $db->f("idartlang");
		}

		// these are the insert/update values
		$aFields = Array("idartlang" => $idartlang,
						 "idart" => $idart,
						 "idlang" => $idlang,
						 "title" => $title,
						 "pagetitle" => $page_title,
						 "summary" => $summary,
						 "artspec" => $artspec,
						 "author" => $auth->auth["uname"],
						 "created" => $created,
						 "modifiedby" => $author,
						 "lastmodified" => $lastmodified,
						 "online" => $online,
						 "redirect" => $redirect,
						 "redirect_url" => $redirect_url,
						 "external_redirect" => $external_redirect,
						 "artsort" => $artsort,
						 "timemgmt" => $usetimemgmt,
						 "datestart" => $datestart,
						 "dateend" => $dateend,
						 "status" => 0,
						 "time_move_cat" => $movetocat,
						 "time_target_cat" => $time_target_cat,
						 "time_online_move" => $onlineaftermove,
						 "free_use_01" => $free_use_01,
						 "free_use_02" => $free_use_02,
						 "free_use_03" => $free_use_03
						);



		// published and publishedby are definied through online status value
		if ($online == 1) {

			$aFields["published"] = date("Y-m-d H:i:s");
			$aFields["publishedby"] = $auth->auth["uname"];

			if (!$createArticle) {
				// check if online id is currently 1
				$sql = "SELECT online FROM ".$cfg["tab"]["art_lang"]." WHERE idartlang='$idartlang'";
				$db->query($sql);
				if($db->next_record()){
					if($db->f("online")==1){
						unset($aFields["published"], $aFields["publishedby"]); // don't update this fields
					}
				}
			}

		} else {
			$aFields["published"] = "";
			$aFields["publishedby"] = "";
		}


		// exclude variables we don't want to update
		// this is done in backend via action con_saveart

		$allow_isstart = true; // enable changing startarticle settings

		if (is_array($exclude)) {

			foreach ($exclude as $value) {
				unset($aFields[$value]);

				if ($value == "online") {
				    unset($aFields["published"], $aFields["publishedby"]); // don't update this fields
				}

				if ($value == "is_start") {
					$allow_isstart = false; // disable changing startarticle settings
				}
			}

		}

		// create new article, insert data in art_lang
		if ($createArticle)
		{

			unset($insertsqlField, $insertsqlValue);
			foreach ($aFields as $key => $value) {
				$insertsqlField[]= "$key";
				$insertsqlValue[]= "'$value'";
			}

			$sql = "INSERT INTO ".$cfg["tab"]["art_lang"]." (".implode(", ", $insertsqlField).") VALUES (".implode(", ", $insertsqlValue).")";

			$db->query($sql);

			unset($insertsqlField, $insertsqlValue, $key, $value);

		}

		 // remove some other value from array, this should not be changed when updating an article
		unset($aFields["idartlang"],
			  $aFields["idart"],
			  $aFields["idlang"],
			  $aFields["author"],
			  $aFields["created"],
			  $aFields["status"]);

		// get all idcats that contain art
		$sql = "SELECT idcat FROM ".$cfg["tab"]["cat_art"]." WHERE idart='$idart'";
		$db->query($sql);

		while ($db->next_record()) {
			$idcatold[] = $db->f("idcat");
		}

		// not sure why this is needed here
		if ( !is_array($idcatnew) ) { $idcatnew[0] = 0; }
		if ( !is_array($idcatold) ) { $idcatold[0] = 0; }

		// multiple article handling
		foreach ($idcatnew as $value) {

			if ( !in_array($value, $idcatold) ) {

				// get new idcatart
				$tmp_idcatart = $db->nextid($cfg["tab"]["cat_art"]);

				// entry in 'cat_art' table
				$sql = "INSERT INTO ".$cfg["tab"]["cat_art"]." (idcatart, idcat, idart) VALUES ('$tmp_idcatart', '$value', '$idart')";
				$db->query($sql);

				// entry in 'stat' table
				$sql = "INSERT INTO ".$cfg["tab"]["stat"]." (idstat, idcatart, idlang, idclient, visited) VALUES ('".$db->nextid($cfg["tab"]["stat"])."', '$tmp_idcatart', '$idlang', '$client', '0')";
				$db->query($sql);

			}
		}

		foreach ($idcatold as $value) {

			if ( !in_array($value, $idcatnew) ) {

				// get idcatart that will no longer exist
				$sql = "SELECT idcatart FROM ".$cfg["tab"]["cat_art"]." WHERE idcat='$value' AND idart='$idart'";
				$db->query($sql);
				$db->next_record();

				$tmp_idcatart = $db->f("idcatart");

				// delete from 'code' table
				$sql = "DELETE FROM ".$cfg["tab"]["code"]." WHERE idcatart='$tmp_idcatart'";
				$db->query($sql);

				// delete from 'stat' table
				$sql = "DELETE FROM ".$cfg["tab"]["stat"]." WHERE idcatart='$tmp_idcatart'";
				$db->query($sql);

				// delete from 'cat_art' table
				$sql = "DELETE FROM ".$cfg["tab"]["cat_art"]." WHERE idart='$idart' AND idcat='$value'";
				$db->query($sql);

				// remove startidartlang
				if (isStartArticle($idartlang, $value, $idlang))
				{
					$sql = "UPDATE ".$cfg["tab"]["cat_lang"]." SET startidartlang='0' WHERE idcat='$value' AND idlang='$idlang'";
					$db->query($sql);
				}

			}

		}


		// update 'art_lang' table
		unset($updatesqlString);
		foreach ($aFields as $key => $value) {
			$updatesqlString[] = $key." = '".$value."'";
		}

		$sql = "UPDATE ".$cfg["tab"]["art_lang"]." SET ".implode(", ", $updatesqlString)." WHERE idartlang='$idartlang'";

		$db->query($sql);

		// set metatags
		$availableTags = conGetAvailableMetaTagTypes();

		foreach ($availableTags as $key => $value)
		{
			conSetMetaValue($idartlang, $key, $_POST['META'.$value["name"]]);
		}

		// startarticle handling
		if ( in_array($idcat, $idcatnew) && $allow_isstart )
		{

			$sql = "SELECT idcatart FROM ".$cfg["tab"]["cat_art"]." WHERE idcat = '".$idcat."' AND idart = '".$idart."'";

			$db->query($sql);
			$db->next_record();

			$tmp_idcatart = $db->f("idcatart");

			conMakeStart($tmp_idcatart, $is_start);

		}

		// set code generation for all articles
		conGenerateCodeForArtInAllCategories($idart);

		return $idart;

}
erweiterungen und umbau der funktion sollten nun um einiges leichter sein..

EDIT: sollte dann ebenfalls korrigiert werden
http://contenido.org/forum/viewtopic.php?t=14252 (bereits gefixt)

im zuge dessen ist mir noch ein bug aufgefallen
ebenfalls functions.con.php folgende bestehende funktion wird durch das hier ersetzt (published status wurde nicht upgedatet, wenn artikel offline geschalten wird)

Code: Alles auswählen

function conMakeOnline($idart, $lang)
{
	global $db, $cfg, $auth;

	$sql = "SELECT online FROM ".$cfg["tab"]["art_lang"]." WHERE idart = '".$idart."' AND idlang = '".$lang."'";
	$db->query($sql);

	$db->next_record();

	$online = ( $db->f("online") == 0 ) ? 1 : 0;

	// these are the update values
	$aFields = Array("online" => $online,
					 "published" => $online ? date("Y-m-d H:i:s") : '',
					 "publishedby" => $online ? $auth->auth["uname"] : '');

	unset($updatesqlString);
	foreach ($aFields as $key => $value) {
		$updatesqlString[] = $key." = '".$value."'";
	}

	$sql = "UPDATE ".$cfg["tab"]["art_lang"]." SET ".implode(", ", $updatesqlString)." WHERE idart = '".$idart."' AND idlang = '".$lang."'";
	$db->query($sql);

}
*** make your own tools (wishlist :: thx)

Gesperrt