-
Notifications
You must be signed in to change notification settings - Fork 9
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 teacher degree apprenticeship filter to Manage #9406
Conversation
Database-level enum changes detected Please include a data migration for these attributes and values:
|
trait :teacher_degree_apprenticeship do | ||
apprenticeship | ||
full_time | ||
description { 'Teacher degree apprenticeship with QTS full time teaching apprenticeship' } |
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.
description { 'Teacher degree apprenticeship with QTS full time teaching apprenticeship' } | |
description { 'Teaching degree apprenticeship with QTS full time teaching apprenticeship' } |
Is it Teaching degree apprenticeship or Teacher degree apprenticeship? We are not consistent. it may be too late to change in some places (eg the programme_type), but we should make an effort where possible.
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.
Actually this comes from the content designer and from the prototype. I might keep as it is
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.
A couple of suggestions wordings 👌
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.
A couple of minor questions/suggestions but looks good, nice work 👍🏼
end | ||
|
||
it 'does include the course type filter' do | ||
expect(filter.filters.size).to be(7) |
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.
Is this adding value? If we add another filter this will fail?
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 just maintaned the same test that was there and added the tda filter. I think I will maintain this.
spec/system/provider_interface/provider_applications_filter_spec.rb
Outdated
Show resolved
Hide resolved
…ec.rb Co-authored-by: avinhurry <[email protected]>
Context
We need to make changes to Manage to allow providers to view applications with the a new type “Teacher degree apprenticeship (TDA) with QTS”.
Changes proposed in this pull request
Guidance to review
If you want to test:
Trello card
https://trello.com/c/HIsIqv1q/1728-add-filters-to-manage-for-tda