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

[17.0][MIG] event_registration_partner_unique: Migration to version 17.0 #390

Merged
merged 22 commits into from
Jul 29, 2024
Merged

[17.0][MIG] event_registration_partner_unique: Migration to version 17.0 #390

merged 22 commits into from
Jul 29, 2024

Conversation

juanjosesegui-tecnativa
Copy link

@juanjosesegui-tecnativa juanjosesegui-tecnativa commented Jul 17, 2024 •
edited by pedrobaeza
Loading

chienandalu
chienandalu reviewed Jul 17, 2024
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.

The name of the PR isn't correct 😅

@juanjosesegui-tecnativa juanjosesegui-tecnativa changed the title [17.0][MIG] sale_order_invoicing_finished_task: Migration to version 17.0 [17.0][MIG] event_registration_partner_unique: Migration to version 17.0 Jul 17, 2024
@pedrobaeza
Copy link
Member

Please check the red CIs, as the module doesn't even install.

@juanjosesegui-tecnativa
Copy link
Author

The module depends on partner_event, I suspect it does not install because partner_event is not in version 17.0 of OCA.

@chienandalu
Copy link
Member

You're suspicions are funded 😄 push the migration of partner_event so you can move forward on this one 😉

@pedrobaeza
Copy link
Member

/ocabot migration event_registration_partner_unique

If this depends in another PR, you can do: https://github.com/OCA/maintainer-tools/wiki/Use-temporary-reference%28s%29-to-another-pull-request%28s%29

@OCA-git-bot OCA-git-bot added this to the 17.0 milestone Jul 18, 2024
@OCA-git-bot OCA-git-bot mentioned this pull request Jul 18, 2024
Open
21 tasks
@pedrobaeza
Copy link
Member

And always put in the main comment such dependency:

Depends on:

- [ ] partner_event: https://github.com/OCA/event/pull/382

@juanjosesegui-tecnativa juanjosesegui-tecnativa force-pushed the 17.0-mig-event_registration_partner_unique branch 2 times, most recently from 08ec984 to 2b32d7d Compare July 18, 2024 08:18
@pedrobaeza
Copy link
Member

You have ignored this part of the guide:

imagen

@juanjosesegui-tecnativa juanjosesegui-tecnativa force-pushed the 17.0-mig-event_registration_partner_unique branch 2 times, most recently from 153a5d9 to 0c78ca4 Compare July 19, 2024 06:59
@juanjosesegui-tecnativa
Copy link
Author

ping @pedrobaeza @chienandalu

chienandalu
chienandalu suggested changes Jul 19, 2024
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.

Tested 👍

Just some small changes:

event_registration_partner_unique/__manifest__.py Outdated
@@ -3,11 +3,12 @@
# Copyright 2018 Tecnativa - Cristina Martin R.
# Copyright 2020 Tecnativa - Víctor Martínez
# Copyright 2023 Tecnativa - Carolina Fernandez
# Copyright 2024 Tecnativa - Juan José Seguí Borja
Copy link
Member

Choose a reason for hiding this comment

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

Normally for such small migration changes, better put yourself in the contributors.md file

event_registration_partner_unique/views/event_event_view.xml Outdated
@@ -10,7 +10,7 @@
<field name="priority" eval="99" />
<field name="inherit_id" ref="event.view_event_form" />
<field name="arch" type="xml">
<xpath expr="//field[@name='auto_confirm']" position="after">
<xpath expr="//field[@name='badge_image']" position="after">
Copy link
Member

Choose a reason for hiding this comment

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

It makes more sense after this one

            <xpath expr="//field[@name='create_partner']" position="after">

Copy link
Author

Choose a reason for hiding this comment

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

Done!

@juanjosesegui-tecnativa juanjosesegui-tecnativa force-pushed the 17.0-mig-event_registration_partner_unique branch from 0c78ca4 to 65d843d Compare July 19, 2024 07:40
chienandalu
chienandalu reviewed Jul 19, 2024
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.

But put yourself in contributors 😅

@juanjosesegui-tecnativa juanjosesegui-tecnativa force-pushed the 17.0-mig-event_registration_partner_unique branch from 65d843d to 05a1a2b Compare July 19, 2024 08:50
chienandalu
chienandalu approved these changes Jul 19, 2024
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.

👍

@chienandalu
Copy link
Member

plase review @carolinafernandez-tecnativa

@carolinafernandez-tecnativa
Copy link

Please Squash administrative commits. Check https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate for details.

