[16.0][ADD] pos_order_to_sale_order_sale_financial_risk: Sale Financial Risk in POS #1024

geomer198 · 2023-07-10T19:09:44Z

This module is a bridging module between sale_financial_risk and pos_order_to_sale_order. It implements control for the Sale Orders created from POS.
Same warning or blocking message will be displayed in POS as if an order was created from the backend.

ivs-cetmix

Functional review ok

ivs-cetmix · 2023-07-11T07:52:04Z

sale_financial_risk_pos_compatibility/__manifest__.py

@@ -0,0 +1,23 @@
+{
+    "name": "Sale Financial Risk Pos Compatibility",


name: "Sale Financial Risk in POS"

ivs-cetmix · 2023-07-11T07:52:58Z

sale_financial_risk_pos_compatibility/__manifest__.py

+    "name": "Sale Financial Risk Pos Compatibility",
+    "version": "16.0.1.0.0",
+    "category": "Sales/Point of Sale",
+    "summary": "Sale Financial Risk Pos Compatibility",


Sale Financial Risk control for Sales Orders created from POS

GabbasovDinar · 2023-07-25T16:28:17Z

sale_financial_risk_pos_compatibility/models/pos_session.py

+        user_dict = super(PosSession, self)._get_pos_ui_res_users(params)
+        user_id = user_dict.get("id")
+        if user_id and (user := self.env["res.users"].browse(user_id)).exists():
+            user_dict.update(
+                has_role_risk_manager=user.has_group(
+                    "account_financial_risk.group_overpass_partner_risk_exception"
+                )
+            )
+        return user_dict


user will exist because the id is from the current user

Suggested change

user_dict = super(PosSession, self)._get_pos_ui_res_users(params)

user_id = user_dict.get("id")

if user_id and (user := self.env["res.users"].browse(user_id)).exists():

user_dict.update(

has_role_risk_manager=user.has_group(

"account_financial_risk.group_overpass_partner_risk_exception"

)

)

return user_dict

data = super()._get_pos_ui_res_users(params)

user = self.env["res.users"].browse(data["id"])

data["has_role_risk_manager"] = user.has_group(

"account_financial_risk.group_overpass_partner_risk_exception",

)

return data

GabbasovDinar · 2023-07-25T16:29:34Z

sale_financial_risk_pos_compatibility/static/src/js/CreateOrderPopup.esm.js

+
+import CreateOrderPopup from "point_of_sale.CreateOrderPopup";
+import Registries from "point_of_sale.Registries";
+import {_t} from "web.core";


You can use this.env._t instead

GabbasovDinar · 2023-07-25T16:54:24Z

sale_financial_risk_pos_compatibility/static/src/js/CreateOrderPopup.esm.js

+                    ],
+                ],
+            });
+            console.log(this.env.pos);


GabbasovDinar · 2023-07-25T16:54:57Z

sale_financial_risk_pos_compatibility/static/src/js/CreateOrderPopup.esm.js

+                    ],
+                ],
+            });
+            console.log(this.env.pos);


GabbasovDinar

rename sale_financial_risk_pos_compatibility to pos_sale_financial_risk

GabbasovDinar

changes are needed regarding the overriding of the createSaleOrder method

GabbasovDinar · 2023-09-04T21:15:10Z

sale_financial_risk_pos_compatibility/static/src/js/CreateOrderPopup.esm.js

+            } else {
+                return await super._actionCreateSaleOrder(order_state);
+            }
+            const {confirmed} = await this.showPopup("ConfirmRiskPopup", {


it seems to be necessary to show different popup depending on the settings

if (this.env.pos.user.has_role_risk_manager) { const {confirmed} = await this.showPopup("ConfirmPopup", { title: this.env._t("Partner risk exceeded"), body: exception_msg, }); if (confirmed) { return await super._actionCreateSaleOrder(order_state); } } else { await this.showPopup("ErrorPopup", { title: this.env._t("Partner risk exceeded"), body: exception_msg, }); } return await this.cancel();

creating a new popup is unnecessary

GabbasovDinar · 2023-09-04T21:23:30Z

sale_financial_risk_pos_compatibility/static/src/js/CreateOrderPopup.esm.js

+                showButton: this.env.pos.user.has_role_risk_manager,
+            });
+            if (confirmed) {
+                this.extraContext = {context: {bypass_risk: true}};


Instead of using this variable, I suggest setting this value as an order property;

order.set_bypass_risk(true);

and order model:

const PosSaleFinancialRiskOrder = (Order) => class PosSaleFinancialRiskOrder extends Order { constructor() { super(...arguments); this.bypass_risk = false; } set_bypass_risk(bypass_risk) { this.bypass_risk = bypass_risk } export_as_JSON() { const result = super.export_as_JSON(...arguments); result.bypass_risk = this.bypass_risk; return result; } init_from_JSON(json) { super.init_from_JSON(...arguments); this.bypass_risk = json.bypass_risk; } } Registries.Model.extend(Order, PosSaleFinancialRiskOrder); export default PosSaleFinancialRiskOrder;

so this values will available in order_data of def create_order_from_pos(self, order_data, action) method

and you can check:

if "bypass_risk" in order_data: self = self.with_context(bypass_risk=bypass_risk) return super(SaleOrder, self).create_order_from_pos(order_data, action)

please check this way

GabbasovDinar · 2023-09-04T21:24:34Z

sale_financial_risk_pos_compatibility/static/src/js/CreateOrderPopup.esm.js

+                })
+                .finally(function () {
+                    framework.unblockUI();
+                });


There is no need to override this method if you will be using the order

GabbasovDinar

please check comments

GabbasovDinar · 2023-09-04T21:32:43Z

sale_financial_risk_pos_compatibility/static/src/js/ConfirmRiskPopup.esm.js

+    showButton: false,
+};
+
+Registries.Component.add(ConfirmRiskPopup);


not need create new popup

geomer198 · 2023-12-22T14:53:59Z

@ivantodorovich, @ivs-cetmix, @GabbasovDinar, @DemchukM Could you please make code review again?

ivantodorovich · 2023-12-26T11:29:46Z

Thanks a lot @geomer198 !

Looks great to me 👍🏻

Before merging, could you squash the 2 commits and reword the first one to account for the new module name? 🙏🏻

geomer198 · 2023-12-26T19:53:40Z

@ivantodorovich Is all okay?

ivantodorovich · 2023-12-26T19:55:46Z

Perfect, thanks!!

/ocabot merge nobump

OCA-git-bot · 2023-12-26T19:55:58Z

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-1024-by-ivantodorovich-bump-nobump, awaiting test results.

Signed-off-by ivantodorovich

OCA-git-bot · 2023-12-26T20:03:40Z

@ivantodorovich your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-1024-by-ivantodorovich-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

ivantodorovich · 2023-12-27T11:23:44Z

The merge command failed with some error.

Could you rebase?

I'll try with ocabot but it can fail depending on the PR permissions

/ocabot rebase

OCA-git-bot · 2023-12-27T11:23:50Z

@ivantodorovich The rebase process failed, because command git push --force cetmix tmp-pr-1024:16.0-t2682-sale_financial_risk_pos_compatibility-add failed with output:

remote: Repository not found.
fatal: repository 'https://github.com/cetmix/pos/' not found

geomer198 · 2023-12-27T12:49:42Z

@ivantodorovich There is a problem with tests, tests that do not depend on this module are not run.

ivantodorovich · 2023-12-27T14:05:38Z

It may be related to this warning:
Tour PosOrderToSaleOrderTour is already defined

You probably have to choose a different and unique name for the test tours

geomer198 · 2023-12-28T16:20:43Z

@ivantodorovich Is all good now?

OCA-git-bot · 2023-12-28T16:25:32Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

ivantodorovich · 2023-12-28T16:49:51Z

Looks great! Thanks!

/ocabot merge nobump

OCA-git-bot · 2023-12-28T16:49:57Z

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-1024-by-ivantodorovich-bump-nobump, awaiting test results.

OCA-git-bot · 2023-12-28T16:58:21Z

Congratulations, your PR was merged at 1af25ee. Thanks a lot for contributing to OCA. ❤️

geomer198 marked this pull request as draft July 10, 2023 19:09

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from feff0b6 to c46c303 Compare July 10, 2023 19:52

geomer198 marked this pull request as ready for review July 10, 2023 20:50

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from e463155 to e8a43ca Compare July 10, 2023 22:02

geomer198 marked this pull request as draft July 13, 2023 20:16

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from 6127e03 to e514684 Compare July 17, 2023 22:07

geomer198 marked this pull request as ready for review July 17, 2023 22:12

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch 6 times, most recently from 06b627c to ed68499 Compare July 24, 2023 17:33

ivs-cetmix approved these changes Jul 25, 2023

View reviewed changes

GabbasovDinar suggested changes Jul 25, 2023

View reviewed changes

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from ed68499 to 0e7ce01 Compare July 25, 2023 18:01

GabbasovDinar approved these changes Jul 28, 2023

View reviewed changes

OCA-git-bot added the approved label Jul 28, 2023

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from 642e150 to bc18518 Compare August 16, 2023 14:25

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from c66a0f0 to 95c1ae6 Compare September 2, 2023 21:40

GabbasovDinar suggested changes Sep 4, 2023

View reviewed changes

OCA-git-bot removed the approved label Sep 4, 2023

GabbasovDinar suggested changes Sep 4, 2023

View reviewed changes

GabbasovDinar reviewed Sep 4, 2023

View reviewed changes

GabbasovDinar suggested changes Sep 4, 2023

View reviewed changes

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from bbbff55 to 8cf2faa Compare September 6, 2023 00:59

legalsylvain added this to the 16.0 milestone Sep 26, 2023

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from 8cf2faa to b1bc5ca Compare September 27, 2023 14:17

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from 1f864b2 to d66b2fb Compare December 26, 2023 19:48

OCA-git-bot added the bot is merging ⏳ label Dec 26, 2023

OCA-git-bot added a commit that referenced this pull request Dec 26, 2023

Merge PR #1024 into 16.0

a8020ce

Signed-off-by ivantodorovich

OCA-git-bot removed the bot is merging ⏳ label Dec 26, 2023

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from d66b2fb to ea24bab Compare December 27, 2023 12:16

ivantodorovich changed the title ~~[16.0][ADD] sale_financial_risk_pos_compatibility~~ [16.0][ADD] pos_order_to_sale_order_sale_financial_risk: Sale Financial Risk in POS Dec 27, 2023

ivantodorovich added the needs fixing label Dec 28, 2023

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from e9ef3cc to 397df34 Compare December 28, 2023 15:56

[ADD] pos_order_to_sale_order_sale_financial_risk: Module added

88e4a95

geomer198 force-pushed the 16.0-t2682-sale_financial_risk_pos_compatibility-add branch from 830b7a4 to 88e4a95 Compare December 28, 2023 16:16

OCA-git-bot added the ready to merge label Dec 28, 2023

OCA-git-bot added the bot is merging ⏳ label Dec 28, 2023

OCA-git-bot merged commit 71488e4 into OCA:16.0 Dec 28, 2023
7 checks passed

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Dec 28, 2023

		@@ -0,0 +1,23 @@
		{
		"name": "Sale Financial Risk Pos Compatibility",

[16.0][ADD] pos_order_to_sale_order_sale_financial_risk: Sale Financial Risk in POS #1024

[16.0][ADD] pos_order_to_sale_order_sale_financial_risk: Sale Financial Risk in POS #1024

Conversation

geomer198 commented Jul 10, 2023

ivs-cetmix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GabbasovDinar left a comment

Choose a reason for hiding this comment

GabbasovDinar left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GabbasovDinar Sep 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GabbasovDinar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geomer198 commented Dec 22, 2023

ivantodorovich commented Dec 26, 2023

geomer198 commented Dec 26, 2023

ivantodorovich commented Dec 26, 2023

OCA-git-bot commented Dec 26, 2023

OCA-git-bot commented Dec 26, 2023

ivantodorovich commented Dec 27, 2023

OCA-git-bot commented Dec 27, 2023

geomer198 commented Dec 27, 2023

ivantodorovich commented Dec 27, 2023

geomer198 commented Dec 28, 2023

OCA-git-bot commented Dec 28, 2023

ivantodorovich commented Dec 28, 2023

OCA-git-bot commented Dec 28, 2023

OCA-git-bot commented Dec 28, 2023

GabbasovDinar left a comment •

edited

Loading

GabbasovDinar Sep 4, 2023 •

edited

Loading