From 570938847fb7a7a66e6c5e64414c7af527d756ab Mon Sep 17 00:00:00 2001 From: Carlos Wu Date: Wed, 10 Jul 2024 15:03:55 +0100 Subject: [PATCH] Add limit and offset to allow pagination for engage pages and takeovers (#193) --- README.md | 6 +- canonicalwebteam/discourse/app.py | 6 +- canonicalwebteam/discourse/exceptions.py | 21 +++++++ canonicalwebteam/discourse/models.py | 40 +++++++++---- setup.py | 2 +- ...tDiscourseAPI.test_index_ep_takeovers.yaml | 55 ++++++++++++++++++ ...ourseAPI.test_individual_ep_takeovers.yaml | 56 +++++++++++++++++++ .../TestDiscourseAPI.test_pagination.yaml | 55 ++++++++++++++++++ tests/test_engage_pages.py | 47 ++++++++++++++++ 9 files changed, 274 insertions(+), 14 deletions(-) create mode 100644 tests/cassettes/TestDiscourseAPI.test_index_ep_takeovers.yaml create mode 100644 tests/cassettes/TestDiscourseAPI.test_individual_ep_takeovers.yaml create mode 100644 tests/cassettes/TestDiscourseAPI.test_pagination.yaml diff --git a/README.md b/README.md index 9f9770e..50c1ae5 100644 --- a/README.md +++ b/README.md @@ -92,4 +92,8 @@ app.add_url_rule( Similarly for takeovers, you just need to pass `page_type="takeovers"`. - To get a list of takeovers `EngagePages(args).get_index()` also. -- To get a list of active takeovers `EngagePages(args).parse_active_takeovers()`. \ No newline at end of file +- To get a list of active takeovers `EngagePages(args).parse_active_takeovers()`. + +## Pagination +- `get_index` provides two additional arguments `limit` and `offset`, to provide pagination functionality. They default to 50 and 0 respectively. +- Use `MaxLimitError` in the `exceptions.py` to handle excessive limit. By default, it will raise an error when it surpasses 500 diff --git a/canonicalwebteam/discourse/app.py b/canonicalwebteam/discourse/app.py index 2d62e43..ede3bdc 100644 --- a/canonicalwebteam/discourse/app.py +++ b/canonicalwebteam/discourse/app.py @@ -308,14 +308,16 @@ def __init__( self.additional_metadata_validation = additional_metadata_validation pass - def get_index(self): + def get_index(self, limit=50, offset=0): """ Get the index topic and split it into: - index document content - URL map And set those as properties on this object """ - list_topics = self.api.engage_pages_by_category(self.category_id) + list_topics = self.api.engage_pages_by_category( + self.category_id, limit, offset + ) topics = [] for topic in list_topics: if topic[6] not in self.exclude_topics: diff --git a/canonicalwebteam/discourse/exceptions.py b/canonicalwebteam/discourse/exceptions.py index d202cb5..bce3acd 100644 --- a/canonicalwebteam/discourse/exceptions.py +++ b/canonicalwebteam/discourse/exceptions.py @@ -62,4 +62,25 @@ def __init__(self, *args: object) -> None: class DataExplorerError(Exception): + """ + Errors raised when the Data Explorer plugin for Discourse + returns an error. + + Will be sent to Sentry + """ + + def __init__(self, *args: object) -> None: + error_message = args[0] + flask.current_app.extensions["sentry"].captureMessage( + f"Engage pages Data Explorer error {error_message}" + ) + pass + + +class MaxLimitError(Exception): + """ + Error raised when limit/offset is too high + most likely spamming. + """ + pass diff --git a/canonicalwebteam/discourse/models.py b/canonicalwebteam/discourse/models.py index ab1fbeb..b52eb7e 100644 --- a/canonicalwebteam/discourse/models.py +++ b/canonicalwebteam/discourse/models.py @@ -1,4 +1,7 @@ -from canonicalwebteam.discourse.exceptions import DataExplorerError +from canonicalwebteam.discourse.exceptions import ( + DataExplorerError, + MaxLimitError, +) class DiscourseAPI: @@ -82,7 +85,7 @@ def get_topics_category(self, category_id, page=0): return response.json() - def engage_pages_by_category(self, category_id=50): + def engage_pages_by_category(self, category_id=50, limit=50, offset=0): """ This endpoint returns engage pages cooked content. This is possible with the Data Explorer plugin for Discourse @@ -93,7 +96,13 @@ def engage_pages_by_category(self, category_id=50): @params - category_id [int]: 50 by default, this is set in the https://discourse.ubuntu.com/admin/plugins/explorer?id=14 + - limit [int]: 50 by default, also set in data explorer + - offset [int]: 0 by default (first page) """ + # Avoid abuse and spamming of pagination + if limit > 500: + raise MaxLimitError() + headers = { "Accept": "application/json", "Content-Type": "multipart/form-data;", @@ -105,18 +114,24 @@ def engage_pages_by_category(self, category_id=50): f"{self.base_url}/admin/plugins/explorer/" f"queries/{data_explorer_id}/run", headers=headers, - data={"params": f'{{"category_id":"{category_id}"}}'}, + data={ + "params": ( + f'{{"category_id": "{category_id}", ' + f'"limit": "{limit}", "offset": "{offset}"}}' + ) + }, ) result = response.json() - - if not result["success"]: + try: + pages = result["rows"] + return pages + except KeyError: raise DataExplorerError(response["errors"][0]) - pages = result["rows"] - return pages - - def get_engage_pages_by_param(self, category_id, key, value): + def get_engage_pages_by_param( + self, category_id, key, value, limit=50, offset=0 + ): """ Uses data-explorer to query topics with the category Engages pages or Takeovers @@ -143,6 +158,10 @@ def get_engage_pages_by_param(self, category_id, key, value): key = path value = /engage/nfv-management-and-orchestration- charmed-open-source-mano + + Args: + - limit [int]: 50 by default, also set in data explorer + - offset [int]: 0 by default (first page) """ headers = { "Accept": "application/json", @@ -158,7 +177,8 @@ def get_engage_pages_by_param(self, category_id, key, value): data={ "params": ( f'{{"category_id": "{category_id}", ' - f'"keyword": "{key}", "value": "{value}"}}' + f'"keyword": "{key}", "value": "{value}", ' + f'"limit": "{limit}", "offset": "{offset}"}}', ) }, ) diff --git a/setup.py b/setup.py index 0abf559..56193f9 100755 --- a/setup.py +++ b/setup.py @@ -4,7 +4,7 @@ setup( name="canonicalwebteam.discourse", - version="5.6.1", + version="5.7.1", author="Canonical webteam", author_email="webteam@canonical.com", url="https://github.com/canonical/canonicalwebteam.discourse", diff --git a/tests/cassettes/TestDiscourseAPI.test_index_ep_takeovers.yaml b/tests/cassettes/TestDiscourseAPI.test_index_ep_takeovers.yaml new file mode 100644 index 0000000..febd61e --- /dev/null +++ b/tests/cassettes/TestDiscourseAPI.test_index_ep_takeovers.yaml @@ -0,0 +1,55 @@ +interactions: +- request: + body: params=%7B%22category_id%22%3A+%2250%22%2C+%22limit%22%3A+%2250%22%2C+%22offset%22%3A+%220%22%7D + headers: + Accept: + - application/json + Accept-Encoding: + - gzip, deflate + Connection: + - keep-alive + Content-Length: + - '96' + Content-Type: + - multipart/form-data; + User-Agent: + - python-requests/2.32.3 + method: POST + uri: https://discourse.ubuntu.com/admin/plugins/explorer/queries/14/run + response: + body: + string: "{\"success\": true, \"errors\": [], \"duration\": 12.5, \"result_count\": 1, \"params\": {\"category_id\": 51, \"limit\": \"1\", \"offset\": \"0\"}, \"rows\": [[\"test metadata table\"]]}" + headers: + Connection: + - keep-alive + Content-Type: + - text/plain; charset=utf-8 + Date: + - Wed, 10 Jul 2024 12:35:59 GMT + Referrer-Policy: + - strict-origin-when-cross-origin + Set-Cookie: + - DISCOURSE-K8S_AFFINITY=1720614960.234.50.132029|758aaddbd3331dc0c8b13bfc67634cb7; + Max-Age=3600; Path=/; Secure; HttpOnly; SameSite=Lax + Strict-Transport-Security: + - max-age=15724800; includeSubDomains + Transfer-Encoding: + - chunked + X-Content-Type-Options: + - nosniff + X-Download-Options: + - noopen + X-Frame-Options: + - SAMEORIGIN + X-Permitted-Cross-Domain-Policies: + - none + X-Request-Id: + - e6a993dbe7af79190d387d6d8aac84a1 + X-Runtime: + - '0.005133' + X-XSS-Protection: + - '0' + status: + code: 200 + message: OK +version: 1 diff --git a/tests/cassettes/TestDiscourseAPI.test_individual_ep_takeovers.yaml b/tests/cassettes/TestDiscourseAPI.test_individual_ep_takeovers.yaml new file mode 100644 index 0000000..3457d78 --- /dev/null +++ b/tests/cassettes/TestDiscourseAPI.test_individual_ep_takeovers.yaml @@ -0,0 +1,56 @@ +interactions: +- request: + body: params=%7B%22category_id%22%3A+%2251%22%2C+%22keyword%22%3A+%22active%22%2C+%22value%22%3A+%22true%22%2C+%22limit%22%3A+%2250%22%2C+%22offset%22%3A+%220%22%7D + headers: + Accept: + - application/json + Accept-Encoding: + - gzip, deflate + Connection: + - keep-alive + Content-Length: + - '158' + Content-Type: + - multipart/form-data; + User-Agent: + - python-requests/2.32.3 + method: POST + uri: https://discourse.ubuntu.com/admin/plugins/explorer/queries/16/run + response: + body: + string: "{\"success\": true, \"errors\": [], \"duration\": 12.5, \"result_count\": 1, \"params\": {\"category_id\": 51, \"limit\": \"1\", \"offset\": \"0\"}, \"rows\": [[\"test metadata table\"]]}" + + headers: + Connection: + - keep-alive + Content-Type: + - text/plain; charset=utf-8 + Date: + - Wed, 10 Jul 2024 12:53:22 GMT + Referrer-Policy: + - strict-origin-when-cross-origin + Set-Cookie: + - DISCOURSE-K8S_AFFINITY=1720616003.069.63.255833|758aaddbd3331dc0c8b13bfc67634cb7; + Max-Age=3600; Path=/; Secure; HttpOnly; SameSite=Lax + Strict-Transport-Security: + - max-age=15724800; includeSubDomains + Transfer-Encoding: + - chunked + X-Content-Type-Options: + - nosniff + X-Download-Options: + - noopen + X-Frame-Options: + - SAMEORIGIN + X-Permitted-Cross-Domain-Policies: + - none + X-Request-Id: + - 9b2bb8a749b5a5e836f38ff8f6fd5be5 + X-Runtime: + - '0.003923' + X-XSS-Protection: + - '0' + status: + code: 200 + message: OK +version: 1 diff --git a/tests/cassettes/TestDiscourseAPI.test_pagination.yaml b/tests/cassettes/TestDiscourseAPI.test_pagination.yaml new file mode 100644 index 0000000..117f729 --- /dev/null +++ b/tests/cassettes/TestDiscourseAPI.test_pagination.yaml @@ -0,0 +1,55 @@ +interactions: +- request: + body: params=%7B%22category_id%22%3A+%2251%22%2C+%22limit%22%3A+%221%22%2C+%22offset%22%3A+%220%22%7D + headers: + Accept: + - application/json + Accept-Encoding: + - gzip, deflate + Connection: + - keep-alive + Content-Length: + - '95' + Content-Type: + - multipart/form-data; + User-Agent: + - python-requests/2.32.3 + method: POST + uri: https://discourse.ubuntu.com/admin/plugins/explorer/queries/14/run + response: + body: + string: "{\"success\": true, \"errors\": [], \"duration\": 12.5, \"result_count\": 1, \"params\": {\"category_id\": 51, \"limit\": \"1\", \"offset\": \"0\"}, \"rows\": [[\"test metadata table\"]]}" + headers: + Connection: + - keep-alive + Content-Type: + - text/plain; charset=utf-8 + Date: + - Tue, 02 Jul 2024 10:32:34 GMT + Referrer-Policy: + - strict-origin-when-cross-origin + Set-Cookie: + - DISCOURSE-K8S_AFFINITY=1719916355.654.10737.274478|758aaddbd3331dc0c8b13bfc67634cb7; + Max-Age=3600; Path=/; Secure; HttpOnly; SameSite=Lax + Strict-Transport-Security: + - max-age=15724800; includeSubDomains + Transfer-Encoding: + - chunked + X-Content-Type-Options: + - nosniff + X-Download-Options: + - noopen + X-Frame-Options: + - SAMEORIGIN + X-Permitted-Cross-Domain-Policies: + - none + X-Request-Id: + - 380c23277fb8e1bafc0dd84b5ae4a875 + X-Runtime: + - '0.004855' + X-XSS-Protection: + - '0' + status: + code: 200 + message: OK +version: 1 diff --git a/tests/test_engage_pages.py b/tests/test_engage_pages.py index cd6216d..50aa325 100644 --- a/tests/test_engage_pages.py +++ b/tests/test_engage_pages.py @@ -5,6 +5,8 @@ from vcr_unittest import VCRTestCase from canonicalwebteam.discourse import DiscourseAPI, EngagePages +from canonicalwebteam.discourse.exceptions import MaxLimitError + this_dir = os.path.dirname(os.path.realpath(__file__)) @@ -47,3 +49,48 @@ def test_get_topic(self): response = self.discourse_api.get_topic(17275) self.assertEqual(response["id"], 17275) + + def test_index_ep_takeovers(self): + """ + Test endpoint that retrieves all takeovers/engage pages + """ + + response = self.discourse_api.engage_pages_by_category() + self.assertEqual(len(response), 1) + + def test_individual_ep_takeovers(self): + """ + Test endpoint that retrieves individual takeovers/engage pages + """ + + response = self.discourse_api.get_engage_pages_by_param( + 51, "active", "true" + ) + + self.assertEqual(len(response), 1) + + def test_pagination(self): + """ + Test limit and offset params + + Args: + - category_id=51, should always be 51 for + https://discourse.ubuntu.com/c/design/engage-pages/51 + """ + response = self.discourse_api.engage_pages_by_category( + limit=1, offset=0 + ) + + self.assertEqual(len(response), 1) + + def test_max_limit_error(self): + """ + Test pagination limit abuse + """ + + self.assertRaises( + MaxLimitError, + self.discourse_api.engage_pages_by_category, + limit=1000, + offset=0, + )