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

[13.0][MIG] crm_event: Migration to 13.0 #240

Merged
merged 3 commits into from
Nov 10, 2021

Conversation

yajo
Copy link
Member

@yajo yajo commented Nov 9, 2021

  • Upstream merged both lead and opportunity form views.

@Tecnativa TT31318

@yajo yajo added this to the 13.0 milestone Nov 9, 2021
@yajo yajo added the migration label Nov 9, 2021
@yajo yajo self-assigned this Nov 9, 2021
@pedrobaeza pedrobaeza changed the title [MIG] crm_event: Migration to 13.0 [13.0][MIG] crm_event: Migration to 13.0 Nov 9, 2021
Copy link
Member

@victoralmau victoralmau left a comment

Choose a reason for hiding this comment

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

Code review OK.

@pedrobaeza
Copy link
Member

Can you please squash together these commits?

Selección_005

And then you can proceed to merge (or tell me for me doing it).

Jairo Llopis added 3 commits November 10, 2021 13:22
This module extends the functionality of CRM opportunities (and leads, if
enabled) to support linking them to event types and to allow you to keep track
of leads interested in an upcoming event of some type.

This is useful if you organize your events based on the amount of people
interested in a certain type of event.

@Tecnativa TT27664

[UPD] Update crm_event.pot

[UPD] README.rst

[ADD] icon.png
- Upstream merged both lead and opportunity form views.

@Tecnativa TT31318
@yajo
Copy link
Member Author

yajo commented Nov 10, 2021

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 13.0-ocabot-merge-pr-240-by-Yajo-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit c422a3a into OCA:13.0 Nov 10, 2021
@OCA-git-bot
Copy link
Contributor

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

def init(self):
"""(Re-)create report view."""
tools.drop_view_if_exists(self.env.cr, self._table)
# pylint: disable=sql-injection
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

@yajo Why disabling this as it was correct? You should pass query params to the second argument of execute().

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case I can't do it through params, because that would escape the SQL code. I need to execute the raw SQL code instead, as this is meant to create a SQL view. So, this is a false positive problem.

However thanks for raising the question, it reflects you reviewed thoroughly.

@yajo yajo deleted the 13.0-mig-crm_event branch November 15, 2021 09:24
@yajo
Copy link
Member Author

yajo commented Nov 15, 2021

/ocabot migration crm_event

@OCA-git-bot OCA-git-bot mentioned this pull request Nov 15, 2021
29 tasks
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.

None yet

5 participants