Skip to content

Commit 42efb27

Browse files
KevinMindeviljeff
authored andcommitted
Add Policies FAQ to links and restructure links to make them more obvious (#23689)
* Refactor documentation URLs in `docs` view and update agreement message in template - Replaced hardcoded URLs in the `docs` view with helper functions for better maintainability. - Updated the agreement message in the `agreement.html` template to include a link to the FAQ and improved wording. - Adjusted test case to reflect the change in the URL structure for the policies documentation. Refactor URL generation in `docs` view and enhance test assertions - Introduced a helper function `get_url` to streamline URL construction in the `docs` view. - Updated test cases to assert the presence of the `utm_referrer=amo` query parameter for 301 redirects. - Improved readability of the test case by unpacking the URL and status code directly in the loop. Update src/olympia/devhub/templates/devhub/includes/agreement.html Co-authored-by: Copilot <[email protected]> TMP: better name for argument * TMP: fix broken link rendering false positives by adding actually referenced links to the docs link test * TMP: fix broken test
1 parent e58db64 commit 42efb27

File tree

3 files changed

+59
-30
lines changed

3 files changed

+59
-30
lines changed

src/olympia/devhub/templates/devhub/includes/agreement.html

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
<form method="post">
22
{% if not agreement_form.has_error('__all__') %}
33
<p>
4-
{{ agreement_message }}
4+
{% trans faq_url=url('devhub.docs', 'policies/faq'), a_attrs='target="_blank" rel="noopener noreferrer"' %}
5+
Before starting, please read and accept our Firefox Add-on Distribution Agreement, Review Policies and Rules and our <a {{ a_attrs }} href="{{ faq_url }}">Frequently Asked Questions</a>. The Firefox Add-on Distribution Agreement also links to our Privacy Notice which explains how we handle your information.
6+
{% endtrans %}
57
</p>
68
<ul class="agreement-links">
79
<li>{{ agreement_form.distribution_agreement }}<a href="{{ url('devhub.docs', 'policies/agreement') }}" target="_blank" rel="noopener noreferrer">{{ _('Firefox Add-on Distribution Agreement') }}</a> {{ agreement_form.distribution_agreement.errors }}</li>
8-
<li>{{ agreement_form.review_policy }}<a href="{{ url('devhub.docs', 'policies/reviews') }}" target="_blank" rel="noopener noreferrer">{{ _('Review Policies and Rules') }}</a> {{ agreement_form.review_policy.errors }}</li>
10+
<li>{{ agreement_form.review_policy }}<a href="{{ url('devhub.docs', 'policies') }}" target="_blank" rel="noopener noreferrer">{{ _('Review Policies and Rules') }}</a> {{ agreement_form.review_policy.errors }}</li>
911
</ul>
1012
<p>
1113
{{ _('I have read and accept this Agreement and the Rules and Policies') }}.

src/olympia/devhub/tests/test_views.py

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2054,22 +2054,39 @@ def test_doc_urls(self):
20542054
assert '/en-US/developers/docs/te' == reverse('devhub.docs', args=['te'])
20552055
assert '/en-US/developers/docs/te/st', reverse('devhub.docs', args=['te/st'])
20562056

2057+
self.client.force_login(user_factory(read_dev_agreement=None))
2058+
response = self.client.get(reverse('devhub.submit.agreement'))
2059+
doc = pq(response.content)
2060+
2061+
# Extract the last part of the docs URL path as the "name" for each link.
2062+
links = [
2063+
a.attr['href'].rstrip('/').split('/developers/docs/', 1)[-1] or ''
2064+
for a in doc('a').items()
2065+
if '/developers/docs/' in a.attr['href']
2066+
]
2067+
2068+
assert len(links) == 3
2069+
20572070
urls = [
2058-
(reverse('devhub.docs', args=['getting-started']), 301),
2059-
(reverse('devhub.docs', args=['how-to']), 301),
2060-
(reverse('devhub.docs', args=['how-to/other-addons']), 301),
2061-
(reverse('devhub.docs', args=['fake-page']), 404),
2062-
(reverse('devhub.docs', args=['how-to/fake-page']), 404),
2071+
# None link
20632072
(reverse('devhub.docs'), 301),
2073+
# Valid links
2074+
*[(reverse('devhub.docs', args=[link]), 301) for link in links],
2075+
# Invalid links
2076+
(reverse('devhub.docs', args=['fake-page']), 404),
2077+
*[(reverse('devhub.docs', args=[f'fake-{link}']), 404) for link in links],
20642078
]
20652079

20662080
index = reverse('devhub.index')
20672081

2068-
for url in urls:
2069-
response = self.client.get(url[0])
2070-
assert response.status_code == url[1]
2082+
for [url, status_code] in urls:
2083+
response = self.client.get(url)
2084+
assert response.status_code == status_code
2085+
2086+
if status_code == 301:
2087+
assert 'utm_referrer=amo' in response.url
20712088

2072-
if url[1] == 302: # Redirect to the index page
2089+
if status_code == 302: # Redirect to the index page
20732090
self.assert3xx(response, index)
20742091

20752092

src/olympia/devhub/views.py

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import os
33
import time
44
from copy import deepcopy
5-
from urllib.parse import quote
5+
from urllib.parse import quote, urlencode, urljoin
66
from uuid import UUID, uuid4
77

88
from django import forms as django_forms, http
@@ -72,7 +72,6 @@
7272
from olympia.reviewers.models import Whiteboard
7373
from olympia.reviewers.utils import ReviewHelper
7474
from olympia.users.models import (
75-
DeveloperAgreementRestriction,
7675
SuppressedEmailVerification,
7776
)
7877
from olympia.users.tasks import send_suppressed_email_confirmation
@@ -1952,26 +1951,38 @@ def request_review(request, addon_id, addon):
19521951

19531952

19541953
def docs(request, doc_name=None):
1954+
def get_url(base, doc_path=None):
1955+
base_url = urljoin(base, doc_path)
1956+
query = urlencode({'utm_referrer': 'amo'})
1957+
return f'{base_url}?{query}'
1958+
1959+
def mdn_url(doc_path):
1960+
return get_url(MDN_BASE, doc_path)
1961+
1962+
def ext_url(doc_path):
1963+
return get_url(settings.EXTENSION_WORKSHOP_URL, doc_path)
1964+
19551965
mdn_docs = {
1956-
None: '',
1957-
'getting-started': '',
1958-
'reference': '',
1959-
'how-to': '',
1960-
'how-to/getting-started': '',
1961-
'how-to/extension-development': '#Extensions',
1962-
'how-to/other-addons': '#Other_types_of_add-ons',
1963-
'how-to/thunderbird-mobile': '#Application-specific',
1964-
'how-to/theme-development': '#Themes',
1965-
'themes': '/Themes/Background',
1966-
'themes/faq': '/Themes/Background/FAQ',
1967-
'policies': '/AMO/Policy',
1968-
'policies/reviews': '/AMO/Policy/Reviews',
1969-
'policies/contact': '/AMO/Policy/Contact',
1970-
'policies/agreement': '/AMO/Policy/Agreement',
1966+
None: mdn_url(''),
1967+
'getting-started': mdn_url(''),
1968+
'reference': mdn_url(''),
1969+
'how-to': mdn_url(''),
1970+
'how-to/getting-started': mdn_url(''),
1971+
'how-to/extension-development': mdn_url('#Extensions'),
1972+
'how-to/other-addons': mdn_url('#Other_types_of_add-ons'),
1973+
'how-to/thunderbird-mobile': mdn_url('#Application-specific'),
1974+
'how-to/theme-development': mdn_url('#Themes'),
1975+
'themes': mdn_url('/Themes/Background'),
1976+
'themes/faq': mdn_url('/Themes/Background/FAQ'),
1977+
'policies': ext_url('/documentation/publish/add-on-policies'),
1978+
'policies/faq': ext_url('/documentation/publish/add-on-policies-faq'),
1979+
'policies/agreement': ext_url(
1980+
'/documentation/publish/firefox-add-on-distribution-agreement'
1981+
),
19711982
}
19721983

19731984
if doc_name in mdn_docs:
1974-
return redirect(MDN_BASE + mdn_docs[doc_name], permanent=True)
1985+
return redirect(mdn_docs[doc_name], permanent=True)
19751986

19761987
raise http.Http404()
19771988

@@ -2009,7 +2020,6 @@ def render_agreement(request, template, next_step, **extra_context):
20092020
# potential errors highlighted)
20102021
context = {
20112022
'agreement_form': form,
2012-
'agreement_message': str(DeveloperAgreementRestriction.error_message),
20132023
}
20142024
context.update(extra_context)
20152025
return TemplateResponse(request, template, context=context)

0 commit comments

Comments
 (0)