1510020 - Use SITE_URL in absolutify filter

Status: RESOLVED FIXED

(developer.mozilla.org Graveyard :: Code Cleanup, enhancement)

Description

[John Whitlock [:jwhitlock]]

What feature should be changed? Please provide the URL of the feature if possible.
==================================================================================
The absolutify filter (bug 1121051) uses Django's current Site, and assumes the protocol is https. There is an optional site parameter to override the site, unused in code.

This should change to use the setting SITE_URL, and drop the optional parameter.

What problems would this solve?
===============================
Using Django's Site requires a database read, although the value is cached for future calls.

In development, SITE_URL is http://localhost:8000. The assumption that the protocol is https creates broken URLs in development.


We don't intend to use Django's Sites framework, so this flexibility goes unused.

Who would use this?
===================
Developers



What would users see?
=====================
Infinitesimally faster tests, since some tests would no longer require a database.

Absolute links would work in development.

What would users do? What would happen as a result?
===================================================
We'd use absolutify as a filter or function more often.

Is there anything else we should know?
======================================

Comment 1

[[github robot]]

Commits pushed to master at https://github.com/mozilla/kuma

https://github.com/mozilla/kuma/commit/df50a449b04cbe97064a6d79a80d1e3dad7ba04d
bug 1510020: Convert to pytest style

https://github.com/mozilla/kuma/commit/c8839358a0d58668f140750b72bdeabad7784057
bug 1510020: Avoid using Site or assuming in tests

Some tests can be re-written to avoid using or overridding
Site.get_current(), or assuming it is set to example.com, before
changing the behaviour of absolutify.

https://github.com/mozilla/kuma/commit/0e7e98b066d93a03645ed5bf239389301afe1d1b
bug 1510020: Use settings.SITE_URL in absolutify

Instead of using Django's Sites framework (which we don't really use in
Kuma, and requires a database), use settings.SITE_URL in absolutify, and
drop the unused optional parameter.

A few tests need to be changed at the same time, since they rely on the
old behavior of absolutify.

https://github.com/mozilla/kuma/commit/ffcdbb311cf367fea120363eb1529aad05723f3c
bug 1510020: Allow from_url to work on full URLs

In TravisCI, kuma/settings/travis.py is used for the "plain" py27 build,
and docker-compose.yml for the docker build.

In the py27 build, DOMAIN is "developer-local.allizom.org" and
SITE_URL is "https://developer-local.allizom.org". The DOMAIN matches the
netloc parsed from SITE_URL.

In the docker build, DOMAIN is "localhost" but SITE_URL is
"http://localhost:8000". The DOMAIN does not match the parsed netloc of
"localhost:8000".

When Site.get_current() was used, the domain was "example.com", and
doc.get_full_url() returned "https://example.com/en-US/docs/<SLUG>". The
method Document.from_url() always saw a different domain from
settings.DOMAIN, and return None for the related document.

When doc.get_full_url() started using settings.SITE_URL, the py27 build
saw a domain match, and Document.from_url() returned doc. The docker
build saw a mismatch between "localhost" and "localhost:8000", and
continued returning None.

This commit changes the tests for Document.from_url() and the base
implementation in get_doc_components_from_url(). If the passed URL has a
matching domain to SITE_URL, then process it. If it doesn't, then reject
it as a possible Document URL.

https://github.com/mozilla/kuma/commit/6aa6b573abd0e993d3b0a5d8e73f0536d678897b
Merge pull request #5138 from jwhitlock/absolutify-1510020

bug 1510020: In absolutify, use settings.SITE_URL instead of Site.get_current()

Comment 2

[John Whitlock [:jwhitlock]]

Deployed