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

[8.0] event contact person #37

Merged
merged 11 commits into from
Mar 30, 2016
Merged

Conversation

mikevhe18
Copy link
Contributor

My goal is to create a module that can display the contact person at the event page on the website.
So before I make that module, i make this module which has the function to add the contact person fields in the event. Thanks

@rafaelbn
Copy link
Member

Hi @mikevhe18 , Odoo already has this fields, why do you want to duplicate this info?

26-02-2016 11-20-54

@rafaelbn rafaelbn added this to the 8.0 milestone Feb 26, 2016
@mikevhe18
Copy link
Contributor Author

@rafaelbn it's because sometimes events can have one or more contact person.
The responsible user is different from the contact person. Responsible user is an organization / person who organizes an event, meanwhile the contact person is the person who responsible to be contacted by people

@rafaelbn
Copy link
Member

OK, thanks. 👍

Please @yajo take a review here.

@@ -0,0 +1,15 @@
# -*- coding: utf-8 -*-
# © 2016 Michael Viriyananda
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agp
Copy link
Member

Choose a reason for hiding this comment

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

Wrong URL, as in all other headers.

@yajo
Copy link
Member

yajo commented Mar 7, 2016

Good work overall @mikevhe18, please fix those little details to get approval. Thanks.

@mikevhe18
Copy link
Contributor Author

Thanks @yajo.
I've made a changes, would you kindly review this PR again ?

{
'name': 'Event Contact Person',
'version': '8.0.1.0.0',
'summary': 'Add a contact persons to event',
Copy link
Member

Choose a reason for hiding this comment

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

Remove the "a"

@andhit-r
Copy link
Member

Tested on runbot 👍

@yajo
Copy link
Member

yajo commented Mar 21, 2016

👍

@andhit-r
Copy link
Member

Hi @OCA/crm-sales-marketing-maintainers . This PR is ready to be merged (3 reviews)

@pedrobaeza
Copy link
Member

For me the only problem is the name: you can say the same with the name event_contact, as person is not a relevant part in my opinion. What do you think?

@yajo
Copy link
Member

yajo commented Mar 22, 2016

Good point, partners can be companies too.

@andhit-r
Copy link
Member

Agree

@mikevhe18
Copy link
Contributor Author

@pedrobaeza @yajo @andhit-r I've made a changes, please review this module again. Thank You..

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 16b88a2 on mikevhe18:8.0-event_contact_person into * on OCA:8.0*.

@pedrobaeza
Copy link
Member

Please don't put plurals in module name. The module name should be event_contact

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b613fa6 on mikevhe18:8.0-event_contact_person into * on OCA:8.0*.

class event_event(models.Model):
_inherit = 'event.event'

contacts_ids = fields.Many2many(
Copy link
Member

Choose a reason for hiding this comment

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

The name must be contact_ids, as the plural is only used in the _ids part

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 3a22fdc on mikevhe18:8.0-event_contact_person into * on OCA:8.0*.

@yajo
Copy link
Member

yajo commented Mar 30, 2016

Finally got it! Thanks 😉 👍

@sergio-teruel
Copy link
Contributor

👍

@rafaelbn
Copy link
Member

Thanks 👍

@rafaelbn rafaelbn merged commit 2b15c31 into OCA:8.0 Mar 30, 2016
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.

7 participants