[18.0] Refactor & Split Proposal for PMS
Dear community,
After working with the pms module in v16 and testing its migration to v18, I found that the module is useful and works well in general.
However, the current structure feels quite monolithic. I believe it would be more maintainable and scalable if we split it into smaller modules.
I have two main suggestions:
- Split the module structure
- Refactor the code to follow OCA standards
What do you think about this idea? @DarioLodeiros
Suggested Module Structure
1. Base Module
pms
Core property and stay management
2. Queue Integration
pms_queue_job
Adds queue job functionality to PMS flows- Depends on:
['pms', 'queue_job']
- Depends on:
3. Partner Contact Extensions
pms_partner_contact
Adds contact details such as gender, birthdate, and nationality- May be split each module or include one module
- Depends on:
['partner_contact_gender', 'partner_contact_birthdate', 'partner_contact_nationality']
4. Other Observations
multi_pms_property
Not sure if this module is still necessary. It might be better to merge its logic intopmsand control it via configuration settings or system parameters.
Code Refactor
During the migration to Odoo 18:
- Found many unused or redundant XML elements (e.g.,
<newline/>, unused views, etc.) - Cleaned up and standardized the code to follow OCA guidelines
Hi @Saran440 !!
First of all, thank you very much for your contribution and sorry for the late reply — we are just coming back from the holiday season.
I fully agree with both of your main suggestions:
- Splitting the module structure into smaller, more maintainable parts.
- Refactoring the code to follow OCA standards.
Current situation
Right now, we are carrying out a strong refactor in the 16.0 branch.
Our initial plan was to finish that refactor in 16.0 and then migrate more easily to 18.0.
However, since the migration to 18 has already started, I think we can try to coordinate:
-
First, make a clean migration of the current state to 18.0 with only the minimal changes required (syntax adjustments or compatibility with Odoo 18).
-
Meanwhile, we can start preparing a document with refactor proposals to achieve the two main goals (split and OCA compliance).
-
Once PMS is operational in 18.0, we can then address the refactor.
- If the refactor is done in 16.0, we will take care of porting it to 18.0 (with the migration script only in 16.0).
- If the refactor starts in 18.0, then we will take care of backporting it to 16.0 and maintaining the migration script there.
This way we keep compatibility and avoid having to maintain migration scripts in both versions — as long as we keep 18.0 in alpha until the process is finished.
If this approach sounds good to you, we can coordinate and start building that document together.
Regarding your specific points
- Base module → 100% agreed, the goal is to make it lighter.
- Queue integration → At the moment, the only feature in PMS requiring jobs is the automated folio invoicing. It might be better to extract that functionality to a specific module, removing the queue dependency from PMS.
- Partner contact extensions → Following the model of the OCA
partner_contactrepo, I think it makes sense to create specific modules per field. Depending on local legislation, different check-in fields are required. This way, each localization can include only the modules it needs (both theres.partnerones and new ones forpms.checkin, e.g.pms_checkin_gender). - multi_pms_property → This module needs to load before the service to alter field attributes and allow the
check_propertiesmechanism similar to companies. That’s the reason it is separated. I believe it should stay that way to ensure correct loading, but if you know another way to “merge” it without breaking that, we are totally open — it would actually be better.
Next steps
Beyond those, we already have many other proposals to improve and mature the project.
If you are OK with it, we can start drafting that plan and later move the proposals into specific issues and PRs for coordination.
Again, thank you very much for your work and input 🙌
If you have availability, we can start as soon as you want.
@DarioLodeiros Thank you for your feedback.
I agree with you to start refactor with v16 and next port to v18 later. Before that, I will use this module in v18 but it is a larger module. So, I can't continue migration and time.
However, If i have a time. I will refactor module in version16 to OCA standard.
From your question
multi_pms_property
- I'm still not clear for this module but maybe it can use rule to visible, invisible property like operating_unit https://github.com/OCA/operating-unit
Hi @Saran440 !
Perfect, if you agree, we’ll start working on a refactoring roadmap on v16. We’ll create a shared document and a general issue for this refactoring to serve as a guide. As we start each part defined there, we’ll open a dedicated issue and PR for it, so we can move forward and try to get it done as soon as possible. Regardless of whether you can work on some of these parts or not, you’re invited to review and discuss the specific proposals whenever you find it convenient. If we can tackle the refactoring before starting the migration, moving it to v18 will be much smoother and more comfortable for everyone :)
As for the proposal regarding the operating_unit rules, I think it could work and remove the need for the module. It’s true that we would have to replace the extra functionality it provides with the check_pms_property field attribute (an attribute added to fields to prevent record inconsistencies between different establishments, similar to what check_company does). However, I think this could be replaced with constrains… let’s analyze it. Thanks!
Thanks a lot for your work on this topic.
What about https://github.com/OCA/pms/pull/360 while pending refactorings ?