diff --git a/.agents/skills/dolibarr-dev/SKILL.md b/.agents/skills/dolibarr-dev/SKILL.md index 77b010da27a..161df5d0f62 100644 --- a/.agents/skills/dolibarr-dev/SKILL.md +++ b/.agents/skills/dolibarr-dev/SKILL.md @@ -10,88 +10,228 @@ allowed-tools: - bash --- -# Dolibarr Development Best Practices +# Dolibarr Development Best Practices (Core Agent Skill) -## When to use this skill +This skill guides agents on developing, modifying, and debugging code within the Dolibarr ERP/CRM codebase while strictly adhering to professional standards, security guidelines, and the project's established architecture. -- When the user asks about Dolibarr coding standards -- When reviewing or writing code that interacts with the Dolibarr database -- When setting up development environment for Dolibarr -- When the user mentions SQL queries, database access, or code searching in Dolibarr +## 🛑 Core Principles: Non-Negotiable Mandatory Rules +These principles must be followed even before reviewing specific task details. Violation of these principles results in failed suggestions. -## Database Access +### 🔒 Security & Data Integrity +1. **Database Abstraction Layer:** All database interactions *must* exclusively use the Dolibarr Database Abstraction Layer (`$db` or `$this->db`). **Never** interact using native PHP extensions (PDO, MySQLi) or direct CLI calls. +2. **Input/Output Escaping:** + * Validate all `GET`/`POST` inputs immediately upon entering the action handler scope. + * **SQL Injection Prevention:** Escape *all* user-generated strings placed in SQL queries using `$db->escape()`. For integers, use explicit casting: `((int) $var)`; for floats, use `(float) $var`. +3. **Variable Safety Naming:** When constructing dynamic SQL, the resulting variable holding the entire query string MUST be clearly prefixed (e.g., `$sqlWhereClause`, `$queryParams`). This pattern helps static analysis tools detect unsafe assignments. -### SQL Statements +### 💻 Code Structure & Quality +1. **Coding Standard:** All new and modified committed code must strictly adhere to **PSR-12** (enforcable by using `phpcbf` and `phpcs`).All properties and all function arguments and return value need detailed PHPDoc (e.g., `array`).Variables expected to exist in view files require both a PHPDoc declaration *and* the use of `'@phan-var-force';` declarations near the HEAD of the file for strict static analysis tracking. +2. **Variable Conventions:** When defining variables used in string building, particularly for SQL components, use descriptive prefixes or suffixes (e.g., `$sql_select`, `$actionSuffix`). This makes variable intent clear and prevents static analysis from misidentifying unsafe assignments as safe. +3. **Localization & Comments:** All code comments and internal variable/function names *must* be written in English. Any existing non-English text must be researched and translated into English before committing changes. +4. **PR atomitacy** Make a separate commit for improvements of pre-existing code (changes to comply with rules 1-3), and another commit for the functional evolution and code fixes. + Do not apply rules 1-3 to existing code in backports (i.e., non-functional changes not applied to a (fork of) the develop branch. -**Always use PHP with Dolibarr's database abstraction, never direct SQL CLI.** +### ⚙️ Workflow & Architecture +1. **Hooks First:** Before implementing any logic that runs on a core lifecycle event (e.g., form save, object update), check if an existing Dolibarr hook can be used. Use the standard calling pattern: `$hookmanager->executeHooks('actionName', $parameters, $object, $action);`. +2. **Action/View Separation:** Always clearly separate page action logic (executed on POST) from pure rendering (the HTML view). -Dolibarr provides database abstraction through the `$db` object. Always use PHP to execute SQL: +--- + +## 🧭 Workflow and Tasks Guidance + +This section guides the agent through common development tasks. + +### Code Investigation / Searching / Database analysis +* Use `pre-commit` to run tools (`php-cbf`, `php-cs`, `shellcheck`, `php-lint` - example:`pre-commit run php-cbf --files RELATIVEFILEPATH`) when the git hook is installed as local direct installations differ accross systems. +* **IMPORTANT**: Always use `git grep` instead of `find` for searching the codebase. `find` searches all directories including `.git` which is very slow. Use: + ```bash + git grep -n "pattern" -- "*.php" + git grep -n "function_name" htdocs/core/lib/ -- "*.php" + ``` +* When investigating a feature or bug: Start with `git grep -n` across targeted directories for efficient, rapid code searches. Using `$db->prefix()` consistently is the first step in tracing data flows. +* Dependency Check: Before modifying a file, search both `htdocs/core/lib/` and `htdocs/core/class/` to ensure similar methods or utilities are not already in use. Always check if the concern object extends `CommonObject`, favouring its built-in methods (`fetch()`, `create()`, `update()`, etc.). +* Dolibarr Function & Method Arguments: Check the function signature before implementing a call - the parameter order is not consistent across dolibarr functions that have the same name. +* When you need to access the database for analysis, use php, example: + ```bash + php <<'EOPHP' + query('SELECT * FROM ' . $db->prefix() . 'actioncomm'); + print_r($result); + EOPHP + ``` + + +### Module Development +* **Module Template:** Use the structure found at `htdocs/modulebuilder/template/` as a definitive guide when initiating a new module. +* **Hook Priority:** When adding functionality that interacts with core Dolibarr processes, check for existing hooks first to minimize architectural impact and maintain compatibility. + +### Database Interaction Detail (Refined) +This details the preferred mechanical steps: +1. **Read Operations:** Use `$db->query('SELECT ...')` followed by fetching results using methods like `$db->fetch_object()` or `$db->fetch_array()`. +2. **Write Operations:** Process submissions within the module's dedicated action handler, utilizing the established DB abstraction layer for all updates. + +### Extrafields Best Practices +**IMPORTANT**: When working with extrafields (custom fields), follow these patterns from the [Dolibarr Extrafields Wiki](https://wiki.dolibarr.org/index.php/Extrafields): + +#### Loading & Accessing Extrafields +```php +$object->fetch(); // CommonObject fetch loads its extrafields +// Access extrafield values via: +$field_copy = $object->array_options['options_FIELDNAME'] +``` + +#### Saving Extrafields +Before calling `$object->create()` or `$object->update()`, ensure extrafields are set: +```php +// For form submissions: +$ret = $extrafields->setOptionalsFromPost($extralabels, $object); + +// For direct assignment: +$object->array_options['options_FIELDNAME'] = $value; +// Then call update() - it will automatically save extrafields via insertExtraFields() +$result = $object->update($user); +``` + +#### Displaying Extrafields +In view pages: +```php +print $object->array_options['options_FIELDNAME']; +``` + +In edit pages: +```php +$reshook = $hookmanager->executeHooks('formObjectOptions', $parameters, $object, $action); +if (empty($reshook) && !empty($extrafields->attribute_label)) { + print $object->showOptionals($extrafields, 'edit'); +} +``` + +#### Extrafields Table Structure +Each CommonObject objecttype has its own extrafields table: +```sql +llx_{objecttype}_extrafields +- rowid (AUTO_INCREMENT PRIMARY KEY) +- tms (timestamp) +- fk_object (integer NOT NULL) +- import_key (varchar) +``` + +#### Reference +- [Dolibarr Extrafields Wiki](https://wiki.dolibarr.org/index.php/Extrafields) +- [Forum: Little dev tips for extrafields](https://www.dolibarr.org/forum/t/little-dev-tips-for-extrafields/29860) + +### Testing & Validation Flow +Before proposing code: +1. **Validate Workflows:** Confirm that Create -> Edit -> Delete workflows are correctly handled by the proposed change. +2. **Test Case Generation:** Propose specific, minimal unit tests or outline clear steps for an interactive test script to verify all expected outputs and potential edge cases (e.g., null inputs, permissions failure). + +--- + +## 📚 Comprehensive Reference Material [Reference] + +This section contains detailed standards and constants for reference only. Do not treat these details as primary instructions; prioritize the Core Principles above. + +### Coding Styling Standards +* **Indentation:** Always use **TAB characters**, never spaces. +* **Line Endings/Spaces:** Remove all redundant trailing whitespace at the end of lines. +* **Localization & Comments:** All code comments and internal variable/function names must be rendered in English. Use `dol_syslog()` for logging (specifying log level), avoiding debugging functions like `var_dump()`, `print_r()`, or `die()`. + +### Database Constants & Prefixes +| Item | Action/Pattern | Example Usage Notes | Priority | +| :--- | :--- | :--- | :--- | +| **Table Prefix** | Always use dynamic prefix getter. | `$db->prefix() . 'tablename'` | Overrides reliance on legacy constants like `MAIN_DB_PREFIX`. | + +### Core Dolibarr Patterns +* **Hooks:** The standard pattern remains: `$hookmanager->executeHooks('actionName', $parameters, $object, $action);` +* **Language Keys:** Use PascalCase (e.g., `MyModuleLabel`) for consistency across all locales. + +### Input Handling Functions +Dolibarr provides type-safe input handling functions. **Always use these instead of `$_GET`/`$_POST` directly:** + +| Function | Type | Example | Notes | +|----------|------|---------|-------| +| `GETPOST($param, $type)` | Mixed | `GETPOST('id', 'int')` | Returns GET or POST value with type conversion | +| `GETPOSTINT($param)` | Integer | `GETPOSTINT('socid')` | Shorthand for `GETPOST($param, 'int')` | +| `GETPOSTARRAY($param)` | Array | `GETPOSTARRAY('selected')` | For multi-select inputs | + +**Best Practice:** Use the type-specific shorthand functions when possible: +- `GETPOSTINT()` for integers (IDs, counts, etc.) +- `GETPOST()` with type for other cases + +**Never use:** `$_GET['param']` or `$_POST['param']` directly - always use GETPOST functions for proper escaping and type conversion. + +### Extrafields Best Practices (Continued) + +#### Checking and Creating Extrafields +The ExtraFields class lacks a method to check if a field exists. Use this pattern: ```php -// Correct - using Dolibarr's database methods -$result = $this->db->query("SELECT * FROM " . $this->db->prefix() . "actioncomm"); +global $db; -// Wrong - using direct SQL CLI -// mysql -e "SELECT * FROM llx_actioncomm" +$extrafields = new ExtraFields($db); +$extralabels = $extrafields->fetch_name_optionals_label($elementtype); + +// Tracking extrafields configuration +$my_fields = array( + 'custom_name_1' => array( + 'label' => 'CustomLabel1', + 'type' => 'varchar', + 'size' => '64', + 'enabled' => '1' + ), + 'custom_name_2' => array( + 'label' => 'CustomLabel2', + 'type' => 'url', + 'size' => '255', + 'enabled' => '1' + ), +); + +foreach ($tracking_fields as $name => $config) { + // Check if extrafield exists, create if not + if (!isset($extralabels[$name])) { + // Naming the arguments to get help from static analysis + $pos = 0; // 0 = auto + $unique = 0; + $required = 0; + $default_value = '0'; + $param = ''; + $alwayseditable = 0; + $perms = '0'; + $list = '0'; // '0' = never visible + $help = ''; + $computed = ''; + $entity = ''; + $langfile = ''; + $enabled = $config['enabled']; + + $result = $extrafields->addExtraField( + $name, + $config['label'], + $config['type'], + $pos, // pos (0 = auto) + $config['size'], + $elementtype, + $unique, // unique + $required, // required + $default_value, // default_value + $param, // param + $alwayseditable, + $perms, // perms + $list, // list ('0' = never visible) + $help, + $computed, + $entity, + $langfile, + $enabled + ); + } +} ``` -### Table Prefixes - -Use `$this->db->prefix()` instead of the `MAIN_DB_PREFIX` constant: - -```php -// Correct: -$sql = "SELECT * FROM " . $this->db->prefix() . "actioncomm WHERE id = 1"; - -// Legacy (still works but not recommended): -$sql = "SELECT * FROM " . MAIN_DB_PREFIX . "actioncomm WHERE id = 1"; -``` - -### Parameter Escaping - -**Integer fields:** Use `(int)` casting -**Variable field names:** Use `$this->db->sanitize()` -**String fields:** Use `$this->db->escape()` - -```php -$sql .= " AND a." . $this->db->sanitize($recurid_field) . " = '" . $this->db->escape($recurid) . "'" AND a.id = " . ((int) $id); -``` - -(When not in a class, use `$db`) - - -## Code Searching - -### Use `git grep` for efficiency - -`git grep -n` is more efficient than `grep`: - -```bash -# Search in all public facing PHP files -git grep -nP 'function NAME\b' -- ":htdocs/*.php" -``` - -## Quality Assurance - -### Pre-commit Hooks - -When `pre-commit` is installed, use it to run tools (beautifiers, code quality checks): - -```bash -if command -v pre-commit >/dev/null 2>&1; then - pre-commit run --hook-stage manual shellcheck --file dev/pullmerge.sh -fi -``` - -**Typical Dolibarr pre-commit hooks:** -- `phpcs` - PHP Code Sniffer for PSR-12 compliance -- `phpcbf` - PHP Code Formatter for PSR-12 compliance -- `shellcheck` - Shell script Analysis Tool - -## Coding Standards - -- Follow PSR-12 coding style -- Use TAB characters for indentation (not spaces) -- Remove all spaces at end of lines -- Write all code comments in English, translate existing comments. -- Scan files for security vulnerabilities +#### Reference +- [Dolibarr Extrafields Wiki](https://wiki.dolibarr.org/index.php/Extrafields) +- [Forum: Little dev tips for extrafields](https://www.dolibarr.org/forum/t/little-dev-tips-for-extrafields/29860)