Automatically assign workshops on application submission

[Hamms]

If we can find an applicable workshop, based on the teacher's assciated
regional partner

[aoby]

Some group 3 partners have local summer workshops, instead of teachercon :(

Probably better to check the form data for regional_partner_workshop_ids. You could also check form data for teachercon instead of calling get_matching_teachercon directly (note the API that supplies that teachercon text uses the same call)

[Hamms]

Right, but aren't those partners just unrepresented in get_matching_teachercon, and so we'll fall through to our base case?

[aoby]

Presumably yes. Good point.

[aoby]

Do we want this "assignment" to only be tracked by the external enrollment? As it stands this is not editable / retrievable here. We can call workshop again, but if the enrollment changes, it won't reflect the actual assigned workshop.

I'd like to see this be idempotent and retrievable. I suggest changing workshop (maybe rename to assigned_workshop?) to inspect enrollments and return the one that is enrolled if any, rather than first, with first as a default when none are enrolled. Then, with assign_workshop! (maybe renamed enroll_in_assigned_workshop?), we should use find_or_create_by! rather than new.

We could make it even more flexible for reassignment by allowing an optional argument for the specific workshop to be assigned and first deleting any other relevant enrollments (same course and subject).

What do you think?

[Hamms]

I agree that this should be explicitly set rather than inferred.

I think the answer to how best to build this is actually going to kind of depend on the answer to whether we need to filter by course and/or subject. Is it the case that all the workshops we will want to be assignable to a teacher application are going to share some data, or do we want to allow partners to assign whatever workshop they want? I think it's gonna be the latter, since some of the G3-associated teachers are going to end up going to Local Summer Workshops and we'll want to handle those differently.

If we want that flexibility, it seems to me that we'll want an actual direct relation from application to workshop or enrollment, and assign_workshop! is refactored to something like self.enrollment = Pd::Enrollment.new(...) unless self.enrollment

[aoby]

The teachercon vs. local summer thing does muddy this up a bit, but the course should always be the applied-for course (CSD vs. CSP), exposed as course in the application, though it will need to be translated to be used in a workshop query (see

code-dot-org/dashboard/app/models/pd/teacher_application.rb

Line 25 in 7b2913f

name: Pd::Workshop::COURSE_CSD,

).

I suppose subject could be the set of

[
  Pd::Workshop::SUBJECT_TEACHER_CON, 
  Pd::Workshop::SUBJECT_FIT
]

[aoby]

Don't we need to filter this by course and subject?
And isn't the association pd_workshops_organized (rather than workshops)?

[Hamms]

d'oh, yes, that is definitely the association.

The first question is probably one for Tanya; I built it off of this comment but it does seem like we'll want to do some kind of filtering.

[aoby]

I think that comment implies workshop to mean of the relevant course (CSD vs. CSP) and subject (5-day summer).

[Hamms]

@aoby what do you think of this approach? We associate the application with a workshop explicitly, and upon creation try to find a default workshop. Then we just need to add the ability to edit the associated workshop and we should be good to go

[aoby]

@Hamms I like this approach. It seems cleaner.

One question: might it be better to instead add a properties field and include SerializedProperties so we can more flexibly store fields like this with the applications outside form data, rather than explicitly adding pd_workshop_id? For instance, facilitator applications don't need a workshop, but might want to store some other kind of data. Thoughts?

[Hamms]

@aoby PTAL

This feature still needs tests; in the interest of unblocking everyone else, I'm inclined to do them in a separate PR. Any opposition?

[aoby]

It's find_or_create_by (missing _by at the end), and we might as well use the bang version find_or_create_by!

Note there is also a first_or_create, but it works on a relation (where clause) and I find it to be less readable.

[aoby]

I don't think we want FIT workshops included here for teacher application, do we? My understanding is those are only for facilitators, i.e. facilitator application.

[aoby]

LGTM (with a few suggestions and questions). I'd like to see tests as a followup, but I agree we should get this out to unblock others first.