Link enrollment to application in workshop view

[hacodeorg]

In the workshop view, add links from enrollments to the corresponding applications. This feature only works for CSD and CSP workshops. (CSF workshop doesn't require applications.)

Before

After

Links

• PLC-747
• Teacher application spec

Testing story

• Unit tested
• Manually tested

  include FactoryGirl::Syntax::Methods
  csd = create :csd_academic_year_workshop
  csp = create :csp_summer_workshop
  u = create :teacher
  csda = create :pd_teacher2021_application, user: u, pd_workshop_id: csd.id
  cspa = create :pd_teacher1920_application, user: u, pd_workshop_id: csp.id
  
  csde = create :pd_enrollment, user: u, workshop: csd
  cspe = create :pd_enrollment, user: u, workshop: csp
  
  # Then go to these pages to verify links show up correctly
  "http://localhost-studio.code.org:3000/pd/workshop_dashboard/workshops/#{csd.id}"
  "http://localhost-studio.code.org:3000/pd/workshop_dashboard/workshops/#{csp.id}"

Reviewer Checklist:

• Tests provide adequate coverage
• Code is well-commented
• New features are translatable or updates will not break translations
• Relevant documentation has been added or updated
• User impact is well-understood and desirable
• Pull Request is labeled appropriately
• Follow-up work items (including potential tech debt) are tracked and linked

[molly-moen]

is there a reason you can't search on user id and application id together?

[islemaster]

I think it's because pd_application_id is not a column in our database, but a serialized property inside the JSON properties column for this application record. We can probably use MySQL 5 JSON features for this, but ActiveRecord won't do it for us.

I was wondering if you can use Pd::Application::TeacherApplicationBase here, which provides pd_workshop_id so you don't need the try below. I'm curious to see if/how that modifies the generated query here.

A bigger perf concern, for me, is that using this method and the enrollment serializer is introducing a query-per-enrollment for a workshop page load, which might be a performance issue for very large workshops. We probably don't need to overthink this yet, but let's keep an eye on this feature - if the workshop detail view noticeably slows down, we probably want to pull this logic out to the controller and look up all applications for all enrollments in one query. (Or figure out a change to our data model that makes this less painful.)

[islemaster]

In fact, thinking on this a little more, it does feel like pd_application_id really ought to be a column on enrollment, not a serialized property, with an ActiveRecord association defined.

[bencodeorg]

To play devil's advocate, the only time we care about associating an enrollment with an application is in the case of a teacher enrolling in a 5-day (CSP/D) summer workshop, right? That's a really small (albeit important) fraction of the overall number of enrollments, so it'd be like 95% null.

[hacodeorg]

Brad's explanation is correct. pd_application_id is a property in property bag, which is saved as a string data type in pd_applications table.

Using TeacherApplicationBase instead of ApplicationBase will not search the entire pd_applications table. It will default to type = TeacherApplicationBase. pd_applications is 1 table shared by multiple models (Single Table Inheritance).

I'll add a JIRA task with potential workarounds Brad and I discussed.

[hacodeorg]

@bencodeorg: Agree, the number of of enrollments can be mapped to applications is small. The problem is when you open a workshop view, it will grab all enrollments for that workshop in 1 query. However, for each enrollment, we then make 1 more query to pd_applications table to get matching application_id (no matter if it exists or not). If there is 100 enrollments in a workshop, it will create a total of 101 queries instead of 1. This could be a potential performance issue.

[hacodeorg]

Jira task created: https://codedotorg.atlassian.net/browse/PLC-809

[islemaster]

Absolute url (production-only) is blocking.

[islemaster]

👍 I think we can ship this, and follow up with iterations on the UX or performance if necessary. Thanks Ha!