-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
1113 Register must include location and made available to local leaders #1279
Conversation
✅ Deploy Preview for cyf-tracks ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cyf-curriculum canceled.
|
✅ Deploy Preview for cyf-sdc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cyf-launch ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cyf-programming ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cyf-common ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cyf-itd ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for cyf-piscine ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
You'll also need to update org-cyf-itp/tooling/netlify/functions/submission-created.js
to get the new field (and stop getting the removed field), and insert it into the row
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.
Looks good to me.
Will this automatically update on the spreadsheet you created?
And will this spreadsheet also then include all other courses (ITD, Piscine, etc)?
It can do. It doesn't yet because that wasn't in scope. But we can do that. |
I can create a new ticket to update the spreadsheet. This way we can deliver this ticket. Thank you. Happy for it to be merged. |
40252cd
to
f29a303
Compare
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.
I made the changes - approving because I'm happy with them, but will let @SallyMcGrath merge
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.
I'm merging but maybe we come back and tag this for a refactor. I don't think all this logic makes sense in this view
What does this change?
pulls location data from course data files stored in org-cyf-theme
removes family name because it's too many fields
Common Content?
courses data files
Common Theme?
Fixes #1113
Register includes locations
Checklist
Who needs to know about this?