MyRadio_Event (Grouping Timeslots across shows) #929
Conversation
|
Thank you for this pull request, @michael-grace! @ColinRoitt, would you mind reviewing this PR, please? Feel free to ask anyone for help if you're not quite sure what you're doing. |
michael-grace
changed the title
|
Fairly sure this used to exist, in the form of show blocks… cc @mstratford do you know more? |
ColinRoitt
left a comment
ColinRoitt
left a comment
There was a problem hiding this comment.
Looks good to me - this isn't implemented anywhere near the front end yet, right?
| { | ||
| "required": ["email"] | ||
| }] | ||
| "required": ["eduroam"] |
markspolakovs
left a comment
markspolakovs
left a comment
There was a problem hiding this comment.
Would ideally like to see all future API stuff done over GraphQL, but the 2016-site infra isn't there yet, so that's fine. Couple small niggles.
| create table schedule.events | ||
| ( | ||
| event_id text not null | ||
| constraint events_pk |
There was a problem hiding this comment.
Unnecessary, PRIMARY KEY would suffice
| @@ -0,0 +1,22 @@ | |||
| BEGIN; | |||
|
|
|||
| create table schedule.events | |||
There was a problem hiding this comment.
Consider consistent singular/plural table names? (e.g. member, show_season)
| 'mixcloud_status' => $this->getMeta('upload_state'), | ||
| 'mixcloud_starttime' => $this->getMeta('upload_starttime'), | ||
| 'mixcloud_endtime' => $this->getMeta('upload_endtime'), | ||
| 'events' => $this->events, |
There was a problem hiding this comment.
You're gonna want to recursively toDataSource this (or use the appropriate CoreUtil), otherwise you'll JSON-encode an array of MyRadio_Event classes
There was a problem hiding this comment.
no, it'll just be the ids, that why $this->events not $this->getEvents()
There was a problem hiding this comment.
We sure we want that? Perhaps make it a mixin to getEvents() - saves N API calls, where N is the number of events
| * @throws MyRadioException | ||
| */ | ||
|
|
||
| public function addEvent($event_id){ |
There was a problem hiding this comment.
Not sure how I feel about having the same-ish method in both Event and Timeslot… but feel free to overrule me
Basically, allows us to have a group of, for example "freshers shows" or "results day shows" etc.. even though they're all different shows.