diff --git a/canonicalwebteam/discourse/app.py b/canonicalwebteam/discourse/app.py index 5f30805..8365e7f 100644 --- a/canonicalwebteam/discourse/app.py +++ b/canonicalwebteam/discourse/app.py @@ -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): """ @@ -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): @@ -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): diff --git a/canonicalwebteam/discourse/parsers/docs.py b/canonicalwebteam/discourse/parsers/docs.py index 6a81fc6..5718c55 100644 --- a/canonicalwebteam/discourse/parsers/docs.py +++ b/canonicalwebteam/discourse/parsers/docs.py @@ -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) diff --git a/setup.py b/setup.py index 4385020..dada60a 100755 --- a/setup.py +++ b/setup.py @@ -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", diff --git a/tests/test_app.py b/tests/test_app.py index ba840f6..05f6243 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -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): """