Skip to content
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

Be lenient in what the feedback explorer can receive in its URL field #222

Merged
merged 4 commits into from Apr 30, 2015

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Apr 29, 2015

Allow full URLs with http and https, as well as partial URLs beginning with gov.uk or www.gov.uk, URLs users have entered with a bad protocol, or simply the plain path itself – with or without leading slash.

The following inputs all give the same results page:

Valid
https://www.gov.uk/vat-rates
http://www.gov.uk/vat-rates

Invalid
http//www.gov.uk/vat-rates
https//:www.gov.uk/vat-rates

Short hand
www.gov.uk/vat-rates
gov.uk/vat-rates
/vat-rates
vat-rates

Also

  • removes inline form validation – what the browser validation defines as a URL is too strict.
  • adds examples to the form

screen shot 2015-04-29 at 16 06 54

cc @rivalee @benilovj

expect(feedex_results).to eq(feedback_reports)

explore_anonymous_feedback_with(url: "vat-rates")
expect(feedex_results).to eq(feedback_reports)

This comment has been minimized.

@benilovj

benilovj Apr 30, 2015
Contributor

It'd be best to test this logic in a unit test, because here:

  • it makes the integration test harder to read
  • it distracts from the main use-case
  • it makes the integration tests slower (because they run through the UI)
@fofr fofr force-pushed the better-url-handling branch from 1352fbf to 3002d08 Apr 30, 2015
fofr added 4 commits Apr 29, 2015
Allow full URLs with http and https, as well as partial URLs beginning
with gov.uk or www.gov.uk, or simply the plain path itself.

The following examples all give the same results:
https://www.gov.uk/vat-rates
http://www.gov.uk/vat-rates
www.gov.uk/vat-rates
gov.uk/vat-rates
/vat-rates

Also removes inline form validation – what the browser validation
defines as a URL is too strict.
* Strip out variations on http
  eg http:/, http//:, http//, http:///
* Allow paths without a leading slash: treat `vat-rates` as if it were
`/vat-rates`
Avoid testing at integration stage as this is slower and obscures the
purpose of that test.
@benilovj benilovj force-pushed the better-url-handling branch from 3002d08 to d89da3d Apr 30, 2015
@benilovj
Copy link
Contributor

@benilovj benilovj commented Apr 30, 2015

👍

benilovj added a commit that referenced this pull request Apr 30, 2015
Be lenient in what the feedback explorer can receive in its URL field
@benilovj benilovj merged commit c0380d4 into master Apr 30, 2015
1 check passed
1 check passed
default "Build #159 succeeded on Jenkins"
Details
@benilovj benilovj deleted the better-url-handling branch Apr 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.