Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warnings headers #73

Merged
merged 1 commit into from
Jan 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 25 additions & 16 deletions canonicalwebteam/discourse/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,29 @@ def init_app(self, app):

app.register_blueprint(self.blueprint, url_prefix=self.url_prefix)

def _set_parser_warnings(self, response):
"""
Append parser warnings to the reponse headers

:param response: A flask response object
"""

# To not make the response too big
# we show only the last ten warnings
warnings = self.parser.warnings[-10:]

for message in warnings:
flask.current_app.logger.warning(message)
response.headers.add(
"discourse-warning",
message,
)

# Reset parser warnings
self.parser.warnings = []

return response


class Docs(Discourse):
"""
Expand Down Expand Up @@ -176,14 +199,7 @@ def document_view(path=""):
)
)

for message in self.parser.warnings:
flask.current_app.logger.warning(message)
response.headers.add(
"Warning",
f'199 canonicalwebteam.discourse "{message}"',
)

return response
return self._set_parser_warnings(response)


class Tutorials(Discourse):
Expand Down Expand Up @@ -263,14 +279,7 @@ def document_view(path=""):
)
)

for message in self.parser.warnings:
flask.current_app.logger.warning(message)
response.headers.add(
"Warning",
f'199 canonicalwebteam.discourse "{message}"',
)

return response
return self._set_parser_warnings(response)


class EngagePages(Discourse):
Expand Down
20 changes: 18 additions & 2 deletions canonicalwebteam/discourse/parsers/docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,12 +140,28 @@ def _parse_topic_url_map(
pretty_path = row.select_one("td:nth-of-type(2)").text
topic_a = row.select_one("td:last-child a[href]")

if not topic_a or not pretty_path:
# URL has a path but is not linked to a topic
if pretty_path and not topic_a:
self.warnings.append(
f"Could not parse URL map item {pretty_path}:{topic_a}"
f"Missing topic link for {pretty_path}"
)
continue

# There is a link to a topic without path
if topic_a and not pretty_path:
topic_url = topic_a.attrs.get("href", "")

# It's fine to not specify a path for the main topic
if self.index_topic != self._get_url_topic_id(topic_url):
self.warnings.append(
f"Missing topic path for {topic_url}"
)
continue

# No need to map them when missing
if not topic_a or not pretty_path:
continue

topic_url = topic_a.attrs.get("href", "")
topic_path = urlparse(topic_url).path
topic_match = TOPIC_URL_MATCH.match(topic_path)
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

setup(
name="canonicalwebteam.discourse",
version="3.0.1",
version="3.0.2",
author="Canonical webteam",
author_email="webteam@canonical.com",
url="https://github.com/canonical-webteam/canonicalwebteam.docs",
Expand Down
6 changes: 4 additions & 2 deletions tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,10 @@ def test_broken_redirects(self):
response_3.headers["location"], "http://localhost/target"
)

# Check we have 3 "Warning" headers from the broken mapping
self.assertEqual(len(response_1.headers.get_all("Warning")), 3)
# Check we have 3 "discourse-warning" headers from the broken mapping
self.assertEqual(
len(response_1.headers.get_all("discourse-warning")), 3
)

def test_document_not_found(self):
"""
Expand Down