Rename Academic Year Workshop subjects #36138
Conversation
| scheduled_start_on_or_after(CUTOFF_DATE) | ||
|
|
||
| csd_workshops.each do |csd_workshop| | ||
| csd_workshop.update(subject: subject_name[:from]) |
There was a problem hiding this comment.
This was done more efficiently in a single update_all last year -- unfortunately, suppress_email is a serialized attribute and I couldn't figure out how to easily update a non-serialized and a serialized attribute at the same time.
There was a problem hiding this comment.
I think this is fine, given the relatively small number of workshop rows.
bencodeorg
changed the title
| csd_workshops.each do |csd_workshop| | ||
| csd_workshop.update( | ||
| subject: subject_name[:to], | ||
| suppress_email: true |
There was a problem hiding this comment.
should we also set the workshop to virtual if it was one of Virtual 1/3/5/7?
There was a problem hiding this comment.
Sounds reasonable to me!
| # with subject naming we'll leave the email suppression as-is. | ||
| def down | ||
| CSD_SUBJECTS.each do |subject_name| | ||
| csd_workshops = Pd::Workshop.where(course: CSD_COURSE, subject: subject_name[:to]). |
There was a problem hiding this comment.
this will make it so all workshops go back to 'Workshop x: Unit y' format, right? Since those are first in the list? It's probably ok, but would be good to call out
There was a problem hiding this comment.
Good catch, will add comment.
| csd_workshops.each do |csd_workshop| | ||
| csd_workshop.update( | ||
| subject: subject_name[:to], | ||
| suppress_email: true |
There was a problem hiding this comment.
Note that I don't think adding suppress_email is strictly necessary here, as I think we trigger sending updates in the update controller action, which wouldn't get called in this case? We have a follow-up to have all AYW suppress email though, so I figured it couldn't hurt.
| SUBJECT_WORKSHOP_2 = 'Academic Year Workshop 2'.freeze | ||
| SUBJECT_WORKSHOP_3 = 'Academic Year Workshop 3'.freeze | ||
| SUBJECT_WORKSHOP_4 = 'Academic Year Workshop 4'.freeze | ||
| SUBJECT_WORKSHOP_5 = 'Academic Year Workshop 1 + 2'.freeze |
There was a problem hiding this comment.
maybe rename these last two to SUBJECT_WORKSHOP_1_2 and SUBJECT_WORKSHOP_3_4? 5 and 6 are a little confusing.
There was a problem hiding this comment.
Ah I see we used the 1-6 version last year. I think we can be more explicit this year since the names are simpler
There was a problem hiding this comment.
Yeah agree these are confusing, I wasn't 100% confident in my find and replace for these as they get moved into JS constants as well, so I kind of kicked the can. I can update though.
There was a problem hiding this comment.
Do you think maybe there's a benefit to leaving as-is, such that they're tied to the LEGACY_SUBJECT_CSP_WORKSHOP_1, etc? Or could change the names of the legacy variables as well...
There was a problem hiding this comment.
Meh actually looks like we've updated these variable names a few times, so not sure there's much historical continuity to maintain. I'll just try to do what makes sense for now.
|
@molly-moen made some updates to hopefully address your comments. Take another look when you have a chance! |
| @@ -47,6 +47,7 @@ def up | |||
| csd_workshops.each do |csd_workshop| | |||
| csd_workshop.update( | |||
| subject: subject_name[:to], | |||
| virtual: true, | |||
There was a problem hiding this comment.
this will always set to virtual right? I think we should only do that if it's one of virtual 1/3/5/7
Updated workshop subjects for 2020-21 academic year. This PR:
Largely based on #30203 (PR to do something very similar from this time last year), as well as follow ups #30270 and #30264.
Note this doesn't fully deprecate the "Virtual Workshop #" subjects -- some additional cleanup is worth doing soon. A couple items to note for that cleanup:
Links
Testing story
I followed the precedent set last year in testing, namely:
Run
upanddownfunctions from migration code. Compare workshop counts by courses & subjects before and after executing each function.UI tests: start rails server and check:
http://localhost-studio.code.org:3000/pd/workshop_dashboard/workshops/filter?course=CS+Principles®ional_partner_id=allhttp://localhost-studio.code.org:3000/pd/workshop_dashboard/workshops/newhttp://localhost-studio.code.org:3000/rails/mailers/--> *csd* or *csp* previews.Reviewer Checklist: