Authentication header xsd validations #5275
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if ($(this).is(':checked')) { | ||
| $($(this).attr('data-follow-up')).find('input, select').attr('disabled', false); | ||
| $($(this).attr('data-follow-up')).show(); | ||
| var $input = $(this); |
There was a problem hiding this comment.
The question-follow-up pattern only assumes that there is one option with the show follow-up option.
So if two or more radio buttons have data-follow-up="#id-details-fields", the final iteration might be one that is not checked—so it will hide #id-details-fields. This change makes it so that we will load the follow-up if any of the options are checked, not just the last one.
There was a problem hiding this comment.
I know that @mrotondo and @anisharamnani worked with this code for the Idaho grocery credit - wondering if one of them could give a closer review
There was a problem hiding this comment.
@mrotondo and i paired on reviewing this.
@embarnard, oof good catch on this bug! this is quite hairy code, so thank you for updating it.
do you mind adding a comment at line 115 just to explain that two questions can refer to the same followup? since this is so tricky, a comment would help.
for future notice, @mrotondo and i spoke about moving this logic to the update command, but that doesn't need to be included in this PR.
There was a problem hiding this comment.
definitely! Thank you for reviewing it so thoroughly
| if ($(this).is(':checked')) { | ||
| $($(this).attr('data-follow-up')).find('input, select').attr('disabled', false); | ||
| $($(this).attr('data-follow-up')).show(); | ||
| var $input = $(this); |
There was a problem hiding this comment.
I know that @mrotondo and @anisharamnani worked with this code for the Idaho grocery credit - wondering if one of them could give a closer review
| @@ -42,7 +42,7 @@ def document | |||
| xml.IPv6AddressTxt device_info&.ip_address if device_info&.ip_address&.ipv6? | |||
| end | |||
| xml.IPTs datetime_type(device_info&.updated_at) | |||
| xml.DeviceId device_info&.device_id || 'AB' * 20 | |||
| xml.DeviceId /^[A-F0-9]{40}$/.match?(device_info&.device_id) ? device_info&.device_id : 'AB' * 20 | |||
There was a problem hiding this comment.
can we write a test for this?
There was a problem hiding this comment.
@anisharamnani done! thanks for adding that, it actually helped me catch that I hadn't added it to the initial device info block so glad you pointed that out
There was a problem hiding this comment.
i made two comments--1 to add a test & the other to add a comment to honeycrisp.js. neither of them are blocking. thank you @embarnard!
Link to pivotal/JIRA issue
Is PM acceptance required? (delete one)
Reminder: merge main into this branch and get green tests before merging to main
What was done?
How to test?
Screenshots (for visual changes)
Added validation to id field
This is how I reviewed the XML field validations for auth header:
