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.

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

[15.0][MIG] partner_event: Migration to version 15.0 #293

Merged
merged 26 commits into from
Jan 25, 2023

Conversation

stefan-tecnativa
Copy link
Contributor

cc @Tecnativa TT41018

@chienandalu @victoralmau please review

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.

Check pre-commit please

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.

Functional test ok.

  • Fix your migration commit message as is reversed with its content
  • Let's update that old icon
    icon(1)
    icon(1)

@OCA-git-bot
Copy link
Contributor

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

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.

Please squash together these translation commits:

Selección_025

and also include the SVG in partner_event/static/description folder

partner_event/__manifest__.py Outdated Show resolved Hide resolved
@stefan-tecnativa
Copy link
Contributor Author

Please squash together these translation commits:

Selección_025

and also include the SVG in partner_event/static/description folder

Would it be appropriate to do this with all translation commits as well?

@pedrobaeza
Copy link
Member

Would it be appropriate to do this with all translation commits as well?

No, because you will lose the attribution to the corresponding translator. That's why I only say to merge together the translations from the same translator.

@pedrobaeza
Copy link
Member

On the new push you have made, I'm not seeing any of the Italian translation commits. Have you removed them or squash with another? I'm now also seeing 2 Swedish translation commits coming from the same person that can be squashed together.

@stefan-tecnativa
Copy link
Contributor Author

Would it be appropriate to do this with all translation commits as well?

No, because you will lose the attribution to the corresponding translator. That's why I only say to merge together the translations from the same translator.

Okay, understood. Thanks.

antespi and others added 11 commits January 25, 2023 13:25
These fields were stored. That usually is good, but if you add partners to an event from the wizard, it means each `res.partner` record gets its `registration_count` field updated.

When you update any record, fields `write_uid` and `write_date` are updated too.

Now imagine you want to add 4000 partners to an event. That would take time. If in the mean time any other user updates the `res.partner` record (along with their `write_UID` and `WRITE_DATE` fields), you would get this error:

```
openerp.sql_db: bad query: UPDATE "res_partner" SET "registration_count"=3,"write_uid"=5,"write_date"=(now() at time zone 'UTC') WHERE id IN (25578)
Traceback (most recent call last):
  File "/opt/odoo/common/openerp/v8/openerp/sql_db.py", line 234, in execute
    res = self._obj.execute(query, params)
TransactionRollbackError: could not serialize access due to concurrent update
CONTEXT:  SQL statement "SELECT 1 FROM ONLY "public"."res_users" x WHERE "id" OPERATOR(pg_catalog.=) $1 FOR KEY SHARE OF x"
```

By removing the `store=True` parameter, you simply avoid those concurrent updates and do not block other users.

But this changes the database structure, so remember to update your database!
If adding 1000 partners, we save now 999 queries.

(cherry picked from commit 89853ef)
@stefan-tecnativa
Copy link
Contributor Author

On the new push you have made, I'm not seeing any of the Italian translation commits. Have you removed them or squash with another? I'm now also seeing 2 Swedish translation commits coming from the same person that can be squashed together.

Change done.

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.

/ocabot merge nobump
/ocabot migration partner_event

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 15.0-ocabot-merge-pr-293-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot mentioned this pull request Jan 25, 2023
16 tasks
@OCA-git-bot OCA-git-bot merged commit 1e59d78 into OCA:15.0 Jan 25, 2023
@OCA-git-bot
Copy link
Contributor

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

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

Successfully merging this pull request may close these issues.