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

Enhancements and Optimizations for the Module Builder #32460

Open
4 of 21 tasks
atm-quentin opened this issue Dec 23, 2024 · 4 comments
Open
4 of 21 tasks

Enhancements and Optimizations for the Module Builder #32460

atm-quentin opened this issue Dec 23, 2024 · 4 comments
Labels
Feature request This is a feature request

Comments

@atm-quentin
Copy link
Contributor

atm-quentin commented Dec 23, 2024

Feature Request

Enhancements and Optimizations for the Module Builder

A series of improvements to make the Module Builder more robust, user-friendly, and capable of generating clean, maintainable, and modern code.

Use case

The Module Builder is a key tool for developers to quickly generate Dolibarr modules and objects. However, the current implementation has some limitations:

  1. Lack of strict typing and PHPDoc consistency in generated code, reducing readability and increasing debugging time.
    
  2. Redundant or unnecessary code generation when certain features (e.g., statuses, lines, or tabs) are not needed.
    
  3. Inadequate handling of modern Dolibarr features, such as Multicompany compatibility and database key management.
    
  4. Presence of placeholders (xxx or myobject) in generated code, which require manual cleanup.
    
  5. Limited configuration options for essential features like menus, rights, and actions on object cards.
    
  6. Issues with event handling for object creation and track IDs for email management.
    

By addressing these limitations, the Module Builder can generate code that is cleaner, more maintainable, and better aligned with Dolibarr's core practices.

Suggested implementation

  • Introduce strict typing for methods and enforce parameter and return types where applicable.
  • Automatically generate comprehensive PHPDoc comments for methods, including parameter types, return types, and possible exceptions.
  • Replace dol_include_once with the DIR constant for consistent path management.
  • Add configuration options in the interface for:
    1. Managing lines or statuses in objects.
    2. Selecting tabs, actions, and menus to include in the generated code.
    3. Dynamically naming statuses.
  • Enhance Multicompany compatibility by handling shared or non-shared references.
  • Use $db->prefix()
  • Generate meaningful trigger names and remove placeholders like xxx and myobject in the output.
  • Optimize database schema generation by offering foreign keys, index creation, and NOT NULL constraints where applicable.
  • Update event handling for object creation and ensure correct track ID generation for emails.
  • Improve access rights management by allowing updates through the configuration interface.

Suggested steps

  • Review the current Module Builder code to identify all instances of outdated practices (e.g., dol_include_once, placeholders, etc.).
  • Define and implement strict typing and PHPDoc templates for generated methods.
  • Update the Module Builder interface to include new configuration options for lines, statuses, tabs, menus, and actions.
  • Implement conditional code generation to avoid unnecessary features based on user choices.
  • Replace static constants like MAIN_DB_PREFIX with $db->prefix() for database prefix management.
  • Add logic to handle database schema improvements (foreign keys, indexes).
  • Test changes on objects with complex configurations (e.g., Multicompany, lines, statuses).
  • Ensure backward compatibility with existing modules generated by the current Module Builder.

Proposed Improvements for the Dolibarr Module Builder

  • 1. Method Typing and Return Types

Action: Add strict typing to all methods (PHP 7.4+ or higher) to ensure consistency in returned data (int, string, void, etc.).

  • 2. PHPDocs Update
    Action: Update PHPDoc comments for all generated methods and classes, specifying parameter types, return types, and possible exceptions.

  • 3. Use __DIR__ Instead of dol_include_once
    Action: Replace all occurrences of dol_include_once with __DIR__ for cleaner and more portable path handling.

  • 4. Specify If the Object Manages Lines
    Action: Add an option in the configuration interface to specify if the object manages lines (e.g., for invoices). If not, skip generating related code.

  • 5. Specify If the Object Manages Statuses
    Action: Add an option to indicate if the object manages statuses (e.g., "Pending", "Validated", "Paid"). If not, exclude status management code.

  • 6. Select Available Tabs
    Action: Allow selection of required tabs for the object (e.g., contacts, history). Exclude unnecessary tabs and related code.

  • 7. Dynamically Name Statuses
    Action: Provide the ability to customize status labels in the configuration interface (e.g., "In Progress", "Completed").

  • 8. Meaningful Trigger Names
    Action: Generate clear and meaningful names for triggers, avoiding generic names like mymodule_trigger.

  • 9. Multicompany Management
    Action: Add an option for Multicompany support. Generate the necessary code to handle shared or non-shared references.

  • 10. Select Actions Available on the Object Card
    Action: Allow users to choose available actions on the object card (e.g., clone, email) and generate corresponding code.

  • 11. Use $db->prefix() Instead of MAIN_DB_PREFIX
    Action: Replace all occurrences of MAIN_DB_PREFIX with $db->prefix() to properly handle table prefixes in a multi-company environment.

  • 12. Foreign Keys and Index Management
    Action: For link-type fields, propose creating a foreign key in the database, with options for cascading delete. Add UNIQUE indexes for unique fields and NOT NULL constraints for required fields.

  • 13. Improved accessforbidden Handling
    Action: Enhance generated accessforbidden conditions by supporting OR conditions.

  • 14. Email and trackid Handling
    Action: Ensure the proper generation of trackid values for emails, avoiding generic placeholders like xxx.

  • 15. Remove Placeholder xxx
    Action: Clean up generated code by removing all placeholder values like xxx.

  • 16. Rename myobject Occurrences
    Action: Replace all occurrences of myobject in generated code with the specific object or module name (e.g., invoice).

  • 17. Menu Generation Option
    Action: Provide an option to generate menus for the object, and adapt the generated code accordingly.

  • 18. Access Rights Generation Option
    Action: Allow users to decide whether access rights should be generated for the object and adjust the code based on their choice.

  • 19. Clean Up PDF Model Code
    Action: Avoid generating code related to PDF models if the object does not require this functionality.

  • 20. Improve Access Rights Management in ModuleBuilder
    Action: Automatically update access rights in files when modifications are made via ModuleBuilder.

  • 21. Fix Event Management for Object Creation
    Action: Ensure event handling works correctly during the creation of objects.

@atm-quentin atm-quentin added the Feature request This is a feature request label Dec 23, 2024
@atm-quentin
Copy link
Contributor Author

@eldy What do you think? Which points should be prioritized?

@eldy
Copy link
Member

eldy commented Jan 15, 2025

@eldy What do you think? Which points should be prioritized?

No preference.
Point 3 however must be avoided. i did tests on past ith this and experienced troubles.

Point 12 must also NOT be done for foreign key. See https://wiki.dolibarr.org/index.php/Language_and_development_rules#Foreign_keys

@atm-quentin
Copy link
Contributor Author

Hello @eldy ,

Thank you for your detailed feedback! Here are some clarifications and arguments regarding points 3 and 12.


Point 3: Using __DIR__ Instead of dol_include_once

I understand your concerns regarding this point, but I would like to offer some additional insights.

Why Use __DIR__?

  • Performance Improvement: By using __DIR__, we eliminate the dynamic resolution overhead caused by dol_buildpath or alternative path handling, leading to better performance, particularly in high-traffic modules.
  • Direct Control: __DIR__ offers explicit management of relative paths, minimizing unexpected behavior related to external dependencies.

Mitigating Risks:

  • If __DIR__ is used strictly within the context of modules (without referencing core or third-party files in Dolibarr), the risks are extremely low.
  • To maintain compatibility with Dolibarr best practices, we can restrict the use of __DIR__ to custom modules or provide a warning when it is enabled.

What do you think about it ?


Point 12: Foreign Keys in the Database

I fully acknowledge your valid points regarding foreign key management. However, I’d like to clarify my initial proposal, as there might be some misunderstanding.

Context of My Proposal:

  • The idea was not to enforce hard foreign keys on Dolibarr’s standard tables but to offer an option in ModuleBuilder for developers to add foreign keys in their custom tables while adhering to naming conventions.

Naming Conventions:

You mentioned that foreign key names must follow the format fk_<referencing_table>_<referencing_field>. I completely agree with this approach, and ModuleBuilder could automatically generate foreign key names using this format.

Why Is This Beneficial?

  1. Foreign keys in custom modules simplify data migrations and ensure data integrity.
  2. An option to generate these foreign keys would allow advanced developers to decide on their inclusion while receiving warnings about potential constraints for standard Dolibarr tables.

Additional Benefits: Managing Actions with Foreign Keys

Foreign keys also offer the advantage of defining actions like:

  • ON DELETE CASCADE: Automatically deletes child rows when the parent row is deleted.
  • ON UPDATE CASCADE: Updates references in child tables when primary keys change.

These actions simplify relationship management and ensure data consistency, even during updates or deletions. If properly implemented, they should not cause issues during migrations as they are natively supported by database engines.


Questions About Your Position on Foreign Keys:

  1. Would it be acceptable to use hard foreign keys strictly in external modules without affecting Dolibarr’s standard tables?
  2. Could we consider an optional feature in ModuleBuilder to add foreign keys with actions like ON DELETE CASCADE or ON UPDATE CASCADE, while providing warnings about their potential impacts?

Conclusion

I believe these two points can be implemented safely if:

  1. __DIR__ is used cautiously and specifically in appropriate cases.
  2. Foreign keys follow strict naming conventions and are limited to external modules.
  3. Actions like ON DELETE CASCADE and ON UPDATE CASCADE are well-configured to enhance data management without disrupting migrations or upgrades.

I’m happy to adjust these proposals further if necessary.

@eldy
Copy link
Member

eldy commented Jan 15, 2025

Questions About Your Position on Foreign Keys:

  1. Would it be acceptable to use hard foreign keys strictly in external modules without affecting Dolibarr’s standard tables?

Yes if foreign key is between a table of external module and another table of same external module, it should be ok. No risk of breaking dolibarr internal features.
So it's ok.
However it still has side effect, for example for upgrade or backup/restore process. Depending on name of table, the process to restore may be broken or not.
We experience this in dolibarr bcore with tables of accountancy.

  1. Could we consider an optional feature in ModuleBuilder to add foreign keys with actions like ON DELETE CASCADE or ON UPDATE CASCADE, while providing warnings about their potential impact

Yes but only beteween tables of the same module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature request This is a feature request
Projects
None yet
Development

No branches or pull requests

2 participants