Add a style guide to suggest to pass a string literal as a URL to http method calls

[moznion]

Original discussion: #328

[pirj]

I’m in general in favour of this change, but I admit that I’m biased because I’m mostly working on APIs, and the general picture may not be as simple as the limited use case I got used to.

Rails guides say (about the use of path helpers in views):

patient_path(@patient) then the router will generate the path /patients/17. This reduces the brittleness of your view and makes your code easier to understand

A lot of to digest in this brief note:

1. Reduces brittleness. Path helpers add an abstraction over a detail, the URL path. It’s a design decision whether those paths should be stable or can be bent at will. For the public API or pages that will be bookmarked, shared or referenced from other resources - it makes sense to keep stable. For internal APIs, APIs for own mobile apps or web - not really.
2. Easier to understand. I don’t really take this argument. Pretty sure this can be stretched in a way that the URL will be easier to understand than a path helper.

What is the source of the brittleness?
A mistake causing a change to the URL. Or a deliberate routes refactoring.
It can indeed cause views and redirects to point to the outdated route.
But with tests?

Should we introduce a concept of route stability, and say that this only makes sense as a measure to ensure route stability? And otherwise path helpers are preferrable to reduce brittleness?

What do you think?

[moznion]

I appreciate your explanation.

Easier to understand. I don’t really take this argument. Pretty sure this can be stretched in a way that the URL will be easier to understand than a path helper.

Yeah, I’m on the same page, and the main motivation comes from this.

Anyway, my understanding of the term “brittleness” is related to path-traversal attacks, where malicious “URL-ish” arguments are injected. I completely agree with the approach of using helpers with URL escaping on the given argument to protect against this type of vulnerability.
However, in testing, this shouldn’t be a problem since it’s just a test. Personally, I believe that other issues, like breaking changes to endpoints, should be caught by the tests.

[dvandersluis]

Based on #328 shouldn't this section mention that it applies to tests and not production code?

[moznion]

@dvandersluis Thank you for your suggestion; it sounds great to me. However, what would be the best approach: should I include that in the description, or move it under the “Testing” section?

[dvandersluis]

Putting it under Testing would make sense to me!

[moznion]

@dvandersluis Thank you, I moved it: 1c4cf0e

[andyw8]

@moznion you might also want to mention tests in the PR title, I was confused the first time reading this.