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

An Event has many years? #36

Closed
nodunayo opened this issue Nov 27, 2017 · 18 comments
Closed

An Event has many years? #36

nodunayo opened this issue Nov 27, 2017 · 18 comments

Comments

@nodunayo
Copy link
Owner

nodunayo commented Nov 27, 2017

At the moment, an Event has a name attribute and a year attribute, e.g. Event.new(name: RubyConf, year: 2017).

I wonder if we should change this to be an event has a name and then many years (or occurrences) attached to it, e.g. RubyConf is an Event, it happened in 2017, 2016, 2015, etc..

This will make it more interesting to view the kinds of submissions that, for example, RubyConf has accepted and rejected over the years. We can also add more information to a particular event, for example, something like: 'RandomConf has a different group of reviewers each round'.

I suspect that the event view will be more interesting than the speaker view for a more general audience.

(You could also then look on a per year basis across a range of events and perhaps see what themes were popular or unpopular among speakers, for example.)

What are people's thoughts on this?

We'd have to change the flow of adding a new event to the platform as well.

@flyinggrizzly
Copy link
Contributor

flyinggrizzly commented Dec 9, 2017

I think that would make a lot of sense; it's kind of how I think of conferences--the con is abstract, and the event is once per unit of time. I'm guessing I'm not the only one.

I'd be tempted to rename the existing Event model to Cohort or EventYear (not sure just Year is as clear as it could be), and make it belong to a new Event record. That way existing proposals still refer to the same record.

Changes that would need to be made:

  • rename existing Event model to Cohort or Year
  • create new Event model
  • set up ActiveRecord associations between Event and Cohort
  • update Proposal to point to renamed Cohort resource
  • update views to refer to the Event's name, not the Cohort's name, effectively sidestepping the danger of having an event cohort called "Bath Ruby Bath Ruby 2015", instead of just "Bath Ruby 2015"
    • would it make sense to allow the updated Cohort model to have a subtitle? Do conferences do this? "Bath Ruby 2018: the Bathening"?
    • on further consideration, a more regular event, maybe like Front End London or Bath Ruby's small-scale quarterly meetups might use this, to note the focus of that particular event
      • "FEL December 2017: Awesome topic" (I don't think they actually subtitle their events)
      • "BathCamp October 2017: the future of health" (BathCamp does in fact subtitle their events)
      • these smaller meetups/events might not be the target event for Speakerline
      • also, this would be pushing an event's year to be a date instead

Models might look like

# new event model
class Event
  validates :name
  validates_uniqueness_of :name
  
  has_many :cohorts # or :years
  has_many :submissions, through: :cohorts
end

# renamed from Event
class Cohort
  belongs_to :event
  validates :year
  validates_uniqueness_of :year, scope: :event

  has_many :submissions

  def name_and_year
   "#{event.name} #{year}"
  end
end

Creating new events/cohorts

This would need to be updated to behave in the same way as Proposals--they require a User to be present to attach the proposal to.

  1. Create Event (new step)
  2. Create Cohort (same step, as before, but looks different and with new requirements)

Submissions

Probably add an extra step in the submission creation flow:

  1. Choose proposal
  2. Choose event (new step)
  3. Choose cohort (same step, looks different now)

@flyinggrizzly
Copy link
Contributor

flyinggrizzly commented Dec 9, 2017

Enh. Event as a name for a new resource is a bad idea. EventSeries makes assumptions that it will repeat though (which we're kind of doing anyways, so maybe not a problem?). I'm not coming up with anything better atm.

@nodunayo
Copy link
Owner Author

Hey @flyinggrizzly — thanks for your thoughts.

Will have a proper read-through at some point in the upcoming week! :-)

@nodunayo
Copy link
Owner Author

FYI, @flyinggrizzly — going through a busy time right now so this will take me a bit longer to look at.

@flyinggrizzly
Copy link
Contributor

@nodunayo no worries--I'm tinkering with it already to see how my idea plays out. If you get curious, take a look!

About to disappear to the US for Christmas though, so no promises as to how much I'll be doing over the next few weeks. Happy to pick it up again in the new year if the help is still wanted/needed!

@nodunayo
Copy link
Owner Author

Ooh, I'll definitely take a look some time soon.

Let's pick up this conversation in the New Year (at the latest) for sure. I'm about to move flat and lots of change is happening at work at the moment. Then it's family time/holidays, etc.

I'd love to keep working with you in the New Year. It's been a pleasure!

Happy holidays if we don't speak before then.

@flyinggrizzly
Copy link
Contributor

@nodunayo this is more than I expected (which in retrospect, was maybe a little obvious...), but I'm still working on it 😅

Trying to figure out how to wire the two models together so you can select or name new Event Series from the Cohort screen r/n

@nodunayo
Copy link
Owner Author

Hey! Hope you've had good holidays.

Looking forward to reading all of this and digging into your work soon. 😁

@flyinggrizzly
Copy link
Contributor

flyinggrizzly commented Jan 5, 2018

Happy new year!

I've got this working, at least for events themselves (haven't gone through and fixed everything that will likely have broken in tests or in some of the related/dependent models). This is probably a good time to look at it and decide if this is the right approach?

If you wanted to kick the tires a bit, best thing to do would be to try add new events, and also take a look at the submission form. I've updated the form and submissions are being accepted too.

It's worth having a few events with at least 2 cohorts each before you go to create submissions so that you can really see if that form is clear enough.

Also, FYI, the migration doesn't completely reverse (it won't add the names back into the events table after recreating the name column on dir.down), so the databases have been renamed, and you'll need to db:create && db:migrate again (then add in your test data manually). Figured this would be easier in the interrim rather than clobbering existing dev databases.

@nodunayo
Copy link
Owner Author

nodunayo commented Jan 5, 2018

Happy New Year, @flyinggrizzly! Looking forward to digging in! I appreciate all of your work! ☺️

@nodunayo
Copy link
Owner Author

nodunayo commented Jan 7, 2018

Still been on holiday but back on things tomorrow. Aiming to find time to look at this this week!

@nodunayo
Copy link
Owner Author

Hey @flyinggrizzly,

I just had a look at this and definitely like the approach! I like the way you've approached the "Add an Event" form — clever! (I'm sure there'll be UI/UX improvements for the best flow, but we can worry about that later). I also think the grouping in the Submissions for Event dropdown makes sense (though I'd want to order them in year order — I'm sure you had that idea too).

I wonder whether Cohort is the right term...perhaps EventYear is better? Or maybe EventOccurrence but that's not fun for spelling purposes...or MAYBE EventInstance?? What do you think?

There are a few things that would need fixing before this would be merged into the new project. However, I know this is an interim stage and you're aware of a lot of these so not leaving detailed comments for now. Also, I'm sure we'll have a proper migration for when we come to applying this to the production DB?

All in all, this looks like we're moving in the right direction!

Do you have any specific questions/concerns for me before you spend any more time working on it?

I'm trying to think of any gotchas I should be thinking about and asking you but none are coming to mind right now. It seems like a low risk change that is a step in the right direction...I already know that we definitely can't keep the events as one long string with a name!

@flyinggrizzly
Copy link
Contributor

flyinggrizzly commented Jan 11, 2018

Hi @nodunayo,

Thanks for looking it over!

I like EventOccurrence or EventInstance because they seem most accurate in terms of relation to reality. EventYear feels too restrictive because I think some events might want to add more occurrences than annually... Occurrence feels more accurate, but I agree on the spelling thing. I'm happy to go with either.

Couple of questions before I forge ahead:

  • EventOccurrence or EventInstance?
  • when you say you'd want the options in year order, do you mean the <optgroup> should be the year rather than the event (see below), or did I just forget to order the occurrences within the <optgroup label="#{event.name}">?
  • migration should be reversible, but it'll take some more thought... watch this space!

optgroup by year:

<optgroup label="2018">
  <option>Awesome event</option>
  <option>The best event</option>
</optgroup>
<optgroup label="2017">
  <option>Awesome event</option>
  <option>The best event</option>
</optgroup>

@nodunayo
Copy link
Owner Author

Hi!

In answer to your points/questions, I'd say:

  • EventInstance? We can always change it, right? But it's more for behind-the-scenes...and I think Instance wins because of the spelling thing...
  • I mean keep it grouped by event.name, but then order by 'year' ascending...so, at the moment, it appears to be ordered by created_at?

screen shot 2018-01-11 at 21 17 54

  • Yes please to a reversible migration! :P Let me know if you want to discuss ideas.

😁

@flyinggrizzly
Copy link
Contributor

Hey @nodunayo just a quick ping so you know I'm still here and working on this--time hasn't been as plentiful lately, but I am still committed!

@nodunayo
Copy link
Owner Author

Thanks for the update! No rush at all!

I'm glad you haven't finished it yet as time has been short on my side too!

Thanks so much for all of your work!

:-)

@flyinggrizzly
Copy link
Contributor

Should we close this issue?

@nodunayo
Copy link
Owner Author

Yes!! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants