freedomofpress / securethenews

An automated scanner and web dashboard for tracking TLS deployment across news organizations
https://securethe.news
GNU Affero General Public License v3.0
100 stars 25 forks source link

Use absolute URLs for Sites in frontend JS #47

Closed garrettr closed 7 years ago

garrettr commented 7 years ago

Tradeoff: this wastes some space in the sites_json, since it repeats the URL prefix for each site. However, this should compress well so I don't think it will have a significant impact on network transfer performance.

Benefit: We can change sites.urls without breaking the frontend code.

Also fixes an issue where the leaderboard on the homepage had incorrect links to the site score pages because the leaderboard view was using relative links.

conorsch commented 7 years ago

Question, to help me understand the mechanics at work here: we don't have a THS for STN, but if we did, would this work well with it? That is, would Django infer the domain as the Onion URL correctly from the vhost and still append the routes to that domain, even given the use of absolute_url?

garrettr commented 7 years ago

@conorsch get_absolute_url returns the "absolute" URL relative to the site's root, e.g. /sites/the-new-york-times. This makes it useful for internal links, but not external ones. For external absolute URLs (e.g. to put in an email), there are other techniques that should be preferred.

In the description, the repeated URL prefix I am referring to is "/sites/", not "http://securethe.news/sites/" or whatever.

This change is useful here because previously in the frontend JS we were manually constructing the site score page URL manually based on the site's slug. get_absolute_url is the preferred technique in Django-land because it means there is one canonical source of truth for your URL configuration: urls.py. It's easy to use get_absolute_url throughout the Python views and templates; this code simply passes it along in the sites_json object so the frontend JS can take advantage of it as well.

garrettr commented 7 years ago

I agree that the terminology is somewhat confusing; get_absolute_url is a Django convention, but it is confusing because it doesn't actually return an absolute URL (including the origin) as they are commonly known.

conorsch commented 7 years ago

Fantastic, thanks for the explanation. Yes, this change seems a lot more durable for those editing the JS code and related templates. :+1: LGTM!