Get workshops information in detail view #19929
Conversation
…ter API to get summer workshops
Use the workshop ID of the selected workshop to prefil the dropdown
…:code-dot-org/code-dot-org into get_workshops_information_in_detail_view
|
|
| handleScoreChange: PropTypes.func, | ||
| courseName: PropTypes.string, | ||
| assignedWorkshopId: PropTypes.number, | ||
| handleSelectedWorkshopChange: PropTypes.func |
There was a problem hiding this comment.
nit: typically event (callback) props are named onAction and the event handlers (functions) are named handleAction. That's how the React docs show it. Unless there's a reason to break convention here, I'd prefer these to follow it (i.e. rename lines 35 and 38 from handle to on).
| } | ||
|
|
||
| state = { | ||
| loading: true |
There was a problem hiding this comment.
this does not appear to be used
| // We aren't testing any of the responses of the workshop selector control, so just | ||
| // return success for all requests for the time being to suppress the warning message | ||
| // in the test output | ||
| sinon.fakeServer.create(); |
There was a problem hiding this comment.
note - the comment isn't quite accurate. This won't return anything (success or otherwise). It creates a stub server, but unless you set a response this will simply not return from the API calls. Therefore it won't hit your done callbacks.
There was a problem hiding this comment.
okay - I don't have any done callbacks at the time so I'm fine with the fake server. I'll just fix the comment
dashboard/app/models/pd/workshop.rb
Outdated
| @@ -334,6 +334,12 @@ def friendly_name | |||
| "#{course_subject} workshop on #{start_time} at #{location_name}"[0...255] | |||
| end | |||
|
|
|||
| def short_name | |||
| return '' unless sessions.size && processed_location | |||
There was a problem hiding this comment.
I don't think you want sessions.size here. In Ruby, 0 is truthy:
irb(main):006:0> !!0
=> true
irb(main):007:0> !0
=> false
Maybe sessions.any? ?
There was a problem hiding this comment.
Also this seems kind of specific to the teachercon / summer workshop view. I'm not sure that short_name is entirely appropriate. We already have a friendly_name and this isn't necessarily shorter. I think this might better belong in the serializer.
Or if we want to leave it in the model, at least name it more appropriately: something like friendly_date_and_location_name.
There was a problem hiding this comment.
I'm fine moving this to the serializer
| @@ -128,7 +128,7 @@ def get_applications_by_role(role) | |||
|
|
|||
| def application_params | |||
| params.require(:application).permit( | |||
| :status, :notes, :regional_partner_filter, :response_scores, :locked | |||
| :status, :notes, :regional_partner_filter, :response_scores, :locked, :pd_workshop_id | |||
There was a problem hiding this comment.
This is causing an error in production:
ActiveModel::UnknownAttributeError: unknown attribute 'pd_workshop_id' for Pd::Application::Facilitator1819Application.
Augment the workshop serializer to provide a friendly short name for the dropdown
Build a SummerWorkshopPicker loader to call the workshop filter and get all summer workshops
Augment filter to allow for multiple subjects (we may want to do this for other workshop criteria
Restyle the control because I didn't like it
Fix all the tests I broke