use new method GetOwnerOrganizationAttCode in Attachment

+ test

addons/userrights/userrightsprofile.class.inc.php

@@ -648,7 +648,7 @@ class UserRightsProfile extends UserRightsAddOnAPI
 		$aConditions =  array();
 
 		// Determine if this class is part of a silo and build the filter for it
-		$sAttCode = self::GetOwnerOrganizationAttCode($sClass);
+		$sAttCode = UserRights::GetOwnerOrganizationAttCode($sClass);
 		if (!is_null($sAttCode))
 		{
 			$aUserOrgs = $this->GetUserOrgs($oUser, $sClass);
@@ -834,7 +834,7 @@ class UserRightsProfile extends UserRightsAddOnAPI
 					// But currently we are checking wether the objects might be written...
 					// Let's exclude the objects based on the relevant criteria
 
-					$sOrgAttCode = self::GetOwnerOrganizationAttCode($sClass);
+					$sOrgAttCode = UserRights::GetOwnerOrganizationAttCode($sClass);
 					if (!is_null($sOrgAttCode))
 					{
 						$aUserOrgs = $this->GetUserOrgs($oUser, $sClass);
@@ -938,31 +938,11 @@ class UserRightsProfile extends UserRightsAddOnAPI
 	 * @param string $sClass
 	 * @return string|null Find out which attribute is corresponding the dimension 'owner org'
 	 *                   returns null if no such attribute has been found (no filtering should occur)
+	 * @deprecated 3.3.0 use @UserRights::GetOwnerOrganizationAttCode instead
 	 */
 	public static function GetOwnerOrganizationAttCode($sClass)
 	{
-		$sAttCode = null;
+		return UserRights::GetOwnerOrganizationAttCode($sClass);
-
-		$aCallSpec = array($sClass, 'MapContextParam');
-		if (($sClass == 'Organization') || is_subclass_of($sClass, 'Organization'))
-		{
-			$sAttCode = 'id';
-		}
-		elseif (is_callable($aCallSpec))
-		{
-			$sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter
-			if (!MetaModel::IsValidAttCode($sClass, $sAttCode))
-			{
-				// Skip silently. The data model checker will tell you something about this...
-				$sAttCode = null;
-			}
-		}
-		elseif(MetaModel::IsValidAttCode($sClass, 'org_id'))
-		{
-			$sAttCode = 'org_id';
-		}
-
-		return $sAttCode;
 	}
 
 	/**

addons/userrights/userrightsprofile.db.class.inc.php

@@ -778,7 +778,7 @@ exit;
 
 		// Determine how to position the objects of this class
 		//
-		$sAttCode = self::GetOwnerOrganizationAttCode($sClass);
+		$sAttCode = UserRights::GetOwnerOrganizationAttCode($sClass);
 		if (is_null($sAttCode))
 		{
 			// No filtering for this object
@@ -909,7 +909,7 @@ exit;
 					// But currently we are checking wether the objects might be written...
 					// Let's exclude the objects based on the relevant criteria
 
-					$sOrgAttCode = self::GetOwnerOrganizationAttCode($sClass);
+					$sOrgAttCode = UserRights::GetOwnerOrganizationAttCode($sClass);
 					if (!is_null($sOrgAttCode))
 					{
 						$aUserOrgs = $this->GetUserOrgs($oUser, $sClass);
@@ -1015,28 +1015,7 @@ exit;
 	 */	 	
 	public static function GetOwnerOrganizationAttCode($sClass)
 	{
-		$sAttCode = null;
+		return UserRights::GetOwnerOrganizationAttCode($sClass);;
-
-		$aCallSpec = array($sClass, 'MapContextParam');
-		if (($sClass == 'Organization') || is_subclass_of($sClass, 'Organization'))
-		{
-			$sAttCode = 'id';
-		}
-		elseif (is_callable($aCallSpec))
-		{
-			$sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter
-			if (!MetaModel::IsValidAttCode($sClass, $sAttCode))
-			{
-				// Skip silently. The data model checker will tell you something about this...
-				$sAttCode = null;
-			}
-		}
-		elseif(MetaModel::IsValidAttCode($sClass, 'org_id'))
-		{
-			$sAttCode = 'org_id';
-		}
-
-		return $sAttCode;
 	}
 
 	/**

core/inlineimage.class.inc.php

@@ -140,36 +140,21 @@ class InlineImage extends DBObject
 	 */
 	public function SetDefaultOrgId()
 	{
-		// First check that the organization CAN be fetched from the target class
+        // If the item class has no organization attribute, then no need to set the organization id
-		//
+		if (is_null(UserRights::GetOwnerOrganizationAttCode( $this->Get('item_class')))) {
-		$sClass = $this->Get('item_class');
+			// No need for silos
-		$aCallSpec = array($sClass, 'MapContextParam');
+			return;
-		if (is_callable($aCallSpec))
+		}
-		{
+        // get  organization attribute code for the person class
-			$sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter					
+		$sOrgAttrCodeForPerson = UserRights::GetOwnerOrganizationAttCode('Person');
-			if (MetaModel::IsValidAttCode($sClass, $sAttCode))
+		if (is_null($sOrgAttrCodeForPerson)) {
-			{
+			// No need for silos
-				// Second: check that the organization CAN be fetched from the current user
+			return;
-				//
+		}
-				if (MetaModel::IsValidClass('Person'))
+
-				{
+		$oCurrentPerson = MetaModel::GetObject('Person', UserRights::GetContactId(), false);
-					$aCallSpec = array($sClass, 'MapContextParam');
+		if ($oCurrentPerson) {
-					if (is_callable($aCallSpec))
+			$this->Set('item_org_id', $oCurrentPerson->Get($sOrgAttrCodeForPerson));
-					{
-						$sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter					
-						if (MetaModel::IsValidAttCode($sClass, $sAttCode))
-						{
-							// OK - try it
-							//
-							$oCurrentPerson = MetaModel::GetObject('Person', UserRights::GetContactId(), false);
-							if ($oCurrentPerson)
-							{
-						 		$this->Set('item_org_id', $oCurrentPerson->Get($sAttCode));
-						 	}
-						}
-					}
-				}
-			}
 		}
 	}
 

core/userrights.class.inc.php

@@ -2033,6 +2033,36 @@ class UserRights
 	{
 		return self::$m_sLastLoginStatus;
 	}
+
+
+	/**
+	 * @param string $sClass
+	 * @return string|null Find out which attribute is corresponding the dimension 'owner org'
+	 *                   returns null if no such attribute has been found (no filtering should occur)
+	 * @since 3.3.0
+	 */
+	public static function GetOwnerOrganizationAttCode($sClass)
+	{
+		$sAttCode = null;
+
+		$aCallSpec = array($sClass, 'MapContextParam');
+		if (($sClass == 'Organization') || is_subclass_of($sClass, 'Organization'))	{
+			$sAttCode = 'id';
+		}
+		elseif (is_callable($aCallSpec)) {
+			$sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter
+			if (!MetaModel::IsValidAttCode($sClass, $sAttCode)) {
+				// Skip silently. The data model checker will tell you something about this...
+				$sAttCode = null;
+			}
+		}
+		elseif(MetaModel::IsValidAttCode($sClass, 'org_id')) {
+			$sAttCode = 'org_id';
+		}
+
+		return $sAttCode;
+	}
+
 }
 
 /**

datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml

@@ -160,26 +160,20 @@
  		$this->Set('item_class', $sClass);
  		$this->Set('item_id', $iItemId);
 
-		$aCallSpec = array($sClass, 'MapContextParam');
+		$sAttCode = UserRights::GetOwnerOrganizationAttCode( $sClass);
-		if (is_callable($aCallSpec))
+		if (is_null($sAttCode)) {
-		{
+			// No need for silos
-			$sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter					
+			return;
-			if (MetaModel::IsValidAttCode($sClass, $sAttCode))
-			{
-				$iOrgId = $oItem->Get($sAttCode);
-				if ($iOrgId > 0)
-				{
-					if ($iOrgId != $this->Get('item_org_id'))
-					{
-						$this->Set('item_org_id', $iOrgId);
-						if ($bUpdateOnChange)
-						{
-							$this->DBUpdate();
-						}
-					}
-				}
-			}
 		}
+    $iOrgId = $oItem->Get($sAttCode);
+    if ($iOrgId > 0) {
+      if ($iOrgId != $this->Get('item_org_id')) {
+        $this->Set('item_org_id', $iOrgId);
+        if ($bUpdateOnChange) {
+          $this->DBUpdate();
+        }
+      }
+    }
 	}]]></code>
         </method>
         <method id="SetDefaultOrgId">
@@ -193,24 +187,15 @@
           <code><![CDATA[	public function SetDefaultOrgId()
 	{
     // Check that the organization CAN be fetched from the current user
-    //
+    $sOrgAttrCodeForPerson = UserRights::GetOwnerOrganizationAttCode('Person');
-    if (MetaModel::IsValidClass('Person'))
+		if (is_null($sOrgAttrCodeForPerson)) {
-    {
+			// No need for silos
-      $aCallSpec = array('Person', 'MapContextParam');
+			return;
-      if (is_callable($aCallSpec))
+		}
-      {
+
-        $sAttCode = call_user_func($aCallSpec, 'org_id'); // Returns null when there is no mapping for this parameter
+		$oCurrentPerson = MetaModel::GetObject('Person', UserRights::GetContactId(), false);
-        if (MetaModel::IsValidAttCode('Person', $sAttCode))
+		if ($oCurrentPerson) {
-        {
+			$this->Set('item_org_id', $oCurrentPerson->Get($sOrgAttrCodeForPerson));
-          // OK - try it
-          //
-          $oCurrentPerson = MetaModel::GetObject('Person', UserRights::GetContactId(), false);
-          if ($oCurrentPerson)
-          {
-            $this->Set('item_org_id', $oCurrentPerson->Get($sAttCode));
-          }
-        }
-      }
 		}
 	}]]></code>
         </method>

datamodels/2.x/itop-structure/module.itop-structure.php

@@ -142,7 +142,7 @@ if (!class_exists('StructureInstaller'))
 
 				$sPersonClass = 'Person';
 				$sPersonStateAttCode = MetaModel::GetStateAttributeCode($sPersonClass);
-				$sPersonOwnerOrgAttCode = UserRightsProfile::GetOwnerOrganizationAttCode($sPersonClass);
+				$sPersonOwnerOrgAttCode = UserRights::GetOwnerOrganizationAttCode($sPersonClass);
 
 				$iClassesWithLogCount = 0;
 				$aCreatedTriggerIds = [];
@@ -177,7 +177,7 @@ if (!class_exists('StructureInstaller'))
 							);
 
 							// Filter on class owner org. if any
-							$sClassOwnerOrgAttCode = UserRightsProfile::GetOwnerOrganizationAttCode($sClass);
+							$sClassOwnerOrgAttCode = UserRights::GetOwnerOrganizationAttCode($sClass);
 							$oOwnerOrgExpr = empty($sClassOwnerOrgAttCode) ? null : new BinaryExpression(
 								new FieldExpression($sPersonOwnerOrgAttCode),
 								'=',

tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php

@@ -1087,10 +1087,10 @@ abstract class ItopDataTestCase extends ItopTestCase
 	 * @param string $sClass
 	 * @param array $aValues
 	 *
-	 * @return DBObject
+	 * @return int
 	 * @throws Exception
 	 */
-	protected function GivenObjectInDB($sClass, $aValues)
+	protected function GivenObjectInDB($sClass, $aValues):int
 	{
 		// Check and complete the values
 		foreach ($aValues as $sAttCode => $oValue) {
@@ -1406,4 +1406,37 @@ abstract class ItopDataTestCase extends ItopTestCase
 			self::markTestSkipped("Test skipped: module '$sModule' is not present");
 		}
 	}
+
+	protected function GivenUserLoggedInWithContact(int $iContactOrgId)
+	{
+		$iContactId = $this->GivenObjectInDB('Person', [
+			'first_name' => 'TestContact',
+			'name' => 'TestContact',
+			'org_id' => $iContactOrgId]);
+		$sLogin = 'demo_test_'.uniqid(__CLASS__, true);
+		$iUser = $this->GivenObjectInDB('UserLocal', [
+			'login' => $sLogin,
+			'password' => 'tagada-Secret,007',
+			'language' => 'EN US',
+			'contactid' => $iContactId,
+			'profile_list' => [
+				'profileid:'.self::$aURP_Profiles['Configuration Manager']
+			]
+		]);
+		\UserRights::Login($sLogin);
+	}
+
+	protected function GivenUserLoggedInWithoutContact()
+	{
+		$sLogin = 'demo_test_'.uniqid(__CLASS__, true);
+		$iUser = $this->GivenObjectInDB('UserLocal', [
+			'login' => $sLogin,
+			'password' => 'tagada-Secret,007',
+			'language' => 'EN US',
+			'profile_list' => [
+				'profileid:'.self::$aURP_Profiles['Configuration Manager']
+			]
+		]);
+		\UserRights::Login($sLogin);
+	}
 }

tests/php-unit-tests/unitary-tests/core/InlineImageTest.php

@@ -59,4 +59,33 @@ class InlineImageTest extends ItopDataTestCase
 			],
 		];
 	}
+
+	public function testSetDefaultOrgIdWhenLoggedInWithContact()
+	{
+		$iContactOrgId = $this->GivenObjectInDB('Organization', ['name' => 'TestOrg']);
+		$this->GivenUserLoggedInWithContact($iContactOrgId);
+
+		$oInlineImage = \MetaModel::NewObject('InlineImage',['item_class' => 'UserRequest']);
+		$oInlineImage->SetDefaultOrgId();
+		$this->assertEquals($iContactOrgId, $oInlineImage->Get('item_org_id'),'The org_id should be the one of the contact');
+
+		$oInlineImage = \MetaModel::NewObject('InlineImage',['item_class' => 'TriggerOnObjectCreate']);
+		$oInlineImage->SetDefaultOrgId();
+		$this->assertEquals(0, $oInlineImage->Get('item_org_id'),'The org_id should be left undefined');
+	}
+
+
+	public function testSetDefaultOrgIdWhenLoggedInWithoutContact()
+	{
+		$this->GivenUserLoggedInWithoutContact();
+
+		$oInlineImage = \MetaModel::NewObject('InlineImage',['item_class' => 'UserRequest']);
+		$oInlineImage->SetDefaultOrgId();
+		$this->assertEquals(0, $oInlineImage->Get('item_org_id'),'The org_id should be left undefined');
+
+		$oInlineImage = \MetaModel::NewObject('InlineImage',['item_class' => 'TriggerOnObjectCreate']);
+		$oInlineImage->SetDefaultOrgId();
+		$this->assertEquals(0, $oInlineImage->Get('item_org_id'),'The org_id should be left undefined');
+	}
+
 }

tests/php-unit-tests/unitary-tests/core/UserRightsTest.php

@@ -488,4 +488,18 @@ class UserRightsTest extends ItopDataTestCase
 			'with Admins hidden' => [true, 0],
 		];
 	}
+
+	public function testGetOwnerOrganizationAttCode()
+	{
+		$this->assertEquals('id', UserRights::GetOwnerOrganizationAttCode('Organization'));
+
+		$this->assertEquals('org_id', UserRights::GetOwnerOrganizationAttCode('Server'));
+		$this->assertEquals('org_id', UserRights::GetOwnerOrganizationAttCode('UserRequest'));
+
+		$this->assertEquals('item_org_id', UserRights::GetOwnerOrganizationAttCode('InlineImage'));
+		$this->assertEquals('item_org_id', UserRights::GetOwnerOrganizationAttCode('Attachment'));
+
+		$this->assertNull(UserRights::GetOwnerOrganizationAttCode('TriggerOnObjectCreation'));
+		$this->assertNull(UserRights::GetOwnerOrganizationAttCode('lnkPersonToTeam'));
+	}
 }

tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-attachments/TestAttachment.php

@@ -69,4 +69,46 @@ class TestAttachment extends ItopDataTestCase
 		$oDocument = $oAttachment->Get('contents');
 		$this->sRemoveAttachmentName = $oDocument->GetFileName();
 	}
+
+
+	public function testSetItemOnObjectWithDefinedOrganization()
+	{
+		$iOrgId = $this->GivenObjectInDB('Organization', ['name' => 'TestOrg']);
+		$oUserRequest = $this->GivenObject('UserRequest', ['title' => 'TestUserRequest', 'org_id'=>$iOrgId]);
+
+		$oAttachment = new \Attachment();
+		$oAttachment->SetItem($oUserRequest);
+		$this->assertEquals($iOrgId, $oAttachment->Get('item_org_id'),'The org_id should be the one of the contact');
+	}
+
+
+	public function testSetItemOnObjectWithoutDefinedOrganization()
+	{
+		$oUserRequest = $this->GivenObject('TriggerOnObjectCreate', ['target_class' => 'UserRequest','description'=>'TestUserRequest']);
+
+		$oAttachment = new \Attachment();
+		$oAttachment->SetItem($oUserRequest);
+		$this->assertEquals(0, $oAttachment->Get('item_org_id'),'The org_id should be the one of the contact');
+	}
+
+
+	public function testSetDefaultOrgIdWhenLoggedInWithContact()
+	{
+		$iContactOrgId = $this->GivenObjectInDB('Organization', ['name' => 'TestOrg']);
+		$this->GivenUserLoggedInWithContact($iContactOrgId);
+
+		$oAttachment = new \Attachment();
+		$oAttachment->SetDefaultOrgId();
+		$this->assertEquals($iContactOrgId, $oAttachment->Get('item_org_id'),'The org_id should be the one of the contact');
+	}
+
+
+	public function testSetDefaultOrgIdWhenLoggedInWithoutContact()
+	{
+		$this->GivenUserLoggedInWithoutContact();
+
+		$oAttachment = new \Attachment();
+		$oAttachment->SetDefaultOrgId();
+		$this->assertEquals(0, $oAttachment->Get('item_org_id'),'The org_id should be left undefined');
+	}
 }