vignesh/dolibarr
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

NEW: ModuleBuilder - consistent status labels and explicit trigger naming (#38883)

Browse source 
* NEW : Derive ModuleBuilder status labels from arrayofkeyval

LibStatut() in the ModuleBuilder object template hardcoded the
Draft/Enabled/Disabled labels, which diverged from the labels defined
in the 'status' field arrayofkeyval used by the list filter and the
select. Build labelStatus/labelStatusShort from arrayofkeyval so the
badge, the filter and the select all show the same configurable labels.

Add ModuleBuilderTemplateConventionsTest covering the derivation.

Next: normalize trigger codes to MYMODULE_MYOBJECT_ACTION in class line 637 + typo line 50, sql/data.sql lines 27/30-32, myobject_card.php line 378; extend the test with trigger assertions.

* NEW : Use explicit MODULE_OBJECT_ACTION trigger naming in template

The ModuleBuilder object template emitted the validate trigger as the
generic MYOBJECT_VALIDATE, inconsistent with the MYMODULE_MYOBJECT
prefix already advertised by $TRIGGER_PREFIX and used by the
unvalidate/cancel/reopen/sentbymail trigger codes. Normalize the
validate trigger, the seed data.sql agenda triggers and the close
notification example to the MYMODULE_MYOBJECT_<ACTION> policy so
generated modules expose clear, non-ambiguous trigger codes. Also fix a
typo in the $TRIGGER_PREFIX comment.

Extend ModuleBuilderTemplateConventionsTest with trigger naming checks.

Next: ChangeLog entry covering both features, then dolibarr-audit + functional verification, then push to remote quentin and run pr-review-v2.

* DOC : Add ChangeLog entries for ModuleBuilder status labels and triggers

Next: dolibarr-audit on both features, then functional verification, then push to remote quentin and run pr-review-v2.

* FIX : Guard ModuleBuilder LibStatut against undefined status key

Hardening from audit: when LibStatut() is called with a status value
that is not present in the 'status' field arrayofkeyval, reading
labelStatus/labelStatusShort raised an undefined-array-key warning under
PHP 8. Default both labels to an empty string, which dolGetStatus
already accepts, so the badge degrades gracefully to the status code.

Next: push branch to remote quentin and run dolibarr-pr-review-v2 on the full diff vs develop.

* CHORE Drop the files CONTRIBUTING forbids to edit in a PR [skip-claudemd]

ChangeLog is generated from the commit messages at release time, and the
language files other than en_US are synced from Transifex.

---------

Co-authored-by: Laurent Destailleur <eldy@destailleur.fr>
Co-authored-by: Alexandre SPANGARO <aspangaro.dolibarr@gmail.com>
This commit is contained in:
VIAL-GOUTEYRON Quentin 2026-08-17 21:41:18 +02:00 committed by GitHub
parent cac0e048ca
commit e837854094
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 136 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
htdocs/modulebuilder/template/class/myobject.class.php
View file

@ -47,7 +47,7 @@ class MyObject extends CommonObject
* @var string Prefix to check for any trigger code of any business class to prevent bad value for trigger code.
* @see CommonTrigger::call_trigger()
*/
public $TRIGGER_PREFIX = 'MYMODULE_MYOBJECT'; // Will be used to build trgiger keys 'MYMODULE_MYOBJECT_MODIFY', ...
public $TRIGGER_PREFIX = 'MYMODULE_MYOBJECT'; // Will be used to build trigger keys 'MYMODULE_MYOBJECT_MODIFY', ...
/**
* @var string Name of table without prefix where object is stored. This is also the key used for extrafields management (so extrafields know the link to the parent table).
@ -636,7 +636,7 @@ class MyObject extends CommonObject
if (!$error && !$notrigger) {
// Call trigger
$result = $this->call_trigger('MYOBJECT_VALIDATE', $user);
$result = $this->call_trigger('MYMODULE_MYOBJECT_VALIDATE', $user);
if ($result < 0) {
$error++;
}
@ -1019,12 +1019,14 @@ class MyObject extends CommonObject
if (empty($this->labelStatus) || empty($this->labelStatusShort)) {
global $langs;
//$langs->load("mymodule@mymodule");
$this->labelStatus[self::STATUS_DRAFT] = $langs->transnoentitiesnoconv('Draft');
$this->labelStatus[self::STATUS_VALIDATED] = $langs->transnoentitiesnoconv('Enabled');
$this->labelStatus[self::STATUS_CANCELED] = $langs->transnoentitiesnoconv('Disabled');
$this->labelStatusShort[self::STATUS_DRAFT] = $langs->transnoentitiesnoconv('Draft');
$this->labelStatusShort[self::STATUS_VALIDATED] = $langs->transnoentitiesnoconv('Enabled');
$this->labelStatusShort[self::STATUS_CANCELED] = $langs->transnoentitiesnoconv('Disabled');
// Build status labels from the 'status' field arrayofkeyval so that the badge (LibStatut),
// the list filter and the select all show the same, configurable labels.
if (!empty($this->fields['status']['arrayofkeyval']) && is_array($this->fields['status']['arrayofkeyval'])) {
foreach ($this->fields['status']['arrayofkeyval'] as $statuskey => $statuslabel) {
$this->labelStatus[$statuskey] = $langs->transnoentitiesnoconv($statuslabel);
$this->labelStatusShort[$statuskey] = $langs->transnoentitiesnoconv($statuslabel);
}
}
}
$statusType = 'status'.$status;
@ -1033,7 +1035,7 @@ class MyObject extends CommonObject
$statusType = 'status6';
}
return dolGetStatus($this->labelStatus[$status], $this->labelStatusShort[$status], '', $statusType, $mode, '', $paramsBadge);
return dolGetStatus($this->labelStatus[$status] ?? '', $this->labelStatusShort[$status] ?? '', '', $statusType, $mode, '', $paramsBadge);
}
/**

2
htdocs/modulebuilder/template/myobject_card.php
View file

@ -375,7 +375,7 @@ if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'crea
// require_once DOL_DOCUMENT_ROOT . '/core/class/notify.class.php';
// $notify = new Notify($db);
// $text .= '<br>';
// $text .= $notify->confirmMessage('MYOBJECT_CLOSE', $object->socid, $object);
// $text .= $notify->confirmMessage('MYMODULE_MYOBJECT_CLOSE', $object->socid, $object);
// }
// $formquestion = array();

8
htdocs/modulebuilder/template/sql/data.sql
View file

@ -24,9 +24,9 @@
-- new types of automatic events to record in agenda
-- 'code' must be a value matching 'MYOBJECT_ACTION'
-- 'code' must be a value matching 'MYMODULE_MYOBJECT_ACTION'
-- 'elementtype' must be value 'mymodule' ('myobject@mymodule' may be possible but should not be required)
--insert into llx_c_action_trigger (code,label,description,elementtype,rang) values ('MYOBJECT_VALIDATE','MyObject validated','Executed when myobject is validated', 'myobject@mymodule', 1000);
--insert into llx_c_action_trigger (code,label,description,elementtype,rang) values ('MYOBJECT_UNVALIDATE','MyObject unvalidated','Executed when myobject is unvalidated', 'myobject@mymodule', 1001);
--insert into llx_c_action_trigger (code,label,description,elementtype,rang) values ('MYOBJECT_DELETE','MyObject deleted','Executed when myobject deleted', 'myobject@mymodule', 1004);
--insert into llx_c_action_trigger (code,label,description,elementtype,rang) values ('MYMODULE_MYOBJECT_VALIDATE','MyObject validated','Executed when myobject is validated', 'myobject@mymodule', 1000);
--insert into llx_c_action_trigger (code,label,description,elementtype,rang) values ('MYMODULE_MYOBJECT_UNVALIDATE','MyObject unvalidated','Executed when myobject is unvalidated', 'myobject@mymodule', 1001);
--insert into llx_c_action_trigger (code,label,description,elementtype,rang) values ('MYMODULE_MYOBJECT_DELETE','MyObject deleted','Executed when myobject deleted', 'myobject@mymodule', 1004);

120
test/phpunit/ModuleBuilderTemplateConventionsTest.php Normal file
View file

@ -0,0 +1,120 @@
<?php
/* Copyright (C) 2026 Quentin VIAL--GOUTEYRON <quentin.vial-gouteyron@atm-consulting.fr>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
/**
* \file test/phpunit/ModuleBuilderTemplateConventionsTest.php
* \ingroup modulebuilder
* \brief PHPUnit test for ModuleBuilder template conventions: status labels derived from
* arrayofkeyval, and normalized trigger naming (MYMODULE_MYOBJECT_ACTION).
*/
global $conf, $user, $langs, $db;
require_once dirname(__FILE__).'/../../htdocs/master.inc.php';
require_once dirname(__FILE__).'/CommonClassTest.class.php';
if (empty($user->id)) {
print "Load permissions for admin user nb 1\n";
$user->fetch(1);
$user->loadRights();
}
$conf->global->MAIN_DISABLE_ALL_MAILS = 1;
/**
* Class ModuleBuilderTemplateConventionsTest
*
* @backupGlobals disabled
* @backupStaticAttributes enabled
* @remarks backupGlobals must be disabled to have db,conf,user and lang not erased.
* @phan-file-suppress PhanUndeclaredExtendedClass
* @phan-file-suppress PhanUndeclaredClass
* @phan-file-suppress PhanUndeclaredMethod
*/
class ModuleBuilderTemplateConventionsTest extends CommonClassTest
{
/**
* @var string Absolute path to the object class template.
*/
const CLASS_TPL = __DIR__.'/../../htdocs/modulebuilder/template/class/myobject.class.php';
/**
* @var string Absolute path to the object card template.
*/
const CARD_TPL = __DIR__.'/../../htdocs/modulebuilder/template/myobject_card.php';
/**
* @var string Absolute path to the seed data SQL template.
*/
const DATA_SQL = __DIR__.'/../../htdocs/modulebuilder/template/sql/data.sql';
/**
* getLibStatut() must use the label defined in the status field arrayofkeyval, not a hardcoded one.
*
* @return void
*/
public function testLibStatutLabelsMatchArrayofkeyval()
{
global $db;
require_once self::CLASS_TPL;
$object = new MyObject($db);
$expected = $object->fields['status']['arrayofkeyval'][MyObject::STATUS_VALIDATED];
$badge = strip_tags($object->LibStatut(MyObject::STATUS_VALIDATED, 1));
$this->assertStringContainsString($expected, $badge, 'Validated badge label should match arrayofkeyval label');
$this->assertStringNotContainsString('Enabled', $badge, 'Validated badge must not show the hardcoded "Enabled" label');
}
/**
* The LibStatut block must no longer hardcode Enabled/Disabled labels.
*
* @return void
*/
public function testNoHardcodedEnabledDisabledInLibStatut()
{
$content = file_get_contents(self::CLASS_TPL);
$this->assertSame(0, preg_match('/labelStatus(Short)?\[[^\]]+\]\s*=\s*\$langs->transnoentitiesnoconv\(\'(Enabled|Disabled)\'\)/', $content), 'Hardcoded Enabled/Disabled label assignment still present in LibStatut');
}
/**
* The validate trigger code must carry the module prefix (policy MYMODULE_MYOBJECT_ACTION).
*
* @return void
*/
public function testValidateTriggerHasModulePrefix()
{
$content = file_get_contents(self::CLASS_TPL);
$this->assertStringContainsString("call_trigger('MYMODULE_MYOBJECT_VALIDATE'", $content, 'Validate trigger must use MYMODULE_MYOBJECT_VALIDATE');
$this->assertSame(0, preg_match("/call_trigger\('MYOBJECT_VALIDATE'/", $content), 'Legacy unprefixed MYOBJECT_VALIDATE trigger still present');
}
/**
* No legacy unprefixed trigger code (MYOBJECT_<ACTION>) must remain in the templates.
* Configuration constants (ADDON_PDF, DRAFT_WATERMARK, QUICKSEARCH_ON_FIELDS) are not triggers
* and are excluded by the action whitelist below.
*
* @return void
*/
public function testNoLegacyTriggerCodeInTemplates()
{
$legacy = '/(?<![A-Z_])MYOBJECT_(VALIDATE|UNVALIDATE|CANCEL|REOPEN|DELETE|SENTBYMAIL|CLOSE|MODIFY|CREATE)\b/';
foreach (array(self::CLASS_TPL, self::CARD_TPL, self::DATA_SQL) as $tpl) {
$content = file_get_contents($tpl);
$this->assertSame(0, preg_match($legacy, $content), 'Legacy unprefixed trigger code found in '.basename($tpl));
}
}
}