-
Notifications
You must be signed in to change notification settings - Fork 1
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
AQTS-632-FI-free-text-validation #2463
Conversation
Review app deployed to https://apply-for-qts-review-2463-web.test.teacherservices.cloud/personas |
validates :response, | ||
presence: { | ||
message: "Please respond to the assessor's request", | ||
} |
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 should use the I18n
locales instead.
Take a look at teacher_interface.en.yml
file for other examples.
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 shouldn't have to make any changes here.
@@ -14,7 +14,6 @@ | |||
let(:response) { nil } | |||
|
|||
it { is_expected.to validate_presence_of(:further_information_request_item) } | |||
it { is_expected.to validate_presence_of(:response) } |
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.
Why is this removed?
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.
If I don't remove it I get this error
and just searching around one ai chat bot said this
This might not be true because it's ai but I understood this to mean that this line I've removed is automatically checking and expecting the error message to be 'can't be blank' by default so I couldn't see a way of changing it. I didn't check any documentation for the 'shoulda-matchers' gem.
@@ -95,6 +95,9 @@ en: | |||
school_details_cannot_be_verified: Tell us more about this school | |||
unrecognised_references: Tell us more about your work history | |||
work_history_break: Tell us more about your work history | |||
further_information_request_item_text_form: | |||
response: | |||
blank: Please respond to the assessor's request |
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 can remove this now
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 💪
Thanks for addressing comments
I've updated the error message and hit text on the page where there is a free text response to and FI request
https://dfedigital.atlassian.net/jira/software/projects/AQTS/boards/207?selectedIssue=AQTS-632