From 144ca490ed7cfbe56ea18261428e8d59c7121275 Mon Sep 17 00:00:00 2001 From: odain Date: Tue, 5 Sep 2023 13:35:12 +0200 Subject: [PATCH 1/3] 5324-adapt test --- .../UserProfilesEventListenerTest.php | 97 ++++++++++++++----- 1 file changed, 71 insertions(+), 26 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-profiles-itil/UserProfilesEventListenerTest.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-profiles-itil/UserProfilesEventListenerTest.php index d791c64ca..de30a7f5c 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-profiles-itil/UserProfilesEventListenerTest.php +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-profiles-itil/UserProfilesEventListenerTest.php @@ -63,7 +63,8 @@ class UserProfilesEventListenerTest extends ItopDataTestCase 'aExpectedAssociatedProfilesAfterUserCreation'=> [ 'Portal power user', 'Portal user', - ] + ], + 'bCheckSessionMessage' => true ], 'Portal power user + Configuration Manager => profiles untouched' => [ 'aAssociatedProfilesBeforeUserCreation' => [ @@ -82,21 +83,47 @@ class UserProfilesEventListenerTest extends ItopDataTestCase * @dataProvider PortaPowerUserProvider */ public function testUserLocalCreation($aAssociatedProfilesBeforeUserCreation, - $aExpectedAssociatedProfilesAfterUserCreation) + $aExpectedAssociatedProfilesAfterUserCreation, $bCheckSessionMessage=false) { + /*if ($bCheckSessionMessage){ + $sLogin = "Admin-" . uniqid(); + $oConnectedUser = $this->CreateContactlessUser($sLogin, 1, "Iuytrez9876543ç_è-("); + $_SESSION = []; + \UserRights::Login($oConnectedUser->Get('login')); + }*/ + $oUser = new \UserLocal(); $sLogin = 'testUserLocalCreationWithPortalPowerUserProfile-'.uniqid(); $oUser->Set('login', $sLogin); $oUser->Set('password', 'ABCD1234@gabuzomeu'); $oUser->Set('language', 'EN US'); $this->commonUserCreationTest($oUser, $aAssociatedProfilesBeforeUserCreation, $aExpectedAssociatedProfilesAfterUserCreation); + + /*if ($bCheckSessionMessage){ + $aObjMessages = Session::Get('obj_messages'); + $this->assertNotEmpty($aObjMessages); + $sKey = sprintf("%s::%s", get_class($oUser), $oUser->GetKey()); + $this->assertTrue(array_key_exists($sKey, $aObjMessages)); + + $sMsg = << 1, + 'severity' => 'WARNING', + 'message' => $sMsg + ] + ]; + $this->assertEquals($aExpectedMessages, array_values($aObjMessages[$sKey]), var_export($aObjMessages[$sKey], true)); + }*/ } /** * @dataProvider PortaPowerUserProvider */ public function testUserLocalUpdate($aAssociatedProfilesBeforeUserCreation, - $aExpectedAssociatedProfilesAfterUserCreation) + $aExpectedAssociatedProfilesAfterUserCreation, $bCheckSessionMessage=false) { $oUser = new \UserLocal(); $sLogin = 'testUserLocalUpdateWithPortalPowerUserProfile-'.uniqid(); @@ -110,7 +137,7 @@ class UserProfilesEventListenerTest extends ItopDataTestCase * @dataProvider PortaPowerUserProvider */ public function testUserLDAPCreation($aAssociatedProfilesBeforeUserCreation, - $aExpectedAssociatedProfilesAfterUserCreation) + $aExpectedAssociatedProfilesAfterUserCreation, $bCheckSessionMessage=false) { $oUser = new \UserLDAP(); $sLogin = 'testUserLDAPCreationWithPortalPowerUserProfile-'.uniqid(); @@ -134,7 +161,7 @@ class UserProfilesEventListenerTest extends ItopDataTestCase * @dataProvider PortaPowerUserProvider */ public function testUserExternalCreation($aAssociatedProfilesBeforeUserCreation, - $aExpectedAssociatedProfilesAfterUserCreation) + $aExpectedAssociatedProfilesAfterUserCreation, $bCheckSessionMessage=false) { $oUser = new \UserExternal(); $sLogin = 'testUserLDAPCreationWithPortalPowerUserProfile-'.uniqid(); @@ -146,7 +173,7 @@ class UserProfilesEventListenerTest extends ItopDataTestCase * @dataProvider PortaPowerUserProvider */ public function testUserExternalUpdate($aAssociatedProfilesBeforeUserCreation, - $aExpectedAssociatedProfilesAfterUserCreation) + $aExpectedAssociatedProfilesAfterUserCreation, $bCheckSessionMessage=false) { $oUser = new \UserExternal(); $sLogin = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid(); @@ -217,7 +244,6 @@ class UserProfilesEventListenerTest extends ItopDataTestCase $_SESSION = []; - //$this->expectException(\Exception::class); UserRights::Login($oUser->Get('login')); if (! UserRights::IsPortalUser()) { @@ -274,6 +300,11 @@ class UserProfilesEventListenerTest extends ItopDataTestCase if ($bRaiseException){ $this->expectException(\DeleteException::class); + $sMessage = <<expectExceptionMessage($sMessage); } $aURPUserProfileByUser = $this->GetURPUserProfileByUser($sId); @@ -316,7 +347,25 @@ class UserProfilesEventListenerTest extends ItopDataTestCase if (!$bRaiseException) { $aExpectedProfilesAfterUpdate = ["Portal power user", "Portal user"]; $this->CheckProfilesAreOkAndThenConnectToITop($sUserClass, $sId, $aExpectedProfilesAfterUpdate); - } + + //check warning + /*$aObjMessages = Session::Get('obj_messages'); + $this->assertNotEmpty($aObjMessages); + $sKey = sprintf("%s::%s", get_class($oURPUserProfile), $oURPUserProfile->GetKey()); + $this->assertTrue(array_key_exists($sKey, $aObjMessages)); + + $sMsg = << 1, + 'severity' => 'WARNING', + 'message' => $sMsg + ] + ]; + $this->assertEquals($aExpectedMessages, array_values($aObjMessages[$sKey]), var_export($aObjMessages[$sKey], true));*/ + } } public function ProfilesLinksProvider() { @@ -333,19 +382,24 @@ class UserProfilesEventListenerTest extends ItopDataTestCase $aInitialProfiles = [ $sProfileNameToMove, "Portal power user"]; $oUser = new \UserExternal(); - $sLogin = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid(); - $oUser->Set('login', $sLogin); + $sLogin1 = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid(); + $oUser->Set('login', $sLogin1); $sUserClass = get_class($oUser); list ($sId, $aProfiles) = $this->CreateUserForProfileTesting($oUser, $aInitialProfiles); $oUser = new \UserExternal(); - $sLogin = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid(); - $oUser->Set('login', $sLogin); + $sLogin2 = 'testUserLDAPUpdateWithPortalPowerUserProfile-'.uniqid(); + $oUser->Set('login', $sLogin2); list ($sAnotherUserId, $aProfiles) = $this->CreateUserForProfileTesting($oUser, ["Configuration Manager"]); if ($bRaiseException){ $this->expectException(\CoreCannotSaveObjectException::class); + $sMessage = <<expectExceptionMessage($sMessage); } $aURPUserProfileByUser = $this->GetURPUserProfileByUser($sId); @@ -590,23 +644,14 @@ class UserProfilesEventListenerTest extends ItopDataTestCase $oUserProfilesEventListener->RegisterEventsAndListeners(); $this->expectException(\CoreCannotSaveObjectException::class); + $sMessage = <<expectExceptionMessage($sMessage); $this->commonUserCreationTest($oUser, $aAssociatedProfilesBeforeUserCreation, $aAssociatedProfilesBeforeUserCreation, false); - /*$aObjMessages = Session::Get('obj_messages'); - $this->assertNotEmpty($aObjMessages); - $sKey = sprintf("%s::%s", get_class($oUser), $oUser->GetKey()); - $this->assertTrue(array_key_exists($sKey, $aObjMessages)); - - $aExpectedMessages = [ - [ - 'rank' => 1, - 'severity' => 'WARNING', - 'message' => \Dict::Format("Class:User/NonStandaloneProfileWarning", 'Portal power user') - ] - ]; - $this->assertEquals($aExpectedMessages, array_values($aObjMessages[$sKey]), var_export($aObjMessages[$sKey], true)); -*/ $_SESSION = []; } } From b3820560f1ff6dd7f1221dec060478a8c6ab7aef Mon Sep 17 00:00:00 2001 From: odain Date: Tue, 5 Sep 2023 15:03:46 +0200 Subject: [PATCH 2/3] 5324-make test work with iTop core profiles AND saas targets --- .../UserProfilesEventListenerTest.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-profiles-itil/UserProfilesEventListenerTest.php b/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-profiles-itil/UserProfilesEventListenerTest.php index de30a7f5c..7bfd5b1f7 100644 --- a/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-profiles-itil/UserProfilesEventListenerTest.php +++ b/tests/php-unit-tests/unitary-tests/datamodels/2.x/itop-profiles-itil/UserProfilesEventListenerTest.php @@ -512,7 +512,7 @@ TXT; public function testInit_ConfWithOneWarningProfile() { \MetaModel::GetConfig()->Set(UserProfilesEventListener::USERPROFILE_REPAIR_ITOP_PARAM_NAME, - ['Ticket Manager' => 'Administrator', 'Portal power user' => null] + ['Configuration Manager' => 'Administrator', 'Portal power user' => null] ); $oUserProfilesEventListener = new UserProfilesEventListener(); $oUserProfilesEventListener->Init(); @@ -521,7 +521,7 @@ TXT; public function testInit_ConfWithFurtherWarningProfiles() { \MetaModel::GetConfig()->Set(UserProfilesEventListener::USERPROFILE_REPAIR_ITOP_PARAM_NAME, - ['Ticket Manager' => null, 'Portal power user' => null] + ['Configuration Manager' => null, 'Portal power user' => null] ); $oUserProfilesEventListener = new UserProfilesEventListener(); $oUserProfilesEventListener->Init(); @@ -530,7 +530,7 @@ TXT; public function testInit_ConfWithFurtherWarningProfilesAndOneRepairment() { \MetaModel::GetConfig()->Set(UserProfilesEventListener::USERPROFILE_REPAIR_ITOP_PARAM_NAME, - ['Portal power user' => null, 'Ticket Manager' => null, 'Administrator' => "Configuration Manager"] + ['Portal power user' => null, 'Configuration Manager' => null, 'Administrator' => "Configuration Manager"] ); $oUserProfilesEventListener = new UserProfilesEventListener(); $oUserProfilesEventListener->Init(); @@ -546,7 +546,7 @@ TXT; $oUser->Set('language', 'EN US'); \MetaModel::GetConfig()->Set(UserProfilesEventListener::USERPROFILE_REPAIR_ITOP_PARAM_NAME, - ['Portal power user' => 'Ticket Manager'] + ['Portal power user' => 'Configuration Manager'] ); $oUserProfilesEventListener = new UserProfilesEventListener(); $oUserProfilesEventListener->Init(); @@ -561,7 +561,7 @@ TXT; $aProfilesAfterCreation[] = $oProfile->Get('profile'); } - $this->assertContains('Ticket Manager', $aProfilesAfterCreation, var_export($aProfilesAfterCreation, true)); + $this->assertContains('Configuration Manager', $aProfilesAfterCreation, var_export($aProfilesAfterCreation, true)); $this->assertContains('Portal power user', $aProfilesAfterCreation, var_export($aProfilesAfterCreation, true)); } @@ -569,8 +569,8 @@ TXT; { \MetaModel::GetConfig()->Set(UserProfilesEventListener::USERPROFILE_REPAIR_ITOP_PARAM_NAME, [ - 'Administrator' => 'Configuration Manager', - 'Portal power user' => 'Ticket Manager' + 'Administrator' => 'Portal user', + 'Portal power user' => 'Configuration Manager' ] ); $oUserProfilesEventListener = new UserProfilesEventListener(); @@ -591,7 +591,7 @@ TXT; $aProfilesAfterCreation[] = $oProfile->Get('profile'); } - $this->assertContains('Ticket Manager', $aProfilesAfterCreation, var_export($aProfilesAfterCreation, true)); + $this->assertContains('Configuration Manager', $aProfilesAfterCreation, var_export($aProfilesAfterCreation, true)); $this->assertContains('Portal power user', $aProfilesAfterCreation, var_export($aProfilesAfterCreation, true)); $oUser2 = new \UserLocal(); @@ -610,7 +610,7 @@ TXT; } $this->assertContains('Administrator', $aProfilesAfterCreation, var_export($aProfilesAfterCreation, true)); - $this->assertContains('Configuration Manager', $aProfilesAfterCreation, var_export($aProfilesAfterCreation, true)); + $this->assertContains('Portal user', $aProfilesAfterCreation, var_export($aProfilesAfterCreation, true)); } public function testUserCreationWithWarningMessageConf() From cc56413ee7874f59abe72936bdae9b07de985e5a Mon Sep 17 00:00:00 2001 From: odain Date: Tue, 5 Sep 2023 15:03:59 +0200 Subject: [PATCH 3/3] 5324-add debug logs --- .../src/UserProfilesEventListener.php | 43 +++++++++++++++---- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/datamodels/2.x/itop-profiles-itil/src/UserProfilesEventListener.php b/datamodels/2.x/itop-profiles-itil/src/UserProfilesEventListener.php index 6bfa0bfc9..d22e1e738 100644 --- a/datamodels/2.x/itop-profiles-itil/src/UserProfilesEventListener.php +++ b/datamodels/2.x/itop-profiles-itil/src/UserProfilesEventListener.php @@ -38,6 +38,7 @@ class UserProfilesEventListener implements iEventServiceSetup $this->Init(); if (false === $this->bIsRepairmentEnabled){ + IssueLog::Debug('UserProfilesEventListener bIsRepairmentEnabled disabled', LogChannels::DM_CRUD); return; } @@ -97,6 +98,7 @@ class UserProfilesEventListener implements iEventServiceSetup $iUserId = $oURP_UserProfile->Get('userid'); $oUser = \MetaModel::GetReentranceObjectByChildClass(\User::class, $iUserId); if (false !== $oUser){ + IssueLog::Debug('OnUserProfileEdition user already being edited', LogChannels::DM_CRUD); //user edition: handled by other event return; } @@ -104,6 +106,7 @@ class UserProfilesEventListener implements iEventServiceSetup $oUser = \MetaModel::GetObject(\User::class, $iUserId); $aChanges = $oURP_UserProfile->ListChanges(); if (array_key_exists('userid', $aChanges)) { + IssueLog::Debug('OnUserProfileEdition userid changed', LogChannels::DM_CRUD); $iUserId = $oURP_UserProfile->GetOriginal('userid'); $oPreviousUser = \MetaModel::GetObject(\User::class, $iUserId); @@ -112,13 +115,22 @@ class UserProfilesEventListener implements iEventServiceSetup $iCount = 0; $sSingleProfileName = null; while ($oCurrentURP_UserProfile = $oProfileLinkSet->Fetch()) { - if ($oCurrentURP_UserProfile->Get('userid') !== $oCurrentURP_UserProfile->GetOriginal('userid')) { + $sNewUserId = $oCurrentURP_UserProfile->Get('userid'); + $sOriginalUserId = $oCurrentURP_UserProfile->GetOriginal('userid'); + if ($sNewUserId !== $sOriginalUserId) { $sRemovedProfileId = $oCurrentURP_UserProfile->GetOriginal('profileid'); + IssueLog::Debug('OnUserProfileEdition profile moved does not count', LogChannels::DM_CRUD, [ + 'URP_UserProfile' => $oURP_UserProfile->GetKey(), + 'sRemovedProfileId' => $sRemovedProfileId, + 'sNewUserId' => $sNewUserId, + 'sOriginalUserId' => $sOriginalUserId, + ]); continue; } $iCount++; if ($iCount > 1){ + IssueLog::Debug('OnUserProfileEdition more than one user', LogChannels::DM_CRUD); //more than one profile: no repairment needed return; } @@ -126,6 +138,7 @@ class UserProfilesEventListener implements iEventServiceSetup } $this->RepairProfileChangesOrWarn($oPreviousUser, $sSingleProfileName, $oURP_UserProfile, $sRemovedProfileId); } else if (array_key_exists('profileid', $aChanges)){ + IssueLog::Debug('OnUserProfileEdition profileid changed', LogChannels::DM_CRUD); $oCurrentUserProfileSet = $oUser->Get('profile_list'); if ($oCurrentUserProfileSet->Count() === 1){ $oProfile = $oCurrentUserProfileSet->Fetch(); @@ -153,6 +166,7 @@ class UserProfilesEventListener implements iEventServiceSetup $iUserId = $oURP_UserProfile->Get('userid'); $oUser = \MetaModel::GetReentranceObjectByChildClass(\User::class, $iUserId); if (false !== $oUser){ + IssueLog::Debug('OnUserProfileLinkDeletion user being deleted already', LogChannels::DM_CRUD); //user edition: handled by other event return; } @@ -181,6 +195,7 @@ class UserProfilesEventListener implements iEventServiceSetup } $iCount++; if ($iCount > 1){ + IssueLog::Debug('OnUserProfileLinkDeletion more than one profile', LogChannels::DM_CRUD); //more than one profile: no repairment needed return; } @@ -217,6 +232,7 @@ class UserProfilesEventListener implements iEventServiceSetup //In that case, itop administrator has to specify it via itop configuration. we dont use default profiles repairment otherwise if (is_null($aNonStandaloneProfiles)){ if (count($aPortalDispatcherData) > 2){ + IssueLog::Debug('Init repairment disabled as there are more than 2 portals (extended customer should decide on their own)', LogChannels::DM_CRUD); $this->bIsRepairmentEnabled = false; return; } @@ -224,6 +240,7 @@ class UserProfilesEventListener implements iEventServiceSetup $aPortalNames = array_keys($aPortalDispatcherData); sort($aPortalNames); if ($aPortalNames !== ['backoffice', 'itop-portal']){ + IssueLog::Debug('Init repairment disabled there is a custom portal', LogChannels::DM_CRUD, [$aPortalNames]); $this->bIsRepairmentEnabled = false; return; } @@ -235,13 +252,14 @@ class UserProfilesEventListener implements iEventServiceSetup } if (! is_array($aNonStandaloneProfiles)){ - \IssueLog::Error(sprintf("%s is badly configured. it should be an array.", self::USERPROFILE_REPAIR_ITOP_PARAM_NAME), null, [self::USERPROFILE_REPAIR_ITOP_PARAM_NAME => $aNonStandaloneProfiles]); + \IssueLog::Error(sprintf("%s is badly configured. it should be an array.", self::USERPROFILE_REPAIR_ITOP_PARAM_NAME),LogChannels::DM_CRUD, [self::USERPROFILE_REPAIR_ITOP_PARAM_NAME => $aNonStandaloneProfiles]); $this->bIsRepairmentEnabled = false; return; } if (empty($aNonStandaloneProfiles)){ //Feature specifically disabled in itop configuration + IssueLog::Debug('Init repairment disabled by conf on purpose', LogChannels::DM_CRUD); $this->bIsRepairmentEnabled = false; return; } @@ -303,6 +321,7 @@ class UserProfilesEventListener implements iEventServiceSetup { $oCurrentUserProfileSet = $oUser->Get('profile_list'); if ($oCurrentUserProfileSet->Count() === 1){ + IssueLog::Debug('ValidateThenRepairOrWarn one profile found', LogChannels::DM_CRUD); $oProfile = $oCurrentUserProfileSet->Fetch(); $this->RepairUserChangesOrWarn($oUser, $oProfile->Get('profile')); @@ -310,12 +329,16 @@ class UserProfilesEventListener implements iEventServiceSetup } public function RepairUserChangesOrWarn(\User $oUser, string $sSingleProfileName) : void { + IssueLog::Debug('RepairUserChangesOrWarn', LogChannels::DM_CRUD, + [ + 'aNonStandaloneProfilesMap' => $this->aNonStandaloneProfilesMap, + 'sSingleProfileName' => $sSingleProfileName + ] + ); + if (array_key_exists($sSingleProfileName, $this->aNonStandaloneProfilesMap)) { $aRepairingProfileInfo = $this->aNonStandaloneProfilesMap[$sSingleProfileName]; if (is_null($aRepairingProfileInfo)){ - //Notify current user via session messages that there will be an issue - //Without preventing from commiting - //$oUser::SetSessionMessage(get_class($oUser), $oUser->GetKey(), 1, $sMessage, 'WARNING', 1); $sMessage = \Dict::Format("Class:User/NonStandaloneProfileWarning", $sSingleProfileName, $oUser->Get('friendlyname')); throw new \CoreCannotSaveObjectException(array('issues' => [$sMessage], 'class' => get_class($oUser), 'id' => $oUser->GetKey())); } else { @@ -332,6 +355,13 @@ class UserProfilesEventListener implements iEventServiceSetup } public function RepairProfileChangesOrWarn(\User $oUser, ?string $sSingleProfileName, \URP_UserProfile $oURP_UserProfile, string $sRemovedProfileId, $bIsRemoval=false) : void { + IssueLog::Debug('RepairUserChangesOrWarn', LogChannels::DM_CRUD, + [ + 'aNonStandaloneProfilesMap' => $this->aNonStandaloneProfilesMap, + 'sSingleProfileName' => $sSingleProfileName + ] + ); + if (is_null($sSingleProfileName)){ return; } @@ -341,9 +371,6 @@ class UserProfilesEventListener implements iEventServiceSetup if (is_null($aRepairingProfileInfo) || ($aRepairingProfileInfo['id'] === $sRemovedProfileId) //cannot repair by readding same remove profile as it will raise uniqueness rule ){ - //Notify current user via session messages that there will be an issue - //Without preventing from commiting - //$oURP_UserProfile::SetSessionMessage(get_class($oURP_UserProfile), $oURP_UserProfile->GetKey(), 1, $sMessage, 'WARNING', 1); $sMessage = \Dict::Format("Class:User/NonStandaloneProfileWarning", $sSingleProfileName, $oUser->Get('friendlyname')); if ($bIsRemoval){ $oURP_UserProfile->AddDeleteIssue($sMessage);