From 4a536d0e005fa511a3273c3c0cf50fdca494e3a0 Mon Sep 17 00:00:00 2001 From: Kerem Dokuz Date: Wed, 6 Sep 2023 10:49:39 +0200 Subject: [PATCH 1/9] ensure external id is unique globally --- sched_slack_bot/model/schedule.py | 2 +- sched_slack_bot/views/schedule_dialog.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/sched_slack_bot/model/schedule.py b/sched_slack_bot/model/schedule.py index d79fe7c..75e8707 100644 --- a/sched_slack_bot/model/schedule.py +++ b/sched_slack_bot/model/schedule.py @@ -148,7 +148,7 @@ def from_modal_submission(cls, submission_body: SlackBody) -> Schedule: schedule_id = submission_body["view"]["external_id"] - if schedule_id == CREATE_NEW_SCHEDULE_VIEW_ID: + if schedule_id.startswith(CREATE_NEW_SCHEDULE_VIEW_ID): schedule_id = str(uuid.uuid4()) return Schedule( diff --git a/sched_slack_bot/views/schedule_dialog.py b/sched_slack_bot/views/schedule_dialog.py index c7b367a..07023fd 100644 --- a/sched_slack_bot/views/schedule_dialog.py +++ b/sched_slack_bot/views/schedule_dialog.py @@ -2,6 +2,7 @@ from enum import StrEnum from typing import Optional, Dict +import uuid from slack_sdk.models.blocks import ( HeaderBlock, @@ -84,7 +85,9 @@ def get_edit_schedule_block( schedule: Optional[Schedule] = None, callback: ScheduleDialogCallback = ScheduleDialogCallback.CREATE_DIALOG ) -> View: modal_type = CREATE_MODAL_TYPE if schedule is None else EDIT_MODAL_TYPE + # make sure external id is unique globally: https://github.com/slackapi/node-slack-sdk/issues/1012#issuecomment-684818059 external_id = schedule.id if schedule is not None else CREATE_NEW_SCHEDULE_VIEW_ID + external_id = f"{external_id}-{str(uuid.uuid4())}" return View( type="modal", external_id=external_id, From 8e44f5129a135bee816e5d0d97a79bd0e29958e3 Mon Sep 17 00:00:00 2001 From: Kerem Dokuz Date: Wed, 6 Sep 2023 10:49:55 +0200 Subject: [PATCH 2/9] ensure time between rotations is positive --- sched_slack_bot/utils/fix_schedule_from_the_past.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sched_slack_bot/utils/fix_schedule_from_the_past.py b/sched_slack_bot/utils/fix_schedule_from_the_past.py index 63417b8..45484ee 100644 --- a/sched_slack_bot/utils/fix_schedule_from_the_past.py +++ b/sched_slack_bot/utils/fix_schedule_from_the_past.py @@ -12,7 +12,7 @@ def fix_schedule_from_the_past(schedule: Schedule) -> Schedule: logger.info(f"Starting to fix schedule with previous date {schedule.next_rotation}") while now >= next_rotation: - next_rotation += schedule.time_between_rotations + next_rotation += abs(schedule.time_between_rotations) logger.info( f"Had to fix schedule with previous date {schedule.next_rotation}, which" From 3d69982146365805be8025e58b0ef2eca2405158 Mon Sep 17 00:00:00 2001 From: Kerem Dokuz Date: Wed, 6 Sep 2023 11:03:26 +0200 Subject: [PATCH 3/9] use delimiter to distinguish view id and objectid --- sched_slack_bot/model/schedule.py | 4 ++++ sched_slack_bot/views/schedule_dialog.py | 3 ++- sched_slack_bot/views/schedule_dialog_block_ids.py | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/sched_slack_bot/model/schedule.py b/sched_slack_bot/model/schedule.py index 75e8707..13a1626 100644 --- a/sched_slack_bot/model/schedule.py +++ b/sched_slack_bot/model/schedule.py @@ -10,6 +10,7 @@ from sched_slack_bot.utils.slack_typing_stubs import SlackState, SlackBody from sched_slack_bot.views.schedule_dialog_block_ids import ( DISPLAY_NAME_BLOCK_ID, + SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER, USERS_INPUT_BLOCK_ID, CHANNEL_INPUT_BLOCK_ID, DatetimeSelectorType, @@ -151,6 +152,9 @@ def from_modal_submission(cls, submission_body: SlackBody) -> Schedule: if schedule_id.startswith(CREATE_NEW_SCHEDULE_VIEW_ID): schedule_id = str(uuid.uuid4()) + if SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER in schedule_id: + schedule_id = schedule_id.split(SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER)[0] + return Schedule( id=schedule_id, display_name=display_name, diff --git a/sched_slack_bot/views/schedule_dialog.py b/sched_slack_bot/views/schedule_dialog.py index 07023fd..81df1c5 100644 --- a/sched_slack_bot/views/schedule_dialog.py +++ b/sched_slack_bot/views/schedule_dialog.py @@ -20,6 +20,7 @@ from sched_slack_bot.views.schedule_dialog_block_ids import ( DISPLAY_NAME_BLOCK_ID, CHANNEL_INPUT_BLOCK_ID, + SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER, USERS_INPUT_BLOCK_ID, FIRST_ROTATION_LABEL, SECOND_ROTATION_LABEL, @@ -87,7 +88,7 @@ def get_edit_schedule_block( modal_type = CREATE_MODAL_TYPE if schedule is None else EDIT_MODAL_TYPE # make sure external id is unique globally: https://github.com/slackapi/node-slack-sdk/issues/1012#issuecomment-684818059 external_id = schedule.id if schedule is not None else CREATE_NEW_SCHEDULE_VIEW_ID - external_id = f"{external_id}-{str(uuid.uuid4())}" + external_id = f"{external_id}{SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER}{str(uuid.uuid4())}" return View( type="modal", external_id=external_id, diff --git a/sched_slack_bot/views/schedule_dialog_block_ids.py b/sched_slack_bot/views/schedule_dialog_block_ids.py index 5831e39..cc41e8d 100644 --- a/sched_slack_bot/views/schedule_dialog_block_ids.py +++ b/sched_slack_bot/views/schedule_dialog_block_ids.py @@ -11,6 +11,8 @@ CREATE_NEW_SCHEDULE_VIEW_ID = "NEW_DIALOG" +SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER = "::" + class DatetimeSelectorType(Enum): DATE = "DATE" From 68f353a58e9f7ffe1019b34fd539c2ae89ece58c Mon Sep 17 00:00:00 2001 From: Kerem Dokuz Date: Wed, 6 Sep 2023 11:41:09 +0200 Subject: [PATCH 4/9] fix tests --- test_sched_slack_bot/test_controller.py | 30 ++++++++++--------- .../views/test_schedule_dialog.py | 4 +-- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/test_sched_slack_bot/test_controller.py b/test_sched_slack_bot/test_controller.py index b98f40a..d32251b 100644 --- a/test_sched_slack_bot/test_controller.py +++ b/test_sched_slack_bot/test_controller.py @@ -151,12 +151,14 @@ def test_handle_clicked_create_opens_create_schedule_dialog( schedule: Schedule, ) -> None: mocked_schedule_access.get_available_schedules.return_value = [schedule] + with mock.patch(target="sched_slack_bot.views.schedule_dialog.uuid", return_value="uuid"): + ack = mock.MagicMock() + controller_with_mocks.handle_clicked_create_schedule(ack=ack, body=slack_body) - ack = mock.MagicMock() - controller_with_mocks.handle_clicked_create_schedule(ack=ack, body=slack_body) - - ack.assert_called_once() - mocked_slack_client.views_open.assert_called_once_with(trigger_id=slack_body["trigger_id"], view=get_edit_schedule_block()) + ack.assert_called_once() + mocked_slack_client.views_open.assert_called_once_with( + trigger_id=slack_body["trigger_id"], view=get_edit_schedule_block() + ) @pytest.mark.parametrize( @@ -194,15 +196,15 @@ def test_handle_clicked_edit_schedule_opens_edit_dialog( ) -> None: mocked_schedule_access.get_schedule.return_value = schedule slack_body["actions"] = [SlackAction(action_id=EDIT_SCHEDULE_ACTION_ID, block_id=schedule.id)] - - ack = mock.MagicMock() - controller_with_mocks.handle_clicked_edit_schedule(ack=ack, body=slack_body) - - ack.assert_called_once() - mocked_slack_client.views_open.assert_called_once_with( - trigger_id=slack_body["trigger_id"], - view=get_edit_schedule_block(schedule=schedule, callback=ScheduleDialogCallback.EDIT_DIALOG), - ) + with mock.patch(target="sched_slack_bot.views.schedule_dialog.uuid", return_value="uuid"): + ack = mock.MagicMock() + controller_with_mocks.handle_clicked_edit_schedule(ack=ack, body=slack_body) + + ack.assert_called_once() + mocked_slack_client.views_open.assert_called_once_with( + trigger_id=slack_body["trigger_id"], + view=get_edit_schedule_block(schedule=schedule, callback=ScheduleDialogCallback.EDIT_DIALOG), + ) def test_handle_submitted_create_schedule_creates_new_schedule( diff --git a/test_sched_slack_bot/views/test_schedule_dialog.py b/test_sched_slack_bot/views/test_schedule_dialog.py index 1a8b3b7..8c6f4c8 100644 --- a/test_sched_slack_bot/views/test_schedule_dialog.py +++ b/test_sched_slack_bot/views/test_schedule_dialog.py @@ -47,7 +47,7 @@ def test_get_create_schedule_dialog() -> None: assert create_block.blocks[0].text is not None assert CREATE_MODAL_TYPE in create_block.blocks[0].text.text - assert create_block.external_id == CREATE_NEW_SCHEDULE_VIEW_ID + assert create_block.external_id.startswith(CREATE_NEW_SCHEDULE_VIEW_ID) def test_get_edit_schedule_dialog(schedule: Schedule) -> None: @@ -59,7 +59,7 @@ def test_get_edit_schedule_dialog(schedule: Schedule) -> None: assert create_block.blocks[0].text is not None assert EDIT_MODAL_TYPE in create_block.blocks[0].text.text - assert create_block.external_id == schedule.id + assert create_block.external_id.startswith(schedule.id) def test_get_display_name_input_creation() -> None: From 3de1a5f1899bcd101b7d5f81dffbca0a6ef6ee16 Mon Sep 17 00:00:00 2001 From: Kerem Dokuz Date: Wed, 6 Sep 2023 11:43:21 +0200 Subject: [PATCH 5/9] save positive rotation time in db --- sched_slack_bot/model/schedule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sched_slack_bot/model/schedule.py b/sched_slack_bot/model/schedule.py index 13a1626..e3750ea 100644 --- a/sched_slack_bot/model/schedule.py +++ b/sched_slack_bot/model/schedule.py @@ -145,7 +145,7 @@ def from_modal_submission(cls, submission_body: SlackBody) -> Schedule: state=state, date_input_block_ids=get_datetime_block_ids(label=SECOND_ROTATION_LABEL) ) - time_between_rotations = second_rotation - next_rotation + time_between_rotations = abs(second_rotation - next_rotation) schedule_id = submission_body["view"]["external_id"] From 2c17833bb90aa4b634bf78ad0a16150537e6072a Mon Sep 17 00:00:00 2001 From: Kerem Dokuz Date: Wed, 6 Sep 2023 11:52:32 +0200 Subject: [PATCH 6/9] remove unnecessary abs assertion --- sched_slack_bot/utils/fix_schedule_from_the_past.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sched_slack_bot/utils/fix_schedule_from_the_past.py b/sched_slack_bot/utils/fix_schedule_from_the_past.py index 45484ee..63417b8 100644 --- a/sched_slack_bot/utils/fix_schedule_from_the_past.py +++ b/sched_slack_bot/utils/fix_schedule_from_the_past.py @@ -12,7 +12,7 @@ def fix_schedule_from_the_past(schedule: Schedule) -> Schedule: logger.info(f"Starting to fix schedule with previous date {schedule.next_rotation}") while now >= next_rotation: - next_rotation += abs(schedule.time_between_rotations) + next_rotation += schedule.time_between_rotations logger.info( f"Had to fix schedule with previous date {schedule.next_rotation}, which" From 4cfb90dbe25f11b15ccb3aa214133240e982427e Mon Sep 17 00:00:00 2001 From: Kerem Dokuz Date: Wed, 6 Sep 2023 12:00:24 +0200 Subject: [PATCH 7/9] fix mypy --- test_sched_slack_bot/views/test_schedule_dialog.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test_sched_slack_bot/views/test_schedule_dialog.py b/test_sched_slack_bot/views/test_schedule_dialog.py index 8c6f4c8..c03943a 100644 --- a/test_sched_slack_bot/views/test_schedule_dialog.py +++ b/test_sched_slack_bot/views/test_schedule_dialog.py @@ -47,7 +47,8 @@ def test_get_create_schedule_dialog() -> None: assert create_block.blocks[0].text is not None assert CREATE_MODAL_TYPE in create_block.blocks[0].text.text - assert create_block.external_id.startswith(CREATE_NEW_SCHEDULE_VIEW_ID) + if create_block.external_id is not None: + assert create_block.external_id.startswith(CREATE_NEW_SCHEDULE_VIEW_ID) def test_get_edit_schedule_dialog(schedule: Schedule) -> None: @@ -59,7 +60,8 @@ def test_get_edit_schedule_dialog(schedule: Schedule) -> None: assert create_block.blocks[0].text is not None assert EDIT_MODAL_TYPE in create_block.blocks[0].text.text - assert create_block.external_id.startswith(schedule.id) + if create_block.external_id is not None: + assert create_block.external_id.startswith(schedule.id) def test_get_display_name_input_creation() -> None: From 1e65b58eb4e960d328de15cce08ccbd272c4dfc0 Mon Sep 17 00:00:00 2001 From: Kerem Dokuz Date: Wed, 6 Sep 2023 13:15:15 +0200 Subject: [PATCH 8/9] extend readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 6f972b3..4ce8dc3 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,7 @@ Your One-Stop-Shop for setting up rotating :calendar: schedules. * Copy & paste app_manifest.yml ![Image of creating a slack app](https://github.com/Germandrummer92/SchedSlackBot/raw/main/assets/slack_bot_creation.png "Creating a Slack bot") * Adjust the request url to something valid (either ngrok or already final k8s deployment url) (you can change it later) +* Make sure the url is verified in the App Manifest section. (it should have the prefix /slack/events) * Install to your workspace ![Image of installing the slack bot](https://github.com/Germandrummer92/SchedSlackBot/raw/main/assets/install.png "Installing the Slack bot") * Copy the Slack_Bot_Token and Slack_Signing_Secret for local development or deployment From fc536c93819f6dcaf226576b560228c2f2124589 Mon Sep 17 00:00:00 2001 From: Kerem Dokuz Date: Wed, 6 Sep 2023 13:33:22 +0200 Subject: [PATCH 9/9] refactoring --- sched_slack_bot/model/schedule.py | 10 +++++----- sched_slack_bot/views/schedule_dialog.py | 8 ++++---- sched_slack_bot/views/schedule_dialog_block_ids.py | 4 ++-- test_sched_slack_bot/views/test_schedule_dialog.py | 10 +++++----- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/sched_slack_bot/model/schedule.py b/sched_slack_bot/model/schedule.py index e3750ea..727eaaf 100644 --- a/sched_slack_bot/model/schedule.py +++ b/sched_slack_bot/model/schedule.py @@ -10,14 +10,14 @@ from sched_slack_bot.utils.slack_typing_stubs import SlackState, SlackBody from sched_slack_bot.views.schedule_dialog_block_ids import ( DISPLAY_NAME_BLOCK_ID, - SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER, + SCHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER, USERS_INPUT_BLOCK_ID, CHANNEL_INPUT_BLOCK_ID, DatetimeSelectorType, get_datetime_block_ids, FIRST_ROTATION_LABEL, SECOND_ROTATION_LABEL, - CREATE_NEW_SCHEDULE_VIEW_ID, + CREATE_NEW_SCHEDULE_VIEW_ID_PREFIX, ) logger = logging.getLogger(__name__) @@ -149,11 +149,11 @@ def from_modal_submission(cls, submission_body: SlackBody) -> Schedule: schedule_id = submission_body["view"]["external_id"] - if schedule_id.startswith(CREATE_NEW_SCHEDULE_VIEW_ID): + if schedule_id.startswith(CREATE_NEW_SCHEDULE_VIEW_ID_PREFIX): schedule_id = str(uuid.uuid4()) - if SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER in schedule_id: - schedule_id = schedule_id.split(SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER)[0] + if SCHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER in schedule_id: + schedule_id = schedule_id.split(SCHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER)[0] return Schedule( id=schedule_id, diff --git a/sched_slack_bot/views/schedule_dialog.py b/sched_slack_bot/views/schedule_dialog.py index 81df1c5..f32f591 100644 --- a/sched_slack_bot/views/schedule_dialog.py +++ b/sched_slack_bot/views/schedule_dialog.py @@ -20,12 +20,12 @@ from sched_slack_bot.views.schedule_dialog_block_ids import ( DISPLAY_NAME_BLOCK_ID, CHANNEL_INPUT_BLOCK_ID, - SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER, + SCHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER, USERS_INPUT_BLOCK_ID, FIRST_ROTATION_LABEL, SECOND_ROTATION_LABEL, DatetimeSelectorType, - CREATE_NEW_SCHEDULE_VIEW_ID, + CREATE_NEW_SCHEDULE_VIEW_ID_PREFIX, ) CREATION_CHANNEL_PLACEHOLDER = "#channel" @@ -87,8 +87,8 @@ def get_edit_schedule_block( ) -> View: modal_type = CREATE_MODAL_TYPE if schedule is None else EDIT_MODAL_TYPE # make sure external id is unique globally: https://github.com/slackapi/node-slack-sdk/issues/1012#issuecomment-684818059 - external_id = schedule.id if schedule is not None else CREATE_NEW_SCHEDULE_VIEW_ID - external_id = f"{external_id}{SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER}{str(uuid.uuid4())}" + external_id = schedule.id if schedule is not None else CREATE_NEW_SCHEDULE_VIEW_ID_PREFIX + external_id = f"{external_id}{SCHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER}{str(uuid.uuid4())}" return View( type="modal", external_id=external_id, diff --git a/sched_slack_bot/views/schedule_dialog_block_ids.py b/sched_slack_bot/views/schedule_dialog_block_ids.py index cc41e8d..3633515 100644 --- a/sched_slack_bot/views/schedule_dialog_block_ids.py +++ b/sched_slack_bot/views/schedule_dialog_block_ids.py @@ -9,9 +9,9 @@ FIRST_ROTATION_LABEL = "First Rotation Reminder/Rotation" SECOND_ROTATION_LABEL = "Second Rotation Reminder/Rotation" -CREATE_NEW_SCHEDULE_VIEW_ID = "NEW_DIALOG" +CREATE_NEW_SCHEDULE_VIEW_ID_PREFIX = "NEW_DIALOG" -SHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER = "::" +SCHEDULE_VIEW_ID_SCHEDULE_ID_DELIMITER = "::" class DatetimeSelectorType(Enum): diff --git a/test_sched_slack_bot/views/test_schedule_dialog.py b/test_sched_slack_bot/views/test_schedule_dialog.py index c03943a..3360913 100644 --- a/test_sched_slack_bot/views/test_schedule_dialog.py +++ b/test_sched_slack_bot/views/test_schedule_dialog.py @@ -19,7 +19,7 @@ get_second_rotation_block, ) from sched_slack_bot.views.schedule_dialog_block_ids import ( - CREATE_NEW_SCHEDULE_VIEW_ID, + CREATE_NEW_SCHEDULE_VIEW_ID_PREFIX, FIRST_ROTATION_LABEL, SECOND_ROTATION_LABEL, ) @@ -47,8 +47,8 @@ def test_get_create_schedule_dialog() -> None: assert create_block.blocks[0].text is not None assert CREATE_MODAL_TYPE in create_block.blocks[0].text.text - if create_block.external_id is not None: - assert create_block.external_id.startswith(CREATE_NEW_SCHEDULE_VIEW_ID) + assert create_block.external_id is not None + assert create_block.external_id.startswith(CREATE_NEW_SCHEDULE_VIEW_ID_PREFIX) def test_get_edit_schedule_dialog(schedule: Schedule) -> None: @@ -60,8 +60,8 @@ def test_get_edit_schedule_dialog(schedule: Schedule) -> None: assert create_block.blocks[0].text is not None assert EDIT_MODAL_TYPE in create_block.blocks[0].text.text - if create_block.external_id is not None: - assert create_block.external_id.startswith(schedule.id) + assert create_block.external_id is not None + assert create_block.external_id.startswith(schedule.id) def test_get_display_name_input_creation() -> None: