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

Add new RegionalPartner relationships #13646

Merged
merged 6 commits into from Mar 13, 2017
Merged

Conversation

Hamms
Copy link
Contributor

@Hamms Hamms commented Mar 8, 2017

RegionalPartners now have and belong to many "program managers" (Users), and have many Workshops, laying the groundwork that will allow organizers to specify one of their associated regional partners for a workshop during creation.

@Hamms Hamms force-pushed the regionalpartner-multiple-contacts branch from a2d5a97 to a13c764 Compare March 8, 2017 23:22
@Hamms Hamms changed the base branch from fully_remove_plp to staging March 8, 2017 23:22
@Hamms Hamms force-pushed the regionalpartner-multiple-contacts branch from a13c764 to 7e7da2d Compare March 8, 2017 23:23
@Hamms Hamms changed the title [WIP] Add new RegionalPartner relationships Add new RegionalPartner relationships Mar 8, 2017
Copy link
Contributor

@aoby aoby left a comment

Choose a reason for hiding this comment

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

A test is failing, and I have some suggestions about naming. Otherwise LGTM

has_many :regional_partner_organizers
has_many :organizers,
class_name: 'User',
through: :regional_partner_organizers
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think organizers is the best name for this association. They're only organizers in relation to workshops, but in the context of the regional partners it may not be the best label. I checked w Carina and she suggested program_managers. What do you think? We could also go with something shorter or just users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, perfect! I tried to find out what we called these folk yesterday, but never got an answer. Program managers sounds perfect, thanks!

# index_regional_partner_organizers_on_regional_partner_id (regional_partner_id)
#

class RegionalPartnerOrganizer < ActiveRecord::Base
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above. I think we should rename. These users don't organize the partner, they are associated with the partner as program managers, and the partner entity can organize a workshop.

@Hamms Hamms force-pushed the regionalpartner-multiple-contacts branch from ab82f8a to caf4d74 Compare March 9, 2017 19:18
@Hamms
Copy link
Contributor Author

Hamms commented Mar 9, 2017

cc @mehalshah

@Hamms Hamms merged commit 91c191c into staging Mar 13, 2017
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

4 participants