-
Notifications
You must be signed in to change notification settings - Fork 480
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
Add workshop box for teachers with RPs that don't offer CSA #45137
Conversation
workshops: | ||
partnerInfo && | ||
partnerInfo.summer_workshops.filter( | ||
workshop => workshop.course === 'CS Discoveries' | ||
workshop => workshop.course === `${ActiveCourseWorkshops.CSD}` |
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.
tiny nit: these are already strings, so they don't need to be wrapped
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.
otherwise, looks great!
@@ -303,9 +304,32 @@ class RegionalPartnerSearch extends Component { | |||
collection => collection.workshops.length === 0 | |||
) && <div>Workshop details coming soon!</div>} |
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.
maybe a weird quirk, if there are no workshops at all, it'll show this text and also the CSA text
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.
Strong point––I'll check with Tess what we want to do in that situation
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.
She says it's not a huge deal since this won't happen next year, but I'll add something for it
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.
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.
perfect!
Currently, if CSA workshops are available in their region, teachers will see this
If CSA workshops are not available in their region, teachers see this
With the new functionality, teachers see this if there are no CSA workshops (screenshot of local environment––I didn't set RP information for
appsPriorityDeadlineDate
so no call to action):And nothing changes if there are CSA workshops (screenshot of local environment––I didn't set RP information for
appsPriorityDeadlineDate
so no call to action):Links
Testing story
Deployment strategy
Follow-up work
I created a follow-up ticket for test coverage: https://codedotorg.atlassian.net/browse/PLAT-1650.
Description:
The RegionalPartnerSearch has 0 test coverage. Right now, trying to add test coverage results in errors about how we are setting state in the component:
Can't call %s on a component that is not yet mounted. This is a no-op, but it might indicate a bug in your application. Instead, assign to
this.statedirectly or define a
state = {};class property with the desired state in the %s component.
We should probably refactor this component to use hooks, which would allow us to avoid mounting the component to test it.
Privacy
Security
Caching
PR Checklist: