From 6cca7a90e07293d60d1462fd980ab70d3db2c0e6 Mon Sep 17 00:00:00 2001 From: Abhay Date: Mon, 20 Jan 2025 09:24:48 +0530 Subject: [PATCH] Added pagination logic in the slack bot (#489) * Added pagination logic in the slack bot * pre-commit * chhnages after conflit and new chnages * fixed bug: do not append a blank button * resolved issues * Update code --------- Co-authored-by: Arkadii Yakovets Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> --- backend/apps/slack/actions/home.py | 55 +++++++++++++++---- backend/apps/slack/blocks.py | 29 ++++++++++ backend/apps/slack/commands/chapters.py | 1 + backend/apps/slack/commands/committees.py | 2 +- backend/apps/slack/commands/projects.py | 1 + .../apps/slack/common/handlers/chapters.py | 26 +++++++-- .../apps/slack/common/handlers/committees.py | 25 +++++++-- .../apps/slack/common/handlers/projects.py | 26 +++++++-- backend/apps/slack/common/presentation.py | 1 + backend/apps/slack/constants.py | 8 +++ backend/tests/slack/actions/home_tests.py | 6 +- .../tests/slack/commands/chapters_tests.py | 5 +- .../tests/slack/commands/committees_tests.py | 5 +- .../tests/slack/commands/projects_tests.py | 5 +- 14 files changed, 162 insertions(+), 33 deletions(-) diff --git a/backend/apps/slack/actions/home.py b/backend/apps/slack/actions/home.py index 531b1d79e..2816d0f06 100644 --- a/backend/apps/slack/actions/home.py +++ b/backend/apps/slack/actions/home.py @@ -10,8 +10,14 @@ from apps.slack.common.presentation import EntityPresentation from apps.slack.constants import ( VIEW_CHAPTERS_ACTION, + VIEW_CHAPTERS_ACTION_NEXT, + VIEW_CHAPTERS_ACTION_PREV, VIEW_COMMITTEES_ACTION, + VIEW_COMMITTEES_ACTION_NEXT, + VIEW_COMMITTEES_ACTION_PREV, VIEW_PROJECTS_ACTION, + VIEW_PROJECTS_ACTION_NEXT, + VIEW_PROJECTS_ACTION_PREV, ) logger = logging.getLogger(__name__) @@ -23,27 +29,44 @@ def handle_home_actions(ack, body, client): action_id = body["actions"][0]["action_id"] user_id = body["user"]["id"] + payload = body.get("actions", [])[0] + value = payload.get("value", "1") try: home_presentation = EntityPresentation( include_feedback=False, include_metadata=True, + include_pagination=True, include_timestamps=False, name_truncation=80, summary_truncation=200, ) + page = int(value) if value.isdigit() else 1 + blocks = [] match action_id: - case "view_chapters_action": - blocks = chapters.get_blocks(limit=10, presentation=home_presentation) - - case "view_committees_action": - blocks = committees.get_blocks(limit=10, presentation=home_presentation) - - case "view_projects_action": - blocks = projects.get_blocks(limit=10, presentation=home_presentation) + case action if action in { + VIEW_CHAPTERS_ACTION, + VIEW_CHAPTERS_ACTION_PREV, + VIEW_CHAPTERS_ACTION_NEXT, + }: + blocks = chapters.get_blocks(page=page, limit=10, presentation=home_presentation) + + case action if action in { + VIEW_COMMITTEES_ACTION, + VIEW_COMMITTEES_ACTION_PREV, + VIEW_COMMITTEES_ACTION_NEXT, + }: + blocks = committees.get_blocks(page=page, limit=10, presentation=home_presentation) + + case action if action in { + VIEW_PROJECTS_ACTION, + VIEW_PROJECTS_ACTION_PREV, + VIEW_PROJECTS_ACTION_NEXT, + }: + blocks = projects.get_blocks(page=page, limit=10, presentation=home_presentation) case _: blocks = [markdown("Invalid action, please try again.")] @@ -63,6 +86,16 @@ def handle_home_actions(ack, body, client): # Register the actions if SlackConfig.app: - SlackConfig.app.action(VIEW_CHAPTERS_ACTION)(handle_home_actions) - SlackConfig.app.action(VIEW_COMMITTEES_ACTION)(handle_home_actions) - SlackConfig.app.action(VIEW_PROJECTS_ACTION)(handle_home_actions) + actions = ( + VIEW_CHAPTERS_ACTION_NEXT, + VIEW_CHAPTERS_ACTION_PREV, + VIEW_CHAPTERS_ACTION, + VIEW_COMMITTEES_ACTION_NEXT, + VIEW_COMMITTEES_ACTION_PREV, + VIEW_COMMITTEES_ACTION, + VIEW_PROJECTS_ACTION_NEXT, + VIEW_PROJECTS_ACTION_PREV, + VIEW_PROJECTS_ACTION, + ) + for action in actions: + SlackConfig.app.action(action)(handle_home_actions) diff --git a/backend/apps/slack/blocks.py b/backend/apps/slack/blocks.py index 56481fea9..feb00bc2a 100644 --- a/backend/apps/slack/blocks.py +++ b/backend/apps/slack/blocks.py @@ -53,3 +53,32 @@ def get_header(): ], }, ] + + +def get_pagination_buttons(entity_type, page, total_pages): + """Get pagination buttons for the blocks.""" + pagination_buttons = [] + + if page > 1: + pagination_buttons.append( + { + "type": "button", + "text": {"type": "plain_text", "text": "Previous"}, + "action_id": f"view_{entity_type}_action_prev", + "value": str(page - 1), + "style": "primary", + } + ) + + if total_pages > page: + pagination_buttons.append( + { + "type": "button", + "text": {"type": "plain_text", "text": "Next"}, + "action_id": f"view_{entity_type}_action_next", + "value": str(page + 1), + "style": "primary", + } + ) + + return pagination_buttons diff --git a/backend/apps/slack/commands/chapters.py b/backend/apps/slack/commands/chapters.py index b54aec438..6c0a80f57 100644 --- a/backend/apps/slack/commands/chapters.py +++ b/backend/apps/slack/commands/chapters.py @@ -37,6 +37,7 @@ def chapters_handler(ack, command, client): presentation=EntityPresentation( include_feedback=True, include_metadata=True, + include_pagination=False, include_timestamps=True, name_truncation=80, summary_truncation=300, diff --git a/backend/apps/slack/commands/committees.py b/backend/apps/slack/commands/committees.py index 59148bc7f..1d38eb64a 100644 --- a/backend/apps/slack/commands/committees.py +++ b/backend/apps/slack/commands/committees.py @@ -22,12 +22,12 @@ def committees_handler(ack, command, client): presentation=EntityPresentation( include_feedback=True, include_metadata=True, + include_pagination=False, include_timestamps=True, name_truncation=80, summary_truncation=300, ), ) - conversation = client.conversations_open(users=command["user_id"]) client.chat_postMessage(channel=conversation["channel"]["id"], blocks=blocks) diff --git a/backend/apps/slack/commands/projects.py b/backend/apps/slack/commands/projects.py index 455220bc0..3f0db3981 100644 --- a/backend/apps/slack/commands/projects.py +++ b/backend/apps/slack/commands/projects.py @@ -22,6 +22,7 @@ def projects_handler(ack, command, client): presentation=EntityPresentation( include_feedback=True, include_metadata=True, + include_pagination=False, include_timestamps=True, name_truncation=80, summary_truncation=300, diff --git a/backend/apps/slack/common/handlers/chapters.py b/backend/apps/slack/common/handlers/chapters.py index 27b9fe97d..e96a0323e 100644 --- a/backend/apps/slack/common/handlers/chapters.py +++ b/backend/apps/slack/common/handlers/chapters.py @@ -7,7 +7,7 @@ from apps.common.constants import NL from apps.common.utils import get_absolute_url -from apps.slack.blocks import markdown +from apps.slack.blocks import get_pagination_buttons, markdown from apps.slack.common.constants import TRUNCATION_INDICATOR from apps.slack.common.presentation import EntityPresentation from apps.slack.constants import FEEDBACK_CHANNEL_MESSAGE @@ -15,7 +15,7 @@ def get_blocks( - search_query: str = "", limit: int = 10, presentation: EntityPresentation | None = None + page=1, search_query: str = "", limit: int = 10, presentation: EntityPresentation | None = None ): """Get chapters blocks.""" from apps.owasp.api.search.chapter import get_chapters @@ -35,7 +35,11 @@ def get_blocks( "idx_url", ] - chapters = get_chapters(search_query, attributes=attributes, limit=limit)["hits"] + offset = (page - 1) * limit + chapters_data = get_chapters(search_query, attributes=attributes, limit=limit, page=page) + chapters = chapters_data["hits"] + total_pages = chapters_data["nbPages"] + if not chapters: return [ markdown( @@ -71,7 +75,7 @@ def get_blocks( blocks.append( markdown( - f"{idx + 1}. <{chapter['idx_url']}|*{name}*>{NL}" + f"{offset + idx + 1}. <{chapter['idx_url']}|*{name}*>{NL}" f"_{location}_{NL}" f"{leaders_text}" f"{escape(summary)}{NL}" @@ -88,4 +92,18 @@ def get_blocks( ) ) + if presentation.include_pagination and ( + pagination_block := get_pagination_buttons( + "chapters", + page, + total_pages - 1, + ) + ): + blocks.append( + { + "type": "actions", + "elements": pagination_block, + } + ) + return blocks diff --git a/backend/apps/slack/common/handlers/committees.py b/backend/apps/slack/common/handlers/committees.py index e721398a2..b715dc4cb 100644 --- a/backend/apps/slack/common/handlers/committees.py +++ b/backend/apps/slack/common/handlers/committees.py @@ -7,7 +7,7 @@ from apps.common.constants import NL from apps.common.utils import get_absolute_url -from apps.slack.blocks import markdown +from apps.slack.blocks import get_pagination_buttons, markdown from apps.slack.common.constants import TRUNCATION_INDICATOR from apps.slack.common.presentation import EntityPresentation from apps.slack.constants import FEEDBACK_CHANNEL_MESSAGE @@ -15,7 +15,7 @@ def get_blocks( - search_query: str = "", limit: int = 10, presentation: EntityPresentation | None = None + page=1, search_query: str = "", limit: int = 10, presentation: EntityPresentation | None = None ): """Get committees blocks.""" from apps.owasp.api.search.committee import get_committees @@ -31,7 +31,11 @@ def get_blocks( "idx_url", ] - committees = get_committees(search_query, attributes=attributes, limit=limit)["hits"] + offset = (page - 1) * limit + committees_data = get_committees(search_query, attributes=attributes, limit=limit, page=page) + committees = committees_data["hits"] + total_pages = committees_data["nbPages"] + if not committees: return [ markdown( @@ -66,7 +70,7 @@ def get_blocks( blocks.append( markdown( - f"{idx + 1}. <{committee['idx_url']}|*{name}*>{NL}" + f"{offset + idx + 1}. <{committee['idx_url']}|*{name}*>{NL}" f"{leaders_text}" f"{escape(summary)}{NL}" ) @@ -81,5 +85,18 @@ def get_blocks( f"{FEEDBACK_CHANNEL_MESSAGE}" ) ) + if presentation.include_pagination and ( + pagination_block := get_pagination_buttons( + "committees", + page, + total_pages - 1, + ) + ): + blocks.append( + { + "type": "actions", + "elements": pagination_block, + } + ) return blocks diff --git a/backend/apps/slack/common/handlers/projects.py b/backend/apps/slack/common/handlers/projects.py index 69c9d15de..efc6270b2 100644 --- a/backend/apps/slack/common/handlers/projects.py +++ b/backend/apps/slack/common/handlers/projects.py @@ -7,7 +7,7 @@ from apps.common.constants import NL from apps.common.utils import get_absolute_url, natural_date -from apps.slack.blocks import markdown +from apps.slack.blocks import get_pagination_buttons, markdown from apps.slack.common.constants import TRUNCATION_INDICATOR from apps.slack.common.presentation import EntityPresentation from apps.slack.constants import FEEDBACK_CHANNEL_MESSAGE @@ -15,7 +15,7 @@ def get_blocks( - search_query: str = "", limit: int = 10, presentation: EntityPresentation | None = None + page=1, search_query: str = "", limit: int = 10, presentation: EntityPresentation | None = None ): """Get projects blocks.""" from apps.owasp.api.search.project import get_projects @@ -36,7 +36,11 @@ def get_blocks( "idx_url", ] - projects = get_projects(search_query, attributes=attributes, limit=limit)["hits"] + offset = (page - 1) * limit + projects_data = get_projects(search_query, attributes=attributes, limit=limit, page=page) + projects = projects_data["hits"] + total_pages = projects_data["nbPages"] + if not projects: return [ markdown( @@ -88,7 +92,7 @@ def get_blocks( blocks.append( markdown( - f"{idx + 1}. <{project['idx_url']}|*{name}*>{NL}" + f"{offset + idx + 1}. <{project['idx_url']}|*{name}*>{NL}" f"{updated_text}" f"{metadata_text}" f"{leader_text}" @@ -106,4 +110,18 @@ def get_blocks( ) ) + if presentation.include_pagination and ( + pagination_block := get_pagination_buttons( + "projects", + page, + total_pages - 1, + ) + ): + blocks.append( + { + "type": "actions", + "elements": pagination_block, + } + ) + return blocks diff --git a/backend/apps/slack/common/presentation.py b/backend/apps/slack/common/presentation.py index 6423dfd9f..7a5b4b56d 100644 --- a/backend/apps/slack/common/presentation.py +++ b/backend/apps/slack/common/presentation.py @@ -9,6 +9,7 @@ class EntityPresentation: include_feedback: bool = True include_metadata: bool = True + include_pagination: bool = True include_timestamps: bool = True name_truncation: int = 80 summary_truncation: int = 300 diff --git a/backend/apps/slack/constants.py b/backend/apps/slack/constants.py index 3f6dcd1d6..b4981b010 100644 --- a/backend/apps/slack/constants.py +++ b/backend/apps/slack/constants.py @@ -22,8 +22,16 @@ OWASP_THREAT_MODELING_CHANNEL_ID = "#C1CS3C6AF" VIEW_PROJECTS_ACTION = "view_projects_action" +VIEW_PROJECTS_ACTION_NEXT = "view_projects_action_next" +VIEW_PROJECTS_ACTION_PREV = "view_projects_action_prev" + VIEW_COMMITTEES_ACTION = "view_committees_action" +VIEW_COMMITTEES_ACTION_NEXT = "view_committees_action_next" +VIEW_COMMITTEES_ACTION_PREV = "view_committees_action_prev" + VIEW_CHAPTERS_ACTION = "view_chapters_action" +VIEW_CHAPTERS_ACTION_NEXT = "view_chapters_action_next" +VIEW_CHAPTERS_ACTION_PREV = "view_chapters_action_prev" FEEDBACK_CHANNEL_MESSAGE = ( diff --git a/backend/tests/slack/actions/home_tests.py b/backend/tests/slack/actions/home_tests.py index cce0ff36d..1aceff470 100644 --- a/backend/tests/slack/actions/home_tests.py +++ b/backend/tests/slack/actions/home_tests.py @@ -31,9 +31,9 @@ def _mock_external_calls(self): patch("apps.owasp.api.search.chapter.get_chapters") as mock_chapters, patch("apps.owasp.api.search.committee.get_committees") as mock_committees, ): - mock_projects.return_value = {"hits": []} - mock_chapters.return_value = {"hits": []} - mock_committees.return_value = {"hits": []} + mock_projects.return_value = {"hits": [], "nbPages": 1} + mock_chapters.return_value = {"hits": [], "nbPages": 1} + mock_committees.return_value = {"hits": [], "nbPages": 1} yield @pytest.mark.parametrize( diff --git a/backend/tests/slack/commands/chapters_tests.py b/backend/tests/slack/commands/chapters_tests.py index 98996bec7..727b6ec3f 100644 --- a/backend/tests/slack/commands/chapters_tests.py +++ b/backend/tests/slack/commands/chapters_tests.py @@ -37,7 +37,7 @@ def mock_client(self): @pytest.fixture(autouse=True) def mock_get_chapters(self): with patch("apps.owasp.api.search.chapter.get_chapters") as mock: - mock.return_value = {"hits": []} + mock.return_value = {"hits": [], "nbPages": 1} yield mock @pytest.mark.parametrize( @@ -73,7 +73,8 @@ def test_chapters_handler_with_results(self, mock_get_chapters, mock_client, moc "idx_region": "Test Region", "idx_updated_at": "2024-01-01", } - ] + ], + "nbPages": 1, } chapters_handler(ack=MagicMock(), command=mock_command, client=mock_client) diff --git a/backend/tests/slack/commands/committees_tests.py b/backend/tests/slack/commands/committees_tests.py index 2ffbab12f..85a331f88 100644 --- a/backend/tests/slack/commands/committees_tests.py +++ b/backend/tests/slack/commands/committees_tests.py @@ -37,7 +37,7 @@ def mock_client(self): @pytest.fixture(autouse=True) def mock_get_committees(self): with patch("apps.owasp.api.search.committee.get_committees") as mock: - mock.return_value = {"hits": []} + mock.return_value = {"hits": [], "nbPages": 1} yield mock @pytest.mark.parametrize( @@ -68,7 +68,8 @@ def test_committees_handler_with_results(self, mock_get_committees, mock_client, "idx_url": "http://example.com", "idx_leaders": ["Leader 1"], } - ] + ], + "nbPages": 1, } committees_handler(ack=MagicMock(), command=mock_command, client=mock_client) diff --git a/backend/tests/slack/commands/projects_tests.py b/backend/tests/slack/commands/projects_tests.py index d73760219..d50dc7af9 100644 --- a/backend/tests/slack/commands/projects_tests.py +++ b/backend/tests/slack/commands/projects_tests.py @@ -38,7 +38,7 @@ def mock_client(self): @pytest.fixture(autouse=True) def mock_get_projects(self): with patch("apps.owasp.api.search.project.get_projects") as mock: - mock.return_value = {"hits": []} + mock.return_value = {"hits": [], "nbPages": 1} yield mock @pytest.mark.parametrize( @@ -75,7 +75,8 @@ def test_projects_handler_with_results(self, mock_get_projects, mock_client, moc "idx_stars_count": 10, "idx_updated_at": test_date.timestamp(), } - ] + ], + "nbPages": 1, } projects_handler(ack=MagicMock(), command=mock_command, client=mock_client)