@juanjosesegui-tecnativa juanjosesegui-tecnativa force-pushed the 17.0-mig-event_registration_partner_unique branch from 05a1a2b to 6db241f Compare July 19, 2024 10:39
@juanjosesegui-tecnativa
Copy link
Author

@carolinafernandez-tecnativa Done!

@carolinafernandez-tecnativa
Copy link

Please check again I still seing administrative commits and pre-commit auto fixes for migration is missing.

@juanjosesegui-tecnativa juanjosesegui-tecnativa force-pushed the 17.0-mig-event_registration_partner_unique branch 3 times, most recently from d55cdde to c3d8bed Compare July 19, 2024 15:53
@carolinafernandez-tecnativa
Copy link

Administrative commits OK

Check please this commit [MIG] event_registration_partner_unique: Migration to 17.0 is empty
and you should not change anything related to migration like this examples in pre-commit f38a2b9#diff-858e724775ccf6c429de4636fe90cfabad2b2a15ef432cd347a8eab571e0d21dR10
f38a2b9#diff-84d67375c5db3c3d8f88527f5e791e6be3011ae1d5e53927986dce40cfb51545R13

@juanjosesegui-tecnativa juanjosesegui-tecnativa force-pushed the 17.0-mig-event_registration_partner_unique branch from c3d8bed to acad67d Compare July 22, 2024 09:12
Victor M.M. Torres and others added 9 commits July 22, 2024 11:39
@juanjosesegui-tecnativa
[MIG] event_registration_partner_unique: Migration to v12
b7c2b1a
@victoralmau @juanjosesegui-tecnativa
[IMP] event_registration_partner_unique: black, isort, prettier
76fb3d9
@victoralmau @juanjosesegui-tecnativa
[MIG] event_registration_partner_unique: Migration to 13.0
cf96e33
@anasuarez1 @juanjosesegui-tecnativa
Translated using Weblate (Spanish)
ababb78 
Currently translated at 100.0% (5 of 5 strings)

Translation: event-13.0/event-13.0-event_registration_partner_unique
Translate-URL: https://translation.odoo-community.org/projects/event-13-0/event-13-0-event_registration_partner_unique/es/
@pedrobaeza @juanjosesegui-tecnativa
[FIX] event_registration_partner_unique: Avoid false positives
cd80e98 
Steps to reproduce the problem:

- Register anonymously a first attendee for an event from website.
- The registration is saved with no attendee_partner_id.
- Repeat the registration again anonymously.
- You get the error "Duplicated partners found in event...".

That's because current duplicated domain is searching for other
registrations regardless of if there's a linked partner or not.

We now restrict the search to those with a linked partner.

TT32961
@juanjosesegui-tecnativa
[IMP] event_registration_partner_unique: black, isort, prettier
9c100c1
@juanjosesegui-tecnativa
[MIG] event_registration_partner_unique: Migration to 14.0
8297799
@juanjosesegui-tecnativa
[FIX] event_registration_partner_unique: improve README
1cba4f9
@ernesto-garcia-tecnativa @juanjosesegui-tecnativa
[MIG] event_registration_partner_unique: Migration to 15.0
597b2fa
@juanjosesegui-tecnativa juanjosesegui-tecnativa force-pushed the 17.0-mig-event_registration_partner_unique branch from acad67d to 9c3a944 Compare July 22, 2024 16:02
@carolinafernandez-tecnativa
Copy link

Please Squash administrative commits. There are still some from translation that should be removed. Check https://github.com/OCA/maintainer-tools/wiki/Merge-commits-in-pull-requests#mergesquash-the-commits-generated-by-bots-or-weblate for details.

@juanjosesegui-tecnativa juanjosesegui-tecnativa force-pushed the 17.0-mig-event_registration_partner_unique branch from 9c3a944 to 09158e8 Compare July 23, 2024 09:08
carolinafernandez-tecnativa
carolinafernandez-tecnativa approved these changes Jul 29, 2024
View reviewed changes
Copy link

Choose a reason for hiding this comment

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

LGTM Thanks :)

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

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-390-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit b42c201 into OCA:17.0 Jul 29, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza deleted the 17.0-mig-event_registration_partner_unique branch July 29, 2024 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chienandalu chienandalu chienandalu approved these changes

@carolinafernandez-tecnativa carolinafernandez-tecnativa carolinafernandez-tecnativa approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
17.0
Development

Successfully merging this pull request may close these issues.