help: Clean up settings_html and subscriptions_html.

After some thinking, I don't think there's any actual value to doing
the ../ style relative links here, whereas there is actual harm from
the links being slightly broken in the current model.  We fix this by
just using /#settings as the URL.

Fixes #8978.
This commit is contained in:
Tim Abbott
2018-04-05 14:39:35 -07:00
parent a29b1c1569
commit b0b134cb4c
3 changed files with 8 additions and 10 deletions

View File

@@ -44,9 +44,7 @@ class HelpDocumentationSpider(UnusedImagesLinterSpider):
name = "help_documentation_crawler" name = "help_documentation_crawler"
start_urls = ['http://localhost:9981/help'] start_urls = ['http://localhost:9981/help']
deny_domains = [] # type: List[str] deny_domains = [] # type: List[str]
deny = ['/privacy', deny = ['/privacy']
# Ignored because scrapy seems to not process relatively URLs with ../ properly.
'[.][.]/']
images_path = "static/images/help" images_path = "static/images/help"

View File

@@ -128,7 +128,7 @@ class HelpTest(ZulipTestCase):
def test_html_settings_links(self) -> None: def test_html_settings_links(self) -> None:
result = self.client_get('/help/message-a-stream-by-email') result = self.client_get('/help/message-a-stream-by-email')
self.assertEqual(result.status_code, 200) self.assertEqual(result.status_code, 200)
self.assertIn('<a target="_blank" href="../../#streams">streams page</a>', str(result.content)) self.assertIn('<a target="_blank" href="/#streams">streams page</a>', str(result.content))
class IntegrationTest(TestCase): class IntegrationTest(TestCase):
def test_check_if_every_integration_has_logo_that_exists(self) -> None: def test_check_if_every_integration_has_logo_that_exists(self) -> None:
@@ -174,19 +174,19 @@ class IntegrationTest(TestCase):
add_api_uri_context(context, HostRequestMock(host="mysubdomain.testserver")) add_api_uri_context(context, HostRequestMock(host="mysubdomain.testserver"))
self.assertEqual( self.assertEqual(
context['settings_html'], context['settings_html'],
'<a href="../../#settings">Zulip settings page</a>') '<a href="/#settings">Zulip settings page</a>')
self.assertEqual( self.assertEqual(
context['subscriptions_html'], context['subscriptions_html'],
'<a target="_blank" href="../../#streams">streams page</a>') '<a target="_blank" href="/#streams">streams page</a>')
context = dict() context = dict()
add_api_uri_context(context, HostRequestMock()) add_api_uri_context(context, HostRequestMock())
self.assertEqual( self.assertEqual(
context['settings_html'], context['settings_html'],
'<a href="../../#settings">Zulip settings page</a>') '<a href="/#settings">Zulip settings page</a>')
self.assertEqual( self.assertEqual(
context['subscriptions_html'], context['subscriptions_html'],
'<a target="_blank" href="../../#streams">streams page</a>') '<a target="_blank" href="/#streams">streams page</a>')
class AboutPageTest(ZulipTestCase): class AboutPageTest(ZulipTestCase):
def setUp(self) -> None: def setUp(self) -> None:

View File

@@ -37,8 +37,8 @@ def add_api_uri_context(context: Dict[str, Any], request: HttpRequest) -> None:
context["html_settings_links"] = html_settings_links context["html_settings_links"] = html_settings_links
if html_settings_links: if html_settings_links:
settings_html = '<a href="../../#settings">Zulip settings page</a>' settings_html = '<a href="/#settings">Zulip settings page</a>'
subscriptions_html = '<a target="_blank" href="../../#streams">streams page</a>' subscriptions_html = '<a target="_blank" href="/#streams">streams page</a>'
else: else:
settings_html = 'Zulip settings page' settings_html = 'Zulip settings page'
subscriptions_html = 'streams page' subscriptions_html = 'streams page'