[FIX] mail: splicing schedule method

[somu-odoo]

Description of the issue/feature this PR addresses:

Current behavior before PR:

Desired behavior after PR is merged: This PR separates the schedule method from scheduleActivity method, so that it can be overridden. Needed to be done to trigger reloadParentView method for documents.document.

See here: odoo-dev/enterprise@14c7a95

Task - 3786861

---

I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

[robodoo]

.

[alexkuhn]

It's not great to introduce so many artificial splits to enable so much overrides. This makes refactoring the code far more complicated than necessary.

Considering that documents.document override is essentially about doing props.reloadParentView() after many actions, the footprint of override could be much lower if that the original code has a single state flag to determine whether an action triggers a reloadParentView. By default it doesn't, but in documents.document override could be a single LOC that just set this flag to true.

[somu-odoo]

Hi @alexkuhn I do agree that making such artificial splits will introduce a slight complication with code refactoring, but even if I go with your approach of setting a flag after every necessary action called from documents.document, it will need a trigger for the reloadParentView since activity doesn't has an inbuilt way to call the method on it's own. And that will end up being something similar to current one. I hope I understood the approach suggested correctly, if you meant something different, please let me know.

[alexkuhn]

@somu-odoo You could have the flag in the env, so that all children components are aware of this flag, including Activity.

The gain is to reduce drastically the footprint of patches / overrides. This is desirable because every patch / override makes the code much less flexible due to them adding feature on top of existing components by relying on their current code structure. The shape of code is mostly arbitrary, so reducing its flexibility can make refactoring very hard. Also in general, the maintainer of the original component is not necessarily the same as the maintainer of the patch, so that's another reason to reduce drastically the work of the patcher if any is necessary.

I get that in the past we went a lot with patching and overriding stuffs in JS, but we learned that they worsen code maintenance. Ideally all features are available in original code and you just have to use the component. Customising the component in an non-official way like with patching / overriding methods should be very rare and minimal.

[somu-odoo]

@alexkuhn I've pushed another approach which should work as you have suggested, if I understood it correct. Let me know if this needs any changes

[alexkuhn]

@alexkuhn I've pushed another approach which should work as you have suggested, if I understood it correct. Let me know if this needs any changes

This is much better! I like the reduced code in the patch. Thanks :)

[tde-banana-odoo]

Hello @somu-odoo ,

Code LGTM, especially it was already reviewed :) could you just update commit message ? Notably header "[FIX] mail: splitting up methods" does not mean much (more like ease patching chatter update methods, stuff like that). Purpose is to help people understand what and why you did something

Also don't link commits in commit message, as it will be broken once a rebase is done (and it will be done by mergebot anyway). Enterprise PR will be linked automatically so you may just talk about "linked enterprise PR".

Cheers !

[somu-odoo]

@tde-banana-odoo I hope this title works, also updated the ent. commit hash to PR link

[tde-banana-odoo]

@robodoo r+