Skip to content

Rake findorganizers#11

Merged
sandraor merged 16 commits intosandraor:masterfrom
lzcabrera:rake_findorganizers
Jan 17, 2015
Merged

Rake findorganizers#11
sandraor merged 16 commits intosandraor:masterfrom
lzcabrera:rake_findorganizers

Conversation

@lzcabrera
Copy link

I have added the ability to pull organizers data from meetup.com.
Thanks @solsberg for creating the MeetupsData module, I added another class to handle the gathering of organizers data.

To get organizers from groups that don't have organizers listed you can use the following command: 'rake findorganizers'

One thing to note is that the meetup api only lists one organizer.
Wondering if we should remove the organizers from meetups that have more than one organizer to keep things consistent going forward or until the api returns more than one organizer, co-organizer, event organizer?

Also, I'm not quite sure how to handle/hide the API key. Suggestions? :)

@wifelette
Copy link

Adding reference: emberjs#1904

@wifelette
Copy link

Question:

I don't think Meetup limits the number of people you can list as organizers/co-organizers, and I've seen groups (not in Ember) that might have 20 organizers, because they rotate responsibilities, or it's an honorary title, or whatever. I wonder if we might need to add code in to cap the number of organizers we pull in? Maybe something like limiting it to 3, and if there are more, we can have it say "...and 2 more" or something.

@lzcabrera Thoughts from a code perspective?
@sandraor Thoughts from a design perspective?

If you run locally and take a look, Toronto is an example of one that right now with just the data entry has 4-5 organizers, and so we have a scrollbar in the pop up. It's not awful, but it's also not wonderful, and would be even more annoying if that list was longer.

Also not a dealbreaker, but just thinking out loud, making sure we account for edge cases :)

@wifelette
Copy link

Annnd I totally didn't read the part where you said the API only returns 1 😛

I'm not actually sure what to do with that. Seems kind'v silly of the API... I don't think we want to limit to one organizer in general, since part of the point is to give people recognition for their work. Thoughts?

@lzcabrera
Copy link
Author

I agree with you. I think we can leave the ones we already have there.
The rake task can be called to get the one organizer until they fix their API :)

In the meantime we will have to manually add any other organizers that don't get returned when running the rake task.

* upstream/master:
  Put back logging block mistakenly omitted
  Insert correct image URLs and properties, include map-cluster icons, and fix JS style
  Add configuration for clusters
  Seperate drawMap from zoomToPosition

Conflicts:
	source/javascripts/meetups.js
Copy link

Choose a reason for hiding this comment

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

Maybe use a regex here instead?

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense. We should be able to type http://meetup.com or meetup.com and still be able to grab the organizers data.

@wycats
Copy link

wycats commented Jan 16, 2015

Also, I'm not quite sure how to handle/hide the API key

The normal approach here is to use an environment variable (ENV["MEETUP_API_KEY"]) and tell people to set it in the README. Before trying to use it, check to make sure it's not empty and give a nice warning to the user if they haven't set it yet. 😄

@lzcabrera
Copy link
Author

@wycats thanks for the feedback. It was very much appreciated and ended up learning quite a few things.

I'm really looking forward to seeing this live :)!

sandraor added a commit that referenced this pull request Jan 17, 2015
@sandraor sandraor merged commit 57f74f6 into sandraor:master Jan 17, 2015
@sandraor
Copy link
Owner

Looks awesome, @lzcabrera! I just merged it in.

I noticed a small issue: The "active" locations in the list seem to stay active (bolded) after clicking on other locations.

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.

4 participants