Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[16.0][OU-ADD] loyalty #4187

Merged
merged 1 commit into from
Jul 9, 2024
Merged

[16.0][OU-ADD] loyalty #4187

merged 1 commit into from
Jul 9, 2024

Conversation

pilarvargas-tecnativa
Copy link
Contributor

@pilarvargas-tecnativa pilarvargas-tecnativa commented Oct 23, 2023 •
edited
Loading

@legalsylvain
Copy link
Contributor

/ocabot migration loyalty

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Oct 23, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Oct 23, 2023
Open
@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-mig-loyalty branch 3 times, most recently from fcfcc65 to e171bdd Compare November 1, 2023 16:57
@pilarvargas-tecnativa pilarvargas-tecnativa marked this pull request as ready for review November 1, 2023 16:58
chienandalu
chienandalu suggested changes Nov 2, 2023
View reviewed changes
Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Starting to look quite good :)

  • Check CIs
  • Comment code as much as you can

openupgrade_scripts/scripts/loyalty/16.0.1.0/noupdate_changes.xml Outdated Show resolved Hide resolved
openupgrade_scripts/scripts/loyalty/16.0.1.0/pre-migration.py Outdated Show resolved Hide resolved
@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-mig-loyalty branch 10 times, most recently from b7584b1 to 824798f Compare November 8, 2023 17:58
cormaza
cormaza suggested changes Dec 23, 2023
View reviewed changes
Copy link

@cormaza cormaza left a comment

Choose a reason for hiding this comment

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

Please, check api call

openupgrade_scripts/scripts/loyalty/16.0.1.0/pre-migration.py Outdated Show resolved Hide resolved
@legalsylvain legalsylvain mentioned this pull request Jan 12, 2024
Merged
@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-mig-loyalty branch 2 times, most recently from 040d8e9 to 57af6c1 Compare May 16, 2024 06:01
pedrobaeza
pedrobaeza requested changes Jul 8, 2024
View reviewed changes
openupgrade_scripts/scripts/loyalty/16.0.1.0/post-migration.py
loyalty module sets the following values in portal_point_name depending on the
program_type field. This is done in post so that the language context can be used."""
portal_point_names = {
"coupons": _("Coupon point(s)"),
Copy link
Member

Choose a reason for hiding this comment

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

AFAIK, these translation are never going to apply, as they are not included in the standard PO/POT files (the reference is by file name).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/odoo/odoo/blob/16.0/addons/loyalty/i18n/es.po#L746
But they are in the standard translation files and that is how the loyalty programs are named:
https://github.com/odoo/odoo/blob/16.0/addons/loyalty/models/loyalty_program.py#L197

Copy link
Member

Choose a reason for hiding this comment

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

But as said, translations work referring the file:

image

so this is not going to work.

openupgrade_scripts/scripts/loyalty/16.0.1.0/post-migration.py Outdated Show resolved Hide resolved
openupgrade_scripts/scripts/loyalty/16.0.1.0/pre-migration.py Outdated Show resolved Hide resolved
openupgrade_scripts/scripts/loyalty/16.0.1.0/pre-migration.py Outdated Show resolved Hide resolved
@pedrobaeza
Copy link
Member

Error trying the scripts:

  File "/opt/odoo/custom/src/openupgrade/openupgrade_scripts/scripts/loyalty/16.0.1.0/pre-migration.py", line 800, in migrate
    update_loyalty_reward_data(env)
  File "/opt/odoo/custom/src/openupgrade/openupgrade_scripts/scripts/loyalty/16.0.1.0/pre-migration.py", line 314, in update_loyalty_reward_data
    openupgrade.logged_query(
  File "/usr/local/lib/python3.10/site-packages/openupgradelib/openupgrade.py", line 1703, in logged_query
    cr.execute(query, args)
  File "/opt/odoo/custom/src/odoo/odoo/sql_db.py", line 321, in execute
    res = self._obj.execute(query, params)
psycopg2.errors.UndefinedColumn: column "minimum_amount_tax_mode" does not exist
LINE 5:                 WHEN minimum_amount_tax_mode = 'tax_included...

@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-mig-loyalty branch 3 times, most recently from 0d5ab47 to 244e48c Compare July 9, 2024 09:35
@pilarvargas-tecnativa
Copy link
Contributor Author

@pedrobaeza All changes done

@pilarvargas-tecnativa pilarvargas-tecnativa force-pushed the 16.0-mig-loyalty branch 4 times, most recently from 1bc2c92 to 72b5c5b Compare July 9, 2024 14:25
pedrobaeza
pedrobaeza approved these changes Jul 9, 2024
View reviewed changes
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

Let's merge for now after first real tests. Later fixup/improvements in subsequent PRs.

@pedrobaeza pedrobaeza merged commit e81babd into OCA:16.0 Jul 9, 2024
2 checks passed
@pedrobaeza pedrobaeza deleted the 16.0-mig-loyalty branch July 9, 2024 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FernandoRomera FernandoRomera FernandoRomera left review comments

@xtanuiha xtanuiha xtanuiha left review comments

@chienandalu chienandalu chienandalu requested changes

@cormaza cormaza cormaza requested changes

@pedrobaeza pedrobaeza pedrobaeza approved these changes

Assignees
No one assigned
Labels
Dependency OK
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
8 participants
@pilarvargas-tecnativa @legalsylvain @pedrobaeza @cormaza @chienandalu @xtanuiha @FernandoRomera @OCA-git-bot