Skip to content

[17.0][MIG] pos_payment_change #1331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 44 commits into
base: 17.0
Choose a base branch
from

Conversation

em230418
Copy link
Contributor

No description provided.

legalsylvain and others added 30 commits March 13, 2025 15:54
- Black python code
- OCA Convention
- Add tests
- add configuration on pos.config, with two option 'refund' or 'update'
- The 'refund' option makes the module compatible with (French) certification
- make the module compatible with pos_order_return
…on PoS Order(s) to know the history of the payments
Currently translated at 100.0% (31 of 31 strings)

Translation: pos-12.0/pos-12.0-pos_payment_change
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_payment_change/es/
Currently translated at 100.0% (33 of 33 strings)

Translation: pos-12.0/pos-12.0-pos_payment_change
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_payment_change/es/
Currently translated at 100.0% (33 of 33 strings)

Translation: pos-12.0/pos-12.0-pos_payment_change
Translate-URL: https://translation.odoo-community.org/projects/pos-12-0/pos-12-0-pos_payment_change/de/
A cash payment method must have a cash journal.
Currently translated at 100.0% (33 of 33 strings)

Translation: pos-16.0/pos-16.0-pos_payment_change
Translate-URL: https://translation.odoo-community.org/projects/pos-16-0/pos-16-0-pos_payment_change/es/
Currently translated at 100.0% (33 of 33 strings)

Translation: pos-16.0/pos-16.0-pos_payment_change
Translate-URL: https://translation.odoo-community.org/projects/pos-16-0/pos-16-0-pos_payment_change/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: pos-16.0/pos-16.0-pos_payment_change
Translate-URL: https://translation.odoo-community.org/projects/pos-16-0/pos-16-0-pos_payment_change/
Currently translated at 100.0% (33 of 33 strings)

Translation: pos-16.0/pos-16.0-pos_payment_change
Translate-URL: https://translation.odoo-community.org/projects/pos-16-0/pos-16-0-pos_payment_change/es/
Currently translated at 100.0% (33 of 33 strings)

Translation: pos-16.0/pos-16.0-pos_payment_change
Translate-URL: https://translation.odoo-community.org/projects/pos-16-0/pos-16-0-pos_payment_change/fr/
In old version, two certifications modules was present in odoo.  l10n_fr_pos_cert and l10n_fr_certification.
In odoo 16, only l10n_fr_pos_cert exists.
@em230418
Copy link
Contributor Author

I am aware of #1264

@em230418 em230418 force-pushed the 17.0-mig-pos_payment_change branch from dd0ca55 to a3c9f37 Compare March 14, 2025 04:34
@em230418 em230418 force-pushed the 17.0-mig-pos_payment_change branch from a3c9f37 to 323a301 Compare March 14, 2025 04:50
Comment on lines 175 to 178
# the demo user should be able to do this
user_demo = self.env.ref("base.user_demo")
wizard = (
self.PosPaymentChangeWizard.with_user(user_demo)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello, @legalsylvain!
Tests are failing because Marc Demo stopped being related to pos manager group.
I don't want to spend time finding it out, why it happens.

How about just creating user with pos manager rights?


# Resale order and mark it as paid
# with the new payment
resale_order = self.copy(default={"pos_reference": self.pos_reference})
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hello, @etobella!
Your module pos_order_copy breaks every other module, that uses "copy" method.
In this case to make compatibility with pos_order_copy, I need to manually copy values from every field, that you marked as "copy=False". Could you please fix your pos_order_copy module?

Here is what happens in auto tests in github actions:

2025-03-14 12:17:12,018 262 ERROR odoo odoo.addons.pos_payment_change.tests.test_module: ERROR: TestModule.test_02_payment_change_policy_refund
Traceback (most recent call last):
  File "/__w/pos/pos/pos_payment_change/tests/test_module.py", line 147, in test_02_payment_change_policy_refund
    self._change_payment(
  File "/__w/pos/pos/pos_payment_change/tests/test_module.py", line 92, in _change_payment
    wizard.button_change_payment()
  File "/__w/pos/pos/pos_payment_change/wizards/pos_payment_change_wizard.py", line 98, in button_change_payment
    orders = order.change_payment(new_payments)
  File "/__w/pos/pos/pos_payment_change/models/pos_order.py", line 72, in change_payment
    resale_order = self.copy(default={"pos_reference": self.pos_reference})
  File "/opt/odoo/odoo/models.py", line 5597, in copy
    record_copy = self.create(vals)
  File "<decorator-gen-227>", line 2, in create
  File "/opt/odoo/odoo/api.py", line 430, in _model_create_multi
    return create(self, [arg])
  File "/opt/odoo/addons/point_of_sale/models/pos_order.py", line 482, in create
    session = self.env['pos.session'].browse(vals['session_id'])
KeyError: 'session_id'

Copy link
Member

Choose a reason for hiding this comment

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

Mmmh, I get your concerns, however, the idea of the module was allowing to make copy properly. It would have no sense to copy the session for example, as the session should be set as the current session. The same for the order_date and so on... Actually, this copy=False should have been defined by Odoo as a core functionality...

We have 2 options here: Just make a copier update and set the incompatibility between this modules or find a way to make them work together.

From my perspective, you can set the information directly on the copy that you are doing if you want them to work together as the copy False that we have settled have a lot of sense (you should set this information on an specific function if other copy=False fields appear over time)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.