Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

N°7963 - Inlineimage::SetDefaultOrgId blend field name between Person and linked class #680

Open
wants to merge 8 commits into
base: support/3.2
Choose a base branch
from
Open

N°7963 - Inlineimage::SetDefaultOrgId blend field name between Person and linked class #680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c2d0c31
N°7963 - Inlineimage::SetDefaultOrgId blend field name between Person…
accognet Nov 13, 2024
83f1476
Refactor
accognet Nov 25, 2024
309b6bd
Replace UserRightsProfile::GetOwnerOrganizationAttCode($sClass) by U…
accognet Dec 9, 2024
f26ed0e
indentation
accognet Dec 16, 2024
77989b6
Add tests
accognet Dec 16, 2024
2415d3d
Add tests
accognet Dec 16, 2024
add7743
Add tests
accognet Dec 16, 2024
3ff97be
use new method GetOwnerOrganizationAttCode in Attachment
accognet Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 4 additions & 24 deletions addons/userrights/userrightsprofile.class.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,7 @@ public function GetSelectFilter($oUser, $sClass, $aSettings = array())
$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);
Expand Down Expand Up @@ -834,7 +834,7 @@ public function IsActionAllowed($oUser, $sClass, $iActionCode, $oInstanceSet = n
// 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);
Expand Down Expand Up @@ -938,31 +938,11 @@ public function FlushPrivileges()
* @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;

$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;
return UserRights::GetOwnerOrganizationAttCode($sClass);
}

/**
Expand Down
27 changes: 3 additions & 24 deletions addons/userrights/userrightsprofile.db.class.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ public function GetSelectFilter($oUser, $sClass, $aSettings = array())

// 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
Expand Down Expand Up @@ -909,7 +909,7 @@ public function IsActionAllowed($oUser, $sClass, $iActionCode, $oInstanceSet = n
// 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);
Expand Down Expand Up @@ -1015,28 +1015,7 @@ public function FlushPrivileges()
*/
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;
return UserRights::GetOwnerOrganizationAttCode($sClass);;
}

/**
Expand Down
45 changes: 15 additions & 30 deletions core/inlineimage.class.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,36 +140,21 @@ public function SetItem(DBObject $oItem, $bUpdateOnChange = false)
*/
public function SetDefaultOrgId()
{
// First check that the organization CAN be fetched from the target class
//
$sClass = $this->Get('item_class');
$aCallSpec = array($sClass, 'MapContextParam');
if (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))
{
// Second: check that the organization CAN be fetched from the current user
//
if (MetaModel::IsValidClass('Person'))
{
$aCallSpec = array($sClass, 'MapContextParam');
if (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))
{
// OK - try it
//
$oCurrentPerson = MetaModel::GetObject('Person', UserRights::GetContactId(), false);
if ($oCurrentPerson)
{
$this->Set('item_org_id', $oCurrentPerson->Get($sAttCode));
}
}
}
}
}
// 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')))) {
// No need for silos
return;
}
// get organization attribute code for the person class
$sOrgAttrCodeForPerson = UserRights::GetOwnerOrganizationAttCode('Person');
if (is_null($sOrgAttrCodeForPerson)) {
// No need for silos
return;
}

$oCurrentPerson = MetaModel::GetObject('Person', UserRights::GetContactId(), false);
if ($oCurrentPerson) {
$this->Set('item_org_id', $oCurrentPerson->Get($sOrgAttrCodeForPerson));
}
}

Expand Down
30 changes: 30 additions & 0 deletions core/userrights.class.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2033,6 +2033,36 @@ public static function GetLastLoginStatus()
{
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;
}

}

/**
Expand Down
59 changes: 22 additions & 37 deletions datamodels/2.x/itop-attachments/datamodel.itop-attachments.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,26 +160,20 @@
$this->Set('item_class', $sClass);
$this->Set('item_id', $iItemId);

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

$oCurrentPerson = MetaModel::GetObject('Person', UserRights::GetContactId(), false);
if ($oCurrentPerson) {
$this->Set('item_org_id', $oCurrentPerson->Get($sOrgAttrCodeForPerson));
}
}]]></code>
</method>
Expand Down
4 changes: 2 additions & 2 deletions datamodels/2.x/itop-structure/module.itop-structure.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public static function AfterDatabaseCreation(Config $oConfiguration, $sPreviousV

$sPersonClass = 'Person';
$sPersonStateAttCode = MetaModel::GetStateAttributeCode($sPersonClass);
$sPersonOwnerOrgAttCode = UserRightsProfile::GetOwnerOrganizationAttCode($sPersonClass);
$sPersonOwnerOrgAttCode = UserRights::GetOwnerOrganizationAttCode($sPersonClass);

$iClassesWithLogCount = 0;
$aCreatedTriggerIds = [];
Expand Down Expand Up @@ -177,7 +177,7 @@ public static function AfterDatabaseCreation(Config $oConfiguration, $sPreviousV
);

// Filter on class owner org. if any
$sClassOwnerOrgAttCode = UserRightsProfile::GetOwnerOrganizationAttCode($sClass);
$sClassOwnerOrgAttCode = UserRights::GetOwnerOrganizationAttCode($sClass);
$oOwnerOrgExpr = empty($sClassOwnerOrgAttCode) ? null : new BinaryExpression(
new FieldExpression($sPersonOwnerOrgAttCode),
'=',
Expand Down
37 changes: 35 additions & 2 deletions tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1087,10 +1087,10 @@ protected function GivenObject(string $sClass, array $aParams): DBObject
* @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) {
Expand Down Expand Up @@ -1406,4 +1406,37 @@ protected function SkipIfModuleNotPresent(string $sModule): void
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);
}
}
29 changes: 29 additions & 0 deletions tests/php-unit-tests/unitary-tests/core/InlineImageTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,33 @@ public function OnFormCancelInvalidTempIdProvider()
],
];
}

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');
}

}
14 changes: 14 additions & 0 deletions tests/php-unit-tests/unitary-tests/core/UserRightsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,4 +488,18 @@ public function NonAdminCannotListAdminProfilesProvider(): array
'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'));
}
}
Loading