New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CSF workshop attendance export to gdrive #39120
Conversation
def scholarship_teacher_attendance(workshop) | ||
num_teachers = 0 | ||
workshop.sessions.first.attendances.each do |attendance| | ||
if Pd::ScholarshipInfo.where(pd_enrollment_id: attendance.pd_enrollment_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't "no" a valid scholarship status? Would this mark those teachers as scholarship recipients?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also have this pretty similar method in the workshop model that I think I added for something similar last year, I wonder if it's worth generalizing / making sure that the way "scholarship attendance" is being calculated here is consistent with how we're doing the same thing elsewhere?
https://github.com/code-dot-org/code-dot-org/blob/staging/dashboard/app/models/pd/workshop.rb#L687
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh excellent point - I totally forgot "No" is a status! And thanks for pointing me to this method! This export was initially getting attendance for each session, but now that I don't need to do that, I can reuse the method from the model. Thanks!
workshop.organizer&.name, | ||
workshop.organizer&.email, | ||
workshop.regional_partner&.name, | ||
workshop.sessions.map(&:formatted_date_with_start_and_end_times).join("\n"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want new lines in the gsheet export?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General question: how does what is done here differ from what is produced in the various serializers we have for exporting workshop data? |
@@ -94,6 +94,8 @@ | |||
cronjob at:'* * * * *', do:deploy_dir('bin', 'cron', 'confirm_usage') | |||
cronjob at:'0 */2 * * *', do:deploy_dir('bin', 'cron', 'teacher_applications_to_gdrive') | |||
cronjob at:'30 */2 * * *', do:deploy_dir('bin', 'cron', 'summer_workshops_to_gdrive') | |||
# [Clare] Disabled so that we can do the first run manually on production-daemon, to update credentials. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you elaborate on this? is the plan to re-enable it in a followup PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! I will run it manually the first time, and if all goes well, I'll submit a follow up to enable this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This will export a summary of CSF workshop data to a google sheet. I followed the process detailed in #32597.
Links
Testing story
Same process as #32597:
Tested end-to-end locally with development secrets configured in Secret Manager.
Note that the configuration added in this PR does not configure development environments to pull anything from Secrets Manager; to do that for local testing, you have to add the following lines to your
config/develoment.yml.erb
file:Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: