Skip to content
This repository was archived by the owner on Feb 16, 2026. It is now read-only.

14.0 purchase sale inter company notify user#4

Closed
ntsirintanis wants to merge 3 commits into14.0from
14.0-purchase_sale_inter_company_notify_user
Closed

14.0 purchase sale inter company notify user#4
ntsirintanis wants to merge 3 commits into14.0from
14.0-purchase_sale_inter_company_notify_user

Conversation

@ntsirintanis
Copy link

No description provided.

@ntsirintanis ntsirintanis force-pushed the 14.0-purchase_sale_inter_company_notify_user branch from f6bd221 to e58908e Compare September 9, 2024 12:25
),
)
)
return purchase.user_id.id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should rather be purchase.company_id.notify_user.id or purchase.user_id.id, I think, so that it can be configured on the target company side.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. So in res.config.settings, I have made notify_user_id a required field, only when notification_side is configured for source company sale.order. So first I should remove this restriction, and then make this change

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yes.


def _get_user_to_notify(self, purchase):
"""Notify user based on res.config.settings"""
if self.company_id.notification_side == "so":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the rare case where someone wants different notification sides for different dropshipping companies, maybe this should be purchase.company_id.notification_side instead. So that people can configure it per target company.

@thomaspaulb
Copy link
Member

Integrated within #8

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants