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

[16.0][MIG] partner_event: Migration to version 16.0 #322

Closed
wants to merge 28 commits into from

Conversation

kwoychesko
Copy link

Biggest change is that _onchange_partner_id was removed in v16 core code so we no longer super it here. Instead, we call the _synchronize_partner_values method, which was the main job of the old _onchange_partner_id.

antespi and others added 27 commits June 23, 2023 07:02
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)
Currently translated at 100.0% (24 of 24 strings)

Translation: event-12.0/event-12.0-partner_event
Translate-URL: https://translation.odoo-community.org/projects/event-12-0/event-12-0-partner_event/es/
Currently translated at 72.0% (18 of 25 strings)

Translation: event-14.0/event-14.0-partner_event
Translate-URL: https://translation.odoo-community.org/projects/event-14-0/event-14-0-partner_event/it/
Squashed commits:
[012682e] [UPD] Update partner_event.pot
[7afe1fd] [UPD] README.rst
Squashed commits:
[1536c5c] Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: event-15.0/event-15.0-partner_event
Translate-URL: https://translation.odoo-community.org/projects/event-15-0/event-15-0-partner_event/
@kwoychesko kwoychesko mentioned this pull request Jun 23, 2023
15 tasks
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.

Thanks :) Some comments:

  • Change your commit message following the guidelines

EventRegistration, self.with_context(**get_attendee_partner_address)
)._onchange_partner_id()
return super(EventRegistration, self)._onchange_partner_id()
# onchange for partner_id removed in v16 core - including functionality here
Copy link
Member

Choose a reason for hiding this comment

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

Why not changing the whole logic to a compute readonly=False store=True as it's donde with the other fields? (e. g: https://github.com/OCA/OCB/blob/4860a93e28a82003f881e5c21ddc2dd4229e14a8/addons/event/models/event_registration.py#L62-L69

Copy link
Author

Choose a reason for hiding this comment

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

I'm sorry I don't understand the question. Are you able to expand any further? Do you mean: change the definition of the attendee_partner_id field to be more like the definition of the email field in the core code? E.g.:

email = fields.Char(string='Email', compute='_compute_email', readonly=False, store=True, tracking=11)

"category": "Marketing",
"author": "Tecnativa," "Odoo Community Association (OCA)",
"website": "https://github.com/OCA/event",
"development_status": "Production/Stable",
"license": "AGPL-3",
"depends": ["event"],
"depends": ["base", "event", "website"],
Copy link
Member

Choose a reason for hiding this comment

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

Why these new deps?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for reviewing! It's as a result of some errors raised when executing the tests. Messages like the following occur:

odoo.addons.base.models.ir_qweb.QWebException: Error while render the template
Exception: Unallowed to fetch files from addon website
Template: web.report_layout

In retrospect, adding the "website" dependency fixes the error and allows the tests to run, and probably "base" is unnecessary.

I'm new to migrating addons, so if you have a better way to handle this I'd appreciate any input!

Biggest change is that _onchange_partner_id was removed in v16 core code so we no longer super it here. Instead, we call the _synchronize_partner_values method, which was the main job of the old _onchange_partner_id.
@pedrobaeza
Copy link
Member

Please include #340

@pedrobaeza
Copy link
Member

/ocabot migration partner_event

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Sep 5, 2023
@carolinafernandez-tecnativa

@kwoychesko could you please include #340 ?
Thnks

@pedrobaeza
Copy link
Member

Superseded by #345

@pedrobaeza pedrobaeza closed this Oct 4, 2023
@kwoychesko kwoychesko deleted the 16.0-mig-partner_event branch October 5, 2023 14:15
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.

None yet