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.

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

✨ Allow AttributeSet in XML sample file #654

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

GurneyHallack
Copy link
Contributor

In XML Data Loader, manage AttributeSet class and its children to be imported , in sample XML files.

Base information

Question Answer
Related to a SourceForge thead / Another PR / Combodo ticket? NA
Type of change? Enhancement

Symptom (bug) / Objective (enhancement)

In the setup (unattended or not), it's not possible to load extensions which contains sample XML file which contain AttributeEnumSet, AttributeClassAttCodeSet, AttributeQueryAttCodeSet or AttributeTagSet attribute, with values (not empty).
It generates an error :

Ok. 80 % done.
Loading data
Executing 'load-data'

Fatal error: Uncaught Error: Call to a member function Count() on string in /var/www/html/core/attributedef.class.inc.php on line 10832

Error: Call to a member function Count() on string in /var/www/html/core/attributedef.class.inc.php on line 10832

Call Stack:
    0.0009     618840   1. {main}() /var/www/html/setup/unattended-install/unattended-install.php:0
    0.2085   26345952   2. ApplicationInstaller->ExecuteAllSteps($bVerbose = ???, $sMessage = ???, $sInstallComment = ???) /var/www/html/setup/unattended-install/unattended-install.php:303
   11.4550   61076744   3. ApplicationInstaller->ExecuteStep($sStep = 'load-data', $sInstallComment = NULL) /var/www/html/setup/applicationinstaller.class.inc.php:117
   11.4551   61077480   4. ApplicationInstaller::DoLoadFiles($aSelectedModules = [0 => 'authent-cas', 1 => 'authent-external', 2 => 'authent-ldap', 3 => 'authent-local', 4 => 'combodo-backoffice-darkmoon-theme', 5 => 'itop-backup', 6 => 'itop-config', 7 => 'itop-files-information', 8 => 'itop-portal-base', 9 => 'itop-profiles-itil', 10 => 'itop-sla-computation', 11 => 'itop-structure', 12 => 'itop-welcome-itil', 13 => 'itop-config-mgmt', 14 => 'itop-attachments', 15 => 'itop-tickets', 16 => 'combodo-db-tools', 17 => 'itop-core-update', 18 => 'itop-hub-connector', 19 => 'itop-oauth-client', 20 => 'itop-themes-compat', 21 => 'itop-bridge-cmdb-ticket', 22 => 'itop-service-mgmt', 23 => 'itop-bridge-cmdb-services', 24 => 'AttributeClassAttCodeSet'], $sModulesDir = 'env-production', $aParamValues = ['mode' => 'install', 'db_server' => 'mariadb', 'db_user' => 'root', 'db_pwd' => 'password', 'db_name' => 'itop', 'new_db_name' => 'itop', 'db_prefix' => '', 'db_tls_enabled' => '', 'db_tls_ca' => '', 'application_path' => 'http://localhost:8080/', 'language' => 'EN US', 'graphviz_path' => '/usr/bin/dot', 'source_dir' => 'datamodels/2.x/'], $sTargetEnvironment = 'production', $bOldAddon = '', $bSampleData = TRUE) /var/www/html/setup/applicationinstaller.class.inc.php:356
   11.4568   61215840   5. RunTimeEnvironment->LoadData($aAvailableModules = ['_Root_' => ['version_db' => '', 'name_db' => '', 'version_code' => '3.2.0-dev-svn', 'name_code' => 'iTop'], 'AttributeClassAttCodeSet' => ['label' => 'AttributeClassAttCodeSet', 'category' => 'admins', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/AttributeClassAttCodeSet', 'module_file' => '/var/www/html/env-production/AttributeClassAttCodeSet/module.AttributeClassAttCodeSet.php', 'version_db' => '', 'version_code' => '1.0.0', 'install' => [...]], 'authent-cas' => ['label' => 'CAS SSO', 'category' => 'authentication', 'dependencies' => [...], 'mandatory' => TRUE, 'visible' => TRUE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/authent-cas', 'module_file' => '/var/www/html/env-production/authent-cas/module.authent-cas.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'authent-external' => ['label' => 'External user authentication', 'category' => 'authentication', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'datamodel' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/authent-external', 'module_file' => '/var/www/html/env-production/authent-external/module.authent-external.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'authent-ldap' => ['label' => 'User authentication based on LDAP', 'category' => 'authentication', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'installer' => 'AuthentLDAPInstaller', 'datamodel' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/authent-ldap', 'module_file' => '/var/www/html/env-production/authent-ldap/module.authent-ldap.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'authent-local' => ['label' => 'User authentication based on the local DB', 'category' => 'authentication', 'dependencies' => [...], 'mandatory' => TRUE, 'visible' => TRUE, 'datamodel' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/authent-local', 'module_file' => '/var/www/html/env-production/authent-local/module.authent-local.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'combodo-backoffice-darkmoon-theme' => ['label' => 'Backoffice: Darkmoon theme', 'category' => 'business', 'dependencies' => [...], 'mandatory' => TRUE, 'visible' => FALSE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/combodo-backoffice-darkmoon-theme', 'module_file' => '/var/www/html/env-production/combodo-backoffice-darkmoon-theme/module.combodo-backoffice-darkmoon-theme.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-attachments' => ['label' => 'Tickets Attachments', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'installer' => 'AttachmentInstaller', 'datamodel' => [...], 'webservice' => [...], 'dictionary' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-attachments', 'module_file' => '/var/www/html/env-production/itop-attachments/module.itop-attachments.php', 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-backup' => ['label' => 'Backup utilities', 'category' => 'Application management', 'dependencies' => [...], 'mandatory' => TRUE, 'visible' => FALSE, 'datamodel' => [...], 'webservice' => [...], 'dictionary' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-backup', 'module_file' => '/var/www/html/env-production/itop-backup/module.itop-backup.php', 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-config' => ['label' => 'Configuration editor', 'category' => 'Application management', 'dependencies' => [...], 'mandatory' => TRUE, 'visible' => FALSE, 'datamodel' => [...], 'webservice' => [...], 'dictionary' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-config', 'module_file' => '/var/www/html/env-production/itop-config/module.itop-config.php', 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-files-information' => ['label' => 'iTop files information', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => FALSE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-files-information', 'module_file' => '/var/www/html/env-production/itop-files-information/module.itop-files-information.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-portal-base' => ['label' => 'Portal Development Library', 'category' => 'Portal', 'dependencies' => [...], 'mandatory' => TRUE, 'visible' => FALSE, 'datamodel' => [...], 'webservice' => [...], 'dictionary' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-portal-base', 'module_file' => '/var/www/html/env-production/itop-portal-base/module.itop-portal-base.php', 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-profiles-itil' => ['label' => 'Create standard ITIL profiles', 'category' => 'create_profiles', 'dependencies' => [...], 'mandatory' => TRUE, 'visible' => FALSE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-profiles-itil', 'module_file' => '/var/www/html/env-production/itop-profiles-itil/module.itop-profiles-itil.php', 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-sla-computation' => ['label' => 'SLA Computation', 'category' => 'sla', 'dependencies' => [...], 'mandatory' => TRUE, 'visible' => FALSE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-sla-computation', 'module_file' => '/var/www/html/env-production/itop-sla-computation/module.itop-sla-computation.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-structure' => ['label' => 'Core iTop Structure', 'category' => 'business', 'dependencies' => [...], 'mandatory' => TRUE, 'visible' => FALSE, 'installer' => 'StructureInstaller', 'datamodel' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-structure', 'module_file' => '/var/www/html/env-production/itop-structure/module.itop-structure.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-themes-compat' => ['label' => 'Light grey and Test red themes compatibility', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-themes-compat', 'module_file' => '/var/www/html/env-production/itop-themes-compat/module.itop-themes-compat.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-tickets' => ['label' => 'Tickets Management', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'installer' => 'TicketsInstaller', 'datamodel' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => 'https://www.itophub.io/wiki/page?id=3_2_0:admin:cron', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-tickets', 'module_file' => '/var/www/html/env-production/itop-tickets/module.itop-tickets.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-welcome-itil' => ['label' => 'ITIL skin', 'category' => 'skin', 'dependencies' => [...], 'mandatory' => TRUE, 'visible' => FALSE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-welcome-itil', 'module_file' => '/var/www/html/env-production/itop-welcome-itil/module.itop-welcome-itil.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'combodo-db-tools' => ['label' => 'Database maintenance tools', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/combodo-db-tools', 'module_file' => '/var/www/html/env-production/combodo-db-tools/module.combodo-db-tools.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-config-mgmt' => ['label' => 'Configuration Management (CMDB)', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'installer' => 'ConfigMgmtInstaller', 'datamodel' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-config-mgmt', 'module_file' => '/var/www/html/env-production/itop-config-mgmt/module.itop-config-mgmt.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-core-update' => ['label' => 'iTop Core Update', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-core-update', 'module_file' => '/var/www/html/env-production/itop-core-update/module.itop-core-update.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-hub-connector' => ['label' => 'iTop Hub Connector', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-hub-connector', 'module_file' => '/var/www/html/env-production/itop-hub-connector/module.itop-hub-connector.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-oauth-client' => ['label' => 'OAuth 2.0 client', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-oauth-client', 'module_file' => '/var/www/html/env-production/itop-oauth-client/module.itop-oauth-client.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-service-mgmt' => ['label' => 'Service Management', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => TRUE, 'installer' => 'ServiceMgmtInstaller', 'datamodel' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-service-mgmt', 'module_file' => '/var/www/html/env-production/itop-service-mgmt/module.itop-service-mgmt.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-bridge-cmdb-services' => ['label' => 'Bridge for CMDB and Services', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => FALSE, 'auto_select' => 'SetupInfo::ModuleIsSelected("itop-config-mgmt") && (SetupInfo::ModuleIsSelected("itop-service-mgmt") || SetupInfo::ModuleIsSelected("itop-service-mgmt-provider")) ', 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-bridge-cmdb-services', 'module_file' => '/var/www/html/env-production/itop-bridge-cmdb-services/module.itop-bridge-cmdb-services.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]], 'itop-bridge-cmdb-ticket' => ['label' => 'Bridge for CMDB and Ticket', 'category' => 'business', 'dependencies' => [...], 'mandatory' => FALSE, 'visible' => FALSE, 'auto_select' => 'SetupInfo::ModuleIsSelected("itop-config-mgmt") && SetupInfo::ModuleIsSelected("itop-tickets") ', 'datamodel' => [...], 'webservice' => [...], 'data.struct' => [...], 'data.sample' => [...], 'doc.manual_setup' => '', 'doc.more_information' => '', 'settings' => [...], 'itop_version' => '1.0.2', 'root_dir' => '/var/www/html/env-production/itop-bridge-cmdb-ticket', 'module_file' => '/var/www/html/env-production/itop-bridge-cmdb-ticket/module.itop-bridge-cmdb-ticket.php', 'dictionary' => [...], 'version_db' => '', 'version_code' => '3.2.0', 'install' => [...]]], $aSelectedModules = [0 => 'authent-cas', 1 => 'authent-external', 2 => 'authent-ldap', 3 => 'authent-local', 4 => 'combodo-backoffice-darkmoon-theme', 5 => 'itop-backup', 6 => 'itop-config', 7 => 'itop-files-information', 8 => 'itop-portal-base', 9 => 'itop-profiles-itil', 10 => 'itop-sla-computation', 11 => 'itop-structure', 12 => 'itop-welcome-itil', 13 => 'itop-config-mgmt', 14 => 'itop-attachments', 15 => 'itop-tickets', 16 => 'combodo-db-tools', 17 => 'itop-core-update', 18 => 'itop-hub-connector', 19 => 'itop-oauth-client', 20 => 'itop-themes-compat', 21 => 'itop-bridge-cmdb-ticket', 22 => 'itop-service-mgmt', 23 => 'itop-bridge-cmdb-services', 24 => 'AttributeClassAttCodeSet'], $bSampleData = TRUE) /var/w2024-06-25T20:09:09.639491138Z ww/html/setup/applicationinstaller.class.inc.php:992
   11.4571   61219192   6. XMLDataLoader->LoadFile($sFilePath = '/var/www/html/env-production/AttributeClassAttCodeSet/data.sample.AttributeClassAttCodeSet.xml', $bUpdateKeyCacheOnly = ???, $bSearch = ???) /var/www/html/setup/runtimeenv.class.inc.php:1202
   11.4576   61222656   7. DBObject->CheckValue($sAttCode = 'target_attcode', $value = 'name') /var/www/html/setup/xmldataloader.class.inc.php:294
   11.4576   61222656   8. AttributeSet->IsNull($proposedValue = 'name') /var/www/html/core/dbobject.class.php:2043

Reproduction procedure (bug)

  1. On iTop "develop"
  2. With PHP 8.3.9
  3. Not installed yet
  4. Modify datamodels itop-structure, in data.sample.persons.xml, the following code :
  5. <TriggerOnObjectUpdate alias="TriggerOnObjectUpdate" id="4"> <description>Contact updated</description> <action_list></action_list> <context>GUI:Console</context> <complement>class restriction: Contact</complement> <subscription_policy>allow_no_channel</subscription_policy> <target_class>Contact</target_class> <filter>SELECT `Person` FROM Person AS `Person` WHERE ((`status` = &apos;active&apos;) AND ((`org_id` = :current_contact-&gt;org_id) OR (`org_id` = :this-&gt;org_id)))</filter> <target_attcodes>email</target_attcodes> <finalclass>TriggerOnObjectUpdate</finalclass> <friendlyname>Contact updated</friendlyname> </TriggerOnObjectUpdate>
  6. Launch setup with sample data

Cause (bug)

Function IsNull of AttributeSet assumes that $proposedValue is a ormSet. However, in XMLDataloader, we send a string.
Thus, the function Count, which doesn't exist in string, generates an exception.

Proposed solution (bug and enhancement)

I added in XMLDataLoader a elseif to manage AttributeSet class and its children.
I didn't change the function IsNull of the AttributeSet, to check if the $proposedValue is of class ormSet.

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have tested all changes I made on an iTop instance
  • I have added a unit test, otherwise I have explained why I couldn't
  • Is the PR clear and detailed enough so anyone can understand digging in the code?

Checklist of things to do before PR is ready to merge

In XML Data Loader, manage AttributeSet class and its children to be imported , in sample XML files.
Copy link
Contributor

@Hipska Hipska left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good!

IMHO wouldn't it be better to remove the elseif section and just add the $value = $oAttDef->MakeRealValue($value, $oTargetObj); part in the existing else?

@Molkobain Molkobain added bug Something isn't working Setup Related to the setup process (install / upgrade) labels Jul 15, 2024
@Molkobain Molkobain self-assigned this Jul 15, 2024
@Molkobain
Copy link
Contributor

Hello Guy,

Thanks for the PR and for the unit test! Seems like a good idea.
Like @Hipska I'm wondering if we could reduce the code duplication, will be discussed during technical review.

@GurneyHallack
Copy link
Contributor Author

Hello,
I did it first like this, as I wanted to use sample data for an extension with an AttributeClassAttCodeSet, and also because there was already an elseif for AttributeTagSet 😁.
I think indeed it would be better like you said, but i need to update the unit test to check for all Attributes classes available and writable (like AttributeCaseLog).

@Molkobain Molkobain requested a review from rquetiez July 31, 2024 07:18
@Molkobain Molkobain removed their assignment Aug 19, 2024
Copy link
Contributor

@Molkobain Molkobain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technical review: The actual fix would be to fix the general else case and remove the TODO:

  • Remove the existing elseif for AttributeTagSet (and your elseif)
  • In the final else
    • Replace $res = $oTargetObj->CheckValue($sAttCode, $value);
    • With $res = $oTargetObj->CheckValue($sAttCode, $oAttDef->MakeRealValue($value, $oTargetObj));

@jf-cbd jf-cbd added this to the 3.2.1 milestone Sep 10, 2024
@jf-cbd
Copy link
Contributor

jf-cbd commented Sep 10, 2024

Accepted in functional review.

Thanks @GurneyHallack for the contribution, we'll make the last edits.

@GurneyHallack
Copy link
Contributor Author

GurneyHallack commented Sep 10, 2024

Hello,
I'm sorry for the delay on feedback on this one, I didn't took the time needed during the summer.

If we change the "else", ideally, we would need to check for every type of Attributes possibles, and with empty or not empty values.

In the XMLDataLoaderTest.php, there is :

  • Found with values
    • AttributeString
    • AttributeEnum
    • AttributeFriendlyName
    • AttributeFinalClass
    • AttributeObsolescenceFlag
  • Added with current PR
    • AttributeEnumSet
    • AttributeClassAttCodeSet
    • AttributeClass
    • AttributeOQL
  • Found but empty
    • AttributeEmailAddress
    • AttributePhoneNumber
    • AttributeDashboard
    • AttributeLinkedSet
    • AttributeLinkedSetIndirect
    • AttributeObsolescenceDate
    • AttributeHierarchicalKey
    • AttributeExternalKey
    • AttributeExternalField
    • AttributeText
    • AttributeImage
  • Missing but could be found in XML classes declarations
    • AttributeIPAddress (eg.: DatacenterDevice)
    • AttributeInteger (eg.: DatacenterDevice)
    • AttributeTagSet (eg.: FAQ FAQ-light)
    • AttributeRedundancySettings (eg.: DatacenterDevice)
    • AttributeHTML (eg.: FAQ FAQ-light)
    • AttributeStopWatch (eg.: UserRequest)
    • AttributeSubItem (eg. UserRequest)
    • AttributeDateTime (eg.: UserRequest)
    • AttributeBlob (eg.: Attachment)
    • AttributeURL (eg.: DocumentWeb)
    • AttributeDate (eg.: Contract)
    • AttributeDuration (eg.: UserRequest)
    • AttributeMetaEnum (eg.: Ticket, with UserRequest)
    • AttributeCaseLog (eg.: UserRequest)
    • AttributePassword (eg.: OAuthClient (abstract))
    • AttributeBoolean (eg.: lnkActionNotificationToContact)
    • AttributeObjectKey (eg.: Attachment)
    • AttributeDecimal (eg.: IPInterface)
  • Missing but could be found in PHP classes declarations
    • AttributePropertySet (eg.: appUserPreferences)
    • AttributeTable (eg.: EventNotificationEmail)
    • AttributeOneWayPassword (eg.: UserLocal, from UserInternal)
    • AttributeTemplateHTML (eg.: ActionEmail)
    • AttributeTemplateString (eg.: ActionEmail)
    • AttributeLongText (eg.: AsyncSendEmail)
    • AttributeEncryptedString (eg.: CMDBChangeOpSetAttributeEncrypted)
    • AttributeApplicationLanguage (eg.: ActionNotification (abstract), which EventNotificationEmail inherites)
    • AttributeClassState(eg.: TriggerOnStateChange (asbtract))
  • Missing and I didn't found them in iTop's classes (PHP nor XML):
    • AttributePercentage
    • AttributeDBFieldVoid (normal as it's basic, but it's not abstract)
    • AttributeDBField (normal as it's basic, but it's not abstract)
    • AttributeTemplateText
    • AttributeDeadline
    • AttributeQueryAttCodeSet
    • AttributeCustomFields
    • AttributeArchiveFlag (magic with archive xml, but didn't check well enough yet to test it)
    • AttributeArchiveDate (magic with archive xml, but didn't check well enough yet to test it)

@jf-cbd jf-cbd self-requested a review October 18, 2024 08:20
@jf-cbd jf-cbd self-assigned this Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Setup Related to the setup process (install / upgrade)
Projects
Status: Pending Combodo update
Development

Successfully merging this pull request may close these issues.

4 participants