[16.0][ADD] confirmation_wizard #942

geomer198 · 2024-08-27T20:27:55Z

The module lets the developer trigger a confirmation wizard during any action in Odoo.
Based on data passed to the wizard and, based on user input, executes specified methods or close wizard.

GabbasovDinar · 2024-08-28T11:47:40Z

confirmation_wizard/wizard/confirmation_message_wizard.py

+
+
+class ConfirmationMessageWizard(models.TransientModel):
+    _name = "confirmation.message.wizard"


The class name ConfirmationMessageWizard can be simplified to ConfirmationWizard. A shorter name improves readability without losing its meaning. And the same for _name = confirmation.wizard

GabbasovDinar · 2024-08-28T11:48:59Z

confirmation_wizard/wizard/confirmation_message_wizard.py

+    _name = "confirmation.message.wizard"
+    _description = "Confirmation Wizard"
+
+    text = fields.Char(string="Confirm Message", required=True)


The field text should be renamed to message to make its purpose clearer, especially in the context of a confirmation dialog.

GabbasovDinar · 2024-08-28T11:52:58Z

confirmation_wizard/wizard/confirmation_message_wizard.py

+    def _prepare_action(self, title=None):
+        action = self.env["ir.actions.actions"]._for_xml_id(
+            "confirmation_wizard.confirmation_message_wizard_action"
+        )
+        if title:
+            action["name"] = title
+        action["context"] = self._context
+        return action


Suggested change

def _prepare_action(self, title=None):

action = self.env["ir.actions.actions"]._for_xml_id(

"confirmation_wizard.confirmation_message_wizard_action"

)

if title:

action["name"] = title

action["context"] = self._context

return action

def _prepare_action(self, title=None):

action = self.env["ir.actions.actions"]._for_xml_id(

"confirmation_wizard.confirmation_wizard_action"

)

action.update({

"name": title or _("Confirmation"),

"context": self._context,

})

return action

GabbasovDinar · 2024-08-28T11:58:35Z

confirmation_wizard/wizard/confirmation_message_wizard.py

+
+    @api.model
+    def confirm_message(
+        self, text, records, title=None, method=None, callback_params=None


Suggested change

self, text, records, title=None, method=None, callback_params=None

self, message, records, title=None, method=None, callback_params=None

GabbasovDinar · 2024-08-28T11:58:47Z

confirmation_wizard/wizard/confirmation_message_wizard.py

+        callback_params = callback_params or {}
+        res_ids = ",".join(map(str, records.ids))
+        wizard = self.create(
+            {
+                "text": text,
+                "res_ids": res_ids,
+                "return_type": "method",
+                "res_model": records._name,
+                "callback_method": method,
+                "callback_params": callback_params,
+            }
+        )


Suggested change

callback_params = callback_params or {}

res_ids = ",".join(map(str, records.ids))

wizard = self.create(

{

"text": text,

"res_ids": res_ids,

"return_type": "method",

"res_model": records._name,

"callback_method": method,

"callback_params": callback_params,

}

)

wizard = self.create(

{

"message": text,

"res_ids": repr(records.ids),

"return_type": "method",

"res_model": records._name,

"callback_method": method,

"callback_params": callback_params or {},

}

)

GabbasovDinar · 2024-08-28T12:06:22Z

confirmation_wizard/wizard/confirmation_message_wizard.py

+
+    def _confirm_method(self):
+        if self.res_ids:
+            res_ids = map(int, self.res_ids.split(","))


use this way:

from ast import literal_eval res_ids = literal_eval(self.res_ids) if self.record_ids else []

GabbasovDinar · 2024-08-28T12:11:30Z

confirmation_wizard/wizard/confirmation_message_wizard.py

+        records = self.env[self.res_model].browse(res_ids)
+        if not records.exists():
+            raise models.UserError(
+                _(f"Records (IDS: {self.res_ids}) not found in model {self.res_model}.")


please, don't use f-string, translations may not work:

_("Records (IDS: '%(ids)s') not found in model '%(model)s'.")

GabbasovDinar · 2024-08-28T12:12:36Z

confirmation_wizard/wizard/confirmation_message_wizard.py

+            raise models.UserError(
+                _(f"Records (IDS: {self.res_ids}) not found in model {self.res_model}.")
+            )
+        if not (records and hasattr(records, self.callback_method)):


Suggested change

if not (records and hasattr(records, self.callback_method)):

if not hasattr(records, self.callback_method):

GabbasovDinar · 2024-08-28T12:14:56Z

confirmation_wizard/wizard/confirmation_message_wizard.py

+                _(f"Records (IDS: {self.res_ids}) not found in model {self.res_model}.")
+            )
+        if not (records and hasattr(records, self.callback_method)):
+            raise models.UserError(_(f"Method ({self.callback_method}) is not found!"))


don't use f-string

"Method '%(callback_method)s' is not found on model '%(res_model)s'."

GabbasovDinar · 2024-08-28T12:16:28Z

confirmation_wizard/wizard/confirmation_message_wizard.py

+            ("window_close", "Return Window Close Action"),
+            ("method", "Return Method"),
+        ],
+        default="window_close",


required=True

Bearnard21

Functionality Tested LGTM

OCA-git-bot · 2024-08-31T15:52:17Z

Sorry @Bearnard21 you are not allowed to rebase.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

OCA-git-bot · 2024-09-23T11:33:13Z

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). 🤖

ivs-cetmix · 2024-11-26T21:24:10Z

Hi @OCA/server-environment-maintainers, any chances to have this reviewed and merged?

rvalyi · 2024-12-29T14:37:46Z

@ivs-cetmix @GabbasovDinar People might have different opinions, but mine is that such a simple tool might get a broader adoption if licensed under LGPL rather than AGPL. Many companies would simply replicate the feature without adopting it if licensed as AGPL. In a word, my opinion is simple framework tools are better licensed under LGPL. But of course the choice is yours and this won't impact for merging the contrib.

ivs-cetmix · 2024-12-29T14:41:04Z

@ivs-cetmix @GabbasovDinar People might have different opinions, but mine is that such a simple tool might get a broader adoption if licensed under LGPL rather than AGPL. Many companies would simply replicate the feature without adopting it if licensed as AGPL. In a word, my opinion is simple framework tools are better licensed under LGPL. But of course the choice is yours and this won't impact for merging the contrib.

Sounds reasonable, we will change the license to "LGPL-3".

confirmation_wizard/readme/USAGE.rst

hoangtrann · 2025-01-26T07:57:23Z

confirmation_wizard/readme/USAGE.rst

+      # Confirmation wizard
+      return (
+        self.env["confirmation.wizard"]
+        .with_context(skip_confirm=True)


I don't see how skip_confirm is being implemented, do I miss anything?

I don't see how skip_confirm is being implemented, do I miss anything?

if not self._context("skip_confirm", False): This line is before confirm_message method

I understand, but it means the developer who is using this confirmation wizard need to implement a context checking, e.g skip_confirm, every time we invoke the wizard. This seems redundant to me

hoangtrann · 2025-01-26T08:00:40Z

confirmation_wizard/wizard/confirmation_wizard.py

+        Confirm message with close window return type
+        :param message: confirmation message
+        :param title: wizard title
+        :return dict: ir actions act window dict


You should include invisible_cancel as a special context here and how it works. I think it should be show_cancel is False to hide it, default to True, or hide_cancel is True to hide the Cancel button. invisible_cancel isn't a proper word in english

You should include invisible_cancel as a special context here and how it works. I think it should be show_cancel is False to hide it, default to True, or hide_cancel is True to hide the Cancel button. invisible_cancel isn't a proper word in english

This context key used as special configuration and use to all methods where confirmation wizard call. This functional is describe in USAGE.rst file.

even it's already been in the documentation, it's a good thing to put that context in this docstring as well as they are closely used together

hoangtrann

Code LGTM

rvalyi · 2025-01-29T20:52:23Z

confirmation_wizard/wizard/confirmation_wizard.py

@@ -0,0 +1,122 @@
+# Copyright (C) 2024 Cetmix OÜ
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).


@ivs-cetmix seems you guys ended up changing the license for LGPL in the manifest file but kept AGPL in the file headers, making the whole inconsistent. Pick the one you want but it should be consistent... cc @geomer198

geomer198 force-pushed the 16.0-t3893-confirmation_wizard-add branch 3 times, most recently from 73f9b19 to af7b05b Compare August 27, 2024 21:53

GabbasovDinar suggested changes Aug 28, 2024

View reviewed changes

geomer198 force-pushed the 16.0-t3893-confirmation_wizard-add branch 2 times, most recently from 9c83ede to d78f015 Compare August 28, 2024 18:54

Bearnard21 approved these changes Aug 28, 2024

View reviewed changes

GabbasovDinar approved these changes Aug 29, 2024

View reviewed changes

OCA-git-bot added the approved label Aug 29, 2024

geomer198 force-pushed the 16.0-t3893-confirmation_wizard-add branch 2 times, most recently from cebe087 to 7b9b96f Compare August 31, 2024 15:57

OCA-git-bot added the ready to merge label Sep 23, 2024

geomer198 force-pushed the 16.0-t3893-confirmation_wizard-add branch from bd8d75a to 1babaf2 Compare September 23, 2024 13:43

geomer198 force-pushed the 16.0-t3893-confirmation_wizard-add branch from 00f10b0 to 62467bd Compare October 21, 2024 11:44

geomer198 force-pushed the 16.0-t3893-confirmation_wizard-add branch from 62467bd to c7d0a9f Compare December 29, 2024 12:53

geomer198 force-pushed the 16.0-t3893-confirmation_wizard-add branch 3 times, most recently from 697a715 to 420be3b Compare January 1, 2025 11:06

[ADD] confirmation_wizard: Module added

efe6033

geomer198 force-pushed the 16.0-t3893-confirmation_wizard-add branch from 7a90287 to efe6033 Compare January 1, 2025 11:40

hoangtrann suggested changes Jan 26, 2025

View reviewed changes

OCA-git-bot removed approved ready to merge labels Jan 26, 2025

geomer198 added 2 commits January 26, 2025 19:13

[FIX] confirmation_wizard: Code refactoring

0659684

[FIX] confirmation_wizard: Code refactoring

291af85

hoangtrann approved these changes Jan 29, 2025

View reviewed changes

OCA-git-bot added the approved label Jan 29, 2025

rvalyi requested changes Jan 29, 2025

View reviewed changes

OCA-git-bot removed the approved label Jan 29, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][ADD] confirmation_wizard #942

[16.0][ADD] confirmation_wizard #942

geomer198 commented Aug 27, 2024 •

edited

Loading

GabbasovDinar Aug 28, 2024

GabbasovDinar Aug 28, 2024

GabbasovDinar Aug 28, 2024

GabbasovDinar Aug 28, 2024

GabbasovDinar Aug 28, 2024

GabbasovDinar Aug 28, 2024

GabbasovDinar Aug 28, 2024

GabbasovDinar Aug 28, 2024

GabbasovDinar Aug 28, 2024

GabbasovDinar Aug 28, 2024

Bearnard21 left a comment

OCA-git-bot commented Aug 31, 2024

OCA-git-bot commented Sep 23, 2024

ivs-cetmix commented Nov 26, 2024

rvalyi commented Dec 29, 2024 •

edited

Loading

ivs-cetmix commented Dec 29, 2024

hoangtrann Jan 26, 2025

geomer198 Jan 26, 2025

hoangtrann Jan 27, 2025

hoangtrann Jan 26, 2025

geomer198 Jan 26, 2025 •

edited

Loading

hoangtrann Jan 27, 2025

hoangtrann left a comment

rvalyi Jan 29, 2025 •

edited

Loading



		class ConfirmationMessageWizard(models.TransientModel):
		_name = "confirmation.message.wizard"

	self, text, records, title=None, method=None, callback_params=None
	self, message, records, title=None, method=None, callback_params=None

	if not (records and hasattr(records, self.callback_method)):
	if not hasattr(records, self.callback_method):

		@@ -0,0 +1,122 @@
		# Copyright (C) 2024 Cetmix OÜ
		# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

[16.0][ADD] confirmation_wizard #942

Are you sure you want to change the base?

[16.0][ADD] confirmation_wizard #942

Conversation

geomer198 commented Aug 27, 2024 • edited Loading

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

Bearnard21 left a comment

Choose a reason for hiding this comment

OCA-git-bot commented Aug 31, 2024

OCA-git-bot commented Sep 23, 2024

ivs-cetmix commented Nov 26, 2024

rvalyi commented Dec 29, 2024 • edited Loading

ivs-cetmix commented Dec 29, 2024

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

geomer198 Jan 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hoangtrann left a comment

Choose a reason for hiding this comment

rvalyi Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

geomer198 commented Aug 27, 2024 •

edited

Loading

rvalyi commented Dec 29, 2024 •

edited

Loading

geomer198 Jan 26, 2025 •

edited

Loading

rvalyi Jan 29, 2025 •

edited

Loading