From d09973a1a27cbf7a37e0f7df07c8d838a5bf25c7 Mon Sep 17 00:00:00 2001 From: Jamyn Shanley <696574+Jamyn@users.noreply.github.com> Date: Sun, 16 Aug 2026 00:15:47 +0000 Subject: [PATCH] fix: load slack project options on demand past the 100-option limit --- .../settings/plugins/configuring-slack.mdx | 6 + .../dispatch_slack/case/interactive.py | 11 +- src/dispatch/plugins/dispatch_slack/config.py | 8 + .../plugins/dispatch_slack/endpoints.py | 14 +- src/dispatch/plugins/dispatch_slack/fields.py | 109 ++++- .../dispatch_slack/incident/interactive.py | 20 +- .../plugins/dispatch_slack/options.py | 82 ++++ src/dispatch/project/service.py | 53 +++ .../dispatch_slack/case/test_case_modals.py | 16 +- tests/plugins/dispatch_slack/conftest.py | 184 ++++++++- .../dispatch_slack/test_project_modals.py | 375 ++++++++++++++++++ .../dispatch_slack/test_project_options.py | 202 ++++++++++ .../dispatch_slack/test_project_select.py | 202 ++++++++++ .../plugins/dispatch_slack/test_slack_live.py | 21 + tests/project/test_project_service.py | 28 ++ 15 files changed, 1306 insertions(+), 25 deletions(-) create mode 100644 src/dispatch/plugins/dispatch_slack/options.py create mode 100644 tests/plugins/dispatch_slack/test_project_modals.py create mode 100644 tests/plugins/dispatch_slack/test_project_options.py create mode 100644 tests/plugins/dispatch_slack/test_project_select.py diff --git a/docs/docs/administration/settings/plugins/configuring-slack.mdx b/docs/docs/administration/settings/plugins/configuring-slack.mdx index 76fec93a1..48fae9a3d 100644 --- a/docs/docs/administration/settings/plugins/configuring-slack.mdx +++ b/docs/docs/administration/settings/plugins/configuring-slack.mdx @@ -61,6 +61,12 @@ Then, under the `Subscribe to bot events` section, add the following bot user ev To enable Dispatch to generate interactive components such as dialogs and modals, ensure that the `Request URL` points to the events action API endpoint of the Dispatch server at `/api/v1//events/slack/action`. Replace `` with your organization's name. The default one is `default`. +### Select Menus + +On the same `Interactivity & Shortcuts` page, under `Select Menus`, set the `Options Load URL` to `/api/v1//events/slack/menu`. Replace `` with your organization's name. The default one is `default`. + +This is a separate field from the `Request URL` above and does not inherit from it. Dispatch uses it for the type-ahead menus that load their choices on demand rather than embedding them in the modal: tag selection, and project selection in deployments with more than 100 enabled projects. If it is unset, those menus open empty and stay empty, and because the field is required the form cannot be submitted. + ### Slash Commands To enable Dispatch's slash commands, you must add them to your Slack app first. Ensure that the `Command` field in the Slack API Web UI matches the name of the command name in the Slack plugin in the Dispatch Web UI and that the `Request URL` points to the events command API endpoint of the Dispatch server at `/api/v1//events/slack/command`. diff --git a/src/dispatch/plugins/dispatch_slack/case/interactive.py b/src/dispatch/plugins/dispatch_slack/case/interactive.py index 29b9c3cd0..72f9fa34c 100644 --- a/src/dispatch/plugins/dispatch_slack/case/interactive.py +++ b/src/dispatch/plugins/dispatch_slack/case/interactive.py @@ -83,6 +83,7 @@ extension_request_checkbox, incident_priority_select, incident_type_select, + project_option, project_select, relative_date_picker_input, resolution_input, @@ -194,7 +195,7 @@ def handle_escalate_case_command( default_title = case.title default_description = case.description - default_project = {"text": case.project.display_name, "value": case.project.id} + default_project = project_option(case.project) blocks = [ Context(elements=[Text(type="mrkdwn", text="Accept the defaults or adjust as needed.")]), @@ -1472,7 +1473,7 @@ def escalate_button_click( description_input(initial_value=case.description), project_select( db_session=db_session, - initial_option={"text": case.project.display_name, "value": case.project.id}, + initial_option=project_option(case.project), action_id=CaseEscalateActions.project_select, dispatch_action=True, ), @@ -1529,7 +1530,7 @@ def handle_project_select_action( description_input(), project_select( db_session=db_session, - initial_option={"text": project.display_name, "value": project.id}, + initial_option=project_option(project), action_id=CaseEscalateActions.project_select, dispatch_action=True, ), @@ -2407,7 +2408,7 @@ def handle_report_project_select_action( description_input(), project_select( db_session=db_session, - initial_option={"text": project.display_name, "value": project.id}, + initial_option=project_option(project), action_id=CaseReportActions.project_select, dispatch_action=True, ), @@ -2517,7 +2518,7 @@ def handle_report_case_type_select_action( description_input(), project_select( db_session=db_session, - initial_option={"text": project.display_name, "value": project.id}, + initial_option=project_option(project), action_id=CaseReportActions.project_select, dispatch_action=True, ), diff --git a/src/dispatch/plugins/dispatch_slack/config.py b/src/dispatch/plugins/dispatch_slack/config.py index ce42fbc1b..0b28e8aeb 100644 --- a/src/dispatch/plugins/dispatch_slack/config.py +++ b/src/dispatch/plugins/dispatch_slack/config.py @@ -4,6 +4,14 @@ MAX_SECTION_TEXT_LENGTH = 2999 +# Slack caps a static select at 100 options and an options-load response at 100 +# options. Both limits are the same number and both are hard: over either one +# the payload is rejected, so nothing may be built or returned past this. +MAX_SELECT_OPTIONS = 100 + +# Longest plain_text an option label may carry. +MAX_OPTION_TEXT_LENGTH = 75 + class SlackConfiguration(BaseConfigurationModel): """Slack configuration description.""" diff --git a/src/dispatch/plugins/dispatch_slack/endpoints.py b/src/dispatch/plugins/dispatch_slack/endpoints.py index 9ccdf425c..b6f6d25af 100644 --- a/src/dispatch/plugins/dispatch_slack/endpoints.py +++ b/src/dispatch/plugins/dispatch_slack/endpoints.py @@ -19,6 +19,10 @@ from .workflow import configure as workflow_configure from .messaging import get_incident_conversation_command_message +# Registers the options-load listener with `app` on import; nothing here calls +# into it, so without this the /slack/menu route has no project handler. +from . import options # noqa: F401 + router = APIRouter() @@ -139,7 +143,13 @@ async def slack_actions(request: Request, organization: str, body: bytes = Depen @router.post( "/slack/menu", ) -async def slack_menus(request: Request, organization: str, body: bytes = Depends(get_body)): - """Handle all incoming Slack menus.""" +def slack_menus(request: Request, organization: str, body: bytes = Depends(get_body)): + """Handle all incoming Slack menus. + + Deliberately not `async`. `handler.handle` is synchronous all the way down + and Bolt polls for the listener's ack in a sleep loop, so awaiting it here + would block the event loop -- for every keystroke, now that the project + select loads its options through this route. + """ handler = get_request_handler(request=request, body=body, organization=organization) return handler.handle(req=request, body=body) diff --git a/src/dispatch/plugins/dispatch_slack/fields.py b/src/dispatch/plugins/dispatch_slack/fields.py index c9bde7bda..900cf69c6 100644 --- a/src/dispatch/plugins/dispatch_slack/fields.py +++ b/src/dispatch/plugins/dispatch_slack/fields.py @@ -5,6 +5,7 @@ from blockkit import ( Checkboxes, DatePicker, + ExternalSelect, Input, MultiExternalSelect, MultiStaticSelect, @@ -24,8 +25,13 @@ from dispatch.incident.severity import service as incident_severity_service from dispatch.incident.type import service as incident_type_service from dispatch.participant.models import Participant -from dispatch.plugins.dispatch_slack.config import MAX_SECTION_TEXT_LENGTH +from dispatch.plugins.dispatch_slack.config import ( + MAX_OPTION_TEXT_LENGTH, + MAX_SECTION_TEXT_LENGTH, + MAX_SELECT_OPTIONS, +) from dispatch.project import service as project_service +from dispatch.project.models import Project from dispatch.signal.models import Signal log = logging.getLogger(__name__) @@ -321,6 +327,76 @@ def multi_select_block( ) +def external_select_block( + placeholder: str, + action_id: str = None, + block_id: str = None, + initial_option: dict[str, str] = None, + label: str = None, + # Slack's own default is 3 characters. One keeps the menu responsive on a + # short project name while still bounding how much a keystroke costs. It is + # also the floor the blockkit library imposes -- Slack itself accepts 0, + # but blockkit refuses to build it, and 0 would buy nothing here: Slack + # loads the menu on open regardless, and this only gates typing. + min_query_length: int = 1, + **kwargs, +): + """Builds an external select block, whose options are loaded on demand. + + Slack asks for the options over the block_suggestion route rather than + reading them out of the view, so nothing here is bounded by the 100-option + limit a static select carries. + """ + processed_initial_option = None + if initial_option: + processed_initial_option = { + k: str(v) if k == "value" else v for k, v in initial_option.items() + } + + return Input( + element=ExternalSelect( + placeholder=placeholder, + initial_option=( + Option(**processed_initial_option) if processed_initial_option else None + ), + min_query_length=min_query_length, + action_id=action_id, + ), + block_id=block_id, + label=label, + **kwargs, + ) + + +def project_label(project: Project) -> str: + """The option text for a project, within Slack's length limit. + + Never empty, because Slack rejects an option with empty text and this is + used for the preselected option too, where no query has filtered anything + out. ``display_name`` is only defaulted to '' at the column level, so a + project created without one falls back to its name -- the same pairing the + rest of Dispatch lists a project under, and what the migration that added + the column seeded it from. Falling back past that to the id keeps a + project whose name is null selectable rather than dropping it from the + menu; it cannot be found by typing, since there is nothing to match on. + """ + label = project.display_name or project.name or f"Project {project.id}" + return label[:MAX_OPTION_TEXT_LENGTH] + + +def project_option(project: Project) -> dict[str, str]: + """Builds the select option for a project. + + The text is what a project is called everywhere else it is listed; the + value is its id, which is what every handler reads back. They are not + interchangeable -- two projects may share a display name. + """ + return { + "text": project_label(project), + "value": str(project.id), + } + + def project_select( db_session: Session, action_id: str = DefaultActionIds.project_select, @@ -329,19 +405,36 @@ def project_select( initial_option: dict = None, **kwargs, ): - """Creates a project select.""" - projects = [ - {"text": p.display_name, "value": p.id} - for p in project_service.get_all(db_session=db_session) - if p.enabled - ] + """Creates a project select. + + Embedding the options only works while there are few enough of them to + embed, so past Slack's limit this hands over to an external select and the + options handler in ``.options``. Both produce the same block and action + ids and the same ``selected_option`` shape, so callers cannot tell which + one they got. + """ + # One row past the limit is all it takes to know which of the two applies, + # and is far cheaper than counting a large project table. + projects = project_service.get_all_enabled(db_session=db_session, limit=MAX_SELECT_OPTIONS + 1) if not projects: log.warning("Unable to create a select block for projects. No projects found.") return + if len(projects) > MAX_SELECT_OPTIONS: + return external_select_block( + # Slack loads the menu on open, so this is still a browse; past a + # hundred projects it is also worth saying that typing narrows it. + placeholder="Select or search for a project", + initial_option=initial_option, + action_id=action_id, + block_id=block_id, + label=label, + **kwargs, + ) + return static_select_block( placeholder="Select Project", - options=projects, + options=[project_option(p) for p in projects], initial_option=initial_option, action_id=action_id, block_id=block_id, diff --git a/src/dispatch/plugins/dispatch_slack/incident/interactive.py b/src/dispatch/plugins/dispatch_slack/incident/interactive.py index e3a067d7a..2485d2b87 100644 --- a/src/dispatch/plugins/dispatch_slack/incident/interactive.py +++ b/src/dispatch/plugins/dispatch_slack/incident/interactive.py @@ -60,7 +60,7 @@ from dispatch.plugins.dispatch_slack.bolt import app from dispatch.plugins.dispatch_slack.decorators import message_dispatcher from dispatch.plugins.dispatch_slack.enums import SlackAPIErrorCode -from dispatch.plugins.dispatch_slack.exceptions import CommandError, EventError +from dispatch.plugins.dispatch_slack.exceptions import CommandError, EventError, SubmissionError from dispatch.plugins.dispatch_slack.fields import ( DefaultActionIds, DefaultBlockIds, @@ -72,6 +72,7 @@ incident_status_select, incident_type_select, participant_select, + project_option, project_select, resolution_input, static_select_block, @@ -335,7 +336,7 @@ def handle_update_incident_project_select_action( incident_status_select(initial_option={"text": incident.status, "value": incident.status}), project_select( db_session=db_session, - initial_option={"text": project.display_name, "value": project.id}, + initial_option=project_option(project), action_id=IncidentUpdateActions.project_select, dispatch_action=True, ), @@ -2739,7 +2740,19 @@ def handle_report_incident_submission_event( tag = tag_service.get(db_session=db_session, tag_id=int(t["value"])) tags.append(tag) - project = {"name": form_data[DefaultBlockIds.project_select]["name"]} + # Resolved from the option's value, not its text: the text is the project's + # label, which is its display name where it has one and is truncated to + # Slack's 75 characters, while the lookup downstream matches on `name` and + # silently falls back to the default project when it misses. + selected_project = project_service.get( + db_session=db_session, + project_id=int(form_data[DefaultBlockIds.project_select]["value"]), + ) + if not selected_project: + raise SubmissionError( + msg="The selected project no longer exists. Please reopen this form and try again." + ) + project = {"name": selected_project.name} incident_type = None if form_data.get(DefaultBlockIds.incident_type_select): @@ -2849,6 +2862,7 @@ def handle_report_incident_project_select_action( description_input(), project_select( db_session=db_session, + initial_option=project_option(project), action_id=IncidentReportActions.project_select, dispatch_action=True, ), diff --git a/src/dispatch/plugins/dispatch_slack/options.py b/src/dispatch/plugins/dispatch_slack/options.py new file mode 100644 index 000000000..7722a6d12 --- /dev/null +++ b/src/dispatch/plugins/dispatch_slack/options.py @@ -0,0 +1,82 @@ +"""Options-load handler for the project select. + +Slack asks for an external select's options over the block_suggestion route +rather than reading them out of the view, which is what lets the project select +carry more projects than a static select's 100-option limit allows (#86). +Requests arrive at the ``/slack/menu`` endpoint, which verifies the signature +and picks the organization before anything here runs. + +Slack will not send them anywhere unless the app's *Options Load URL* is set, +which is a separate field from the Request URL -- see +docs/administration/settings/plugins/configuring-slack. + +The tag lookup is the other options handler and still lives in +incident/interactive.py, though both flows use it. Consolidating them here is +worth doing, but not as part of this fix. +""" + +import logging +import re + +from slack_bolt import Ack, BoltContext +from sqlalchemy.orm import Session + +from dispatch.project import service as project_service + +from .bolt import app +from .config import MAX_SELECT_OPTIONS +from .fields import project_option +from .middleware import action_context_middleware, db_middleware + +log = logging.getLogger(__name__) + +# Every project select's action id ends in "project-select" -- the shared +# default plus one per flow. Matching on the suffix rather than listing them +# means a new call site with its own action id is served too; without a +# matching handler its select would open to a permanently empty menu. Bolt +# matches with `search`, so the leading boundary has to be explicit or an +# unrelated "subproject-select" would be served this list as well. +PROJECT_SELECT_ACTION_ID_PATTERN = re.compile(r"(^|-)project-select$") + + +@app.options( + PROJECT_SELECT_ACTION_ID_PATTERN, middleware=[action_context_middleware, db_middleware] +) +def handle_project_search_action( + ack: Ack, payload: dict, context: BoltContext, db_session: Session +) -> None: + """Serves the project type-ahead. + + ``db_middleware`` binds the session to the organization the modal was + opened in, so this offers exactly what the static select would have -- every + enabled project in that organization's schema, which is the only place the + query can reach. + """ + try: + projects = project_service.get_all_enabled( + db_session=db_session, + query_str=payload.get("value"), + limit=MAX_SELECT_OPTIONS, + ) + except Exception: + # Slack renders whatever comes back; an error here would leave the user + # staring at a spinner, and the detail belongs in the log, not in a menu. + # The slug is what tells an operator which tenant is broken. + log.exception( + "Unable to load project options for the Slack project select. Organization: %s", + context["subject"].organization_slug, + ) + return ack(options=[]) + + options = [] + for project in projects: + option = project_option(project) + options.append( + { + "text": {"type": "plain_text", "text": option["text"]}, + # NOTE: slack doesn't accept int's as values (fails silently) + "value": option["value"], + } + ) + + ack(options=options) diff --git a/src/dispatch/project/service.py b/src/dispatch/project/service.py index 962325423..b757dd2e9 100644 --- a/src/dispatch/project/service.py +++ b/src/dispatch/project/service.py @@ -1,10 +1,21 @@ from pydantic import ValidationError +from sqlalchemy import func, or_ from sqlalchemy.orm import Session from sqlalchemy.sql.expression import true from .models import Project, ProjectCreate, ProjectRead, ProjectUpdate +# Sorts a project under the name it is listed by: its display name, falling +# back to its name. The 2024-12-12 migration seeded display_name from name and +# left the column defaulting to '', so a project created since without one +# would otherwise sort under nothing. +PROJECT_LABEL = func.coalesce(func.nullif(Project.display_name, ""), Project.name) + +# Escaped so a display name containing % or _ matches literally rather than +# turning into a wildcard. +_LIKE_ESCAPE = "\\" + def get(*, db_session: Session, project_id: int) -> Project | None: """Returns a project based on the given project id.""" @@ -70,6 +81,48 @@ def get_all(*, db_session) -> list[Project | None]: return db_session.query(Project) +def get_all_enabled( + *, db_session: Session, query_str: str | None = None, limit: int | None = None +) -> list[Project]: + """Returns enabled projects, ordered by label and narrowed by a substring match. + + Filtering, ordering and limiting all happen in the database so a caller + that only wants a page of options does not pay for the whole table. + Ordering is case-insensitive by label with the id as a tiebreaker, so the + same call always returns the same rows in the same order. + + Deliberately a substring match rather than the full-text `search_vector` + the tag lookup uses: that vector covers `name` and `description` but not + `display_name`, which is the string a selector actually shows and a user + actually types. Full-text search also matches whole word prefixes, so + "ments" would never find "Payments". + + Both `display_name` and `name` are matched even though only the label is + displayed, because operators know projects under both. + """ + query = db_session.query(Project).filter(Project.enabled == true()) + + if query_str: + pattern = "%{}%".format( + query_str.replace(_LIKE_ESCAPE, _LIKE_ESCAPE * 2) + .replace("%", f"{_LIKE_ESCAPE}%") + .replace("_", f"{_LIKE_ESCAPE}_") + ) + query = query.filter( + or_( + Project.display_name.ilike(pattern, escape=_LIKE_ESCAPE), + Project.name.ilike(pattern, escape=_LIKE_ESCAPE), + ) + ) + + query = query.order_by(func.lower(PROJECT_LABEL), Project.id) + + if limit is not None: + query = query.limit(limit) + + return query.all() + + def create(*, db_session, project_in: ProjectCreate) -> Project: """Creates a project.""" from dispatch.organization import service as organization_service diff --git a/tests/plugins/dispatch_slack/case/test_case_modals.py b/tests/plugins/dispatch_slack/case/test_case_modals.py index 7a4a2934b..cb12a256c 100644 --- a/tests/plugins/dispatch_slack/case/test_case_modals.py +++ b/tests/plugins/dispatch_slack/case/test_case_modals.py @@ -10,20 +10,24 @@ from dispatch.plugins.dispatch_slack.case.interactive import ( handle_engage_user_command, + handle_escalate_case_command, handle_update_case_command, + report_issue, resolve_button_click, ) -# Three handlers are deliberately absent. -# handle_list_signals_command puts the Bolt-context plugin config into a query, -# so a stub config cannot drive it. handle_escalate_case_command and -# report_issue call project_select, which emits one option per project with no -# cap; the suite accumulates far more than Slack's 100-option limit, so they -# only build in isolation. +# handle_list_signals_command is deliberately absent: it puts the Bolt-context +# plugin config into a query, so a stub config cannot drive it. MODAL_HANDLERS = [ handle_update_case_command, handle_engage_user_command, resolve_button_click, + # These two build a project select, which the suite's accumulated projects + # used to push past Slack's 100-option limit -- so they only built in + # isolation, and only when they ran early enough (#86). Their behaviour + # either side of that limit is covered in ../test_project_modals.py. + handle_escalate_case_command, + report_issue, ] diff --git a/tests/plugins/dispatch_slack/conftest.py b/tests/plugins/dispatch_slack/conftest.py index dc348c07a..4269fe709 100644 --- a/tests/plugins/dispatch_slack/conftest.py +++ b/tests/plugins/dispatch_slack/conftest.py @@ -8,12 +8,19 @@ """ import inspect -from unittest.mock import MagicMock +from uuid import uuid4 +from unittest.mock import MagicMock, patch import pytest from slack_bolt import BoltContext +from slack_bolt.request import BoltRequest +from slack_sdk.web import SlackResponse, WebClient +from sqlalchemy import insert +import dispatch.database.core as database_core +from dispatch.organization.models import Organization from dispatch.plugins.dispatch_slack.models import SubjectMetadata +from dispatch.project.models import Project from tests.factories import ( CaseFactory, CasePriorityFactory, @@ -23,6 +30,7 @@ IncidentPriorityFactory, IncidentSeverityFactory, IncidentTypeFactory, + OrganizationFactory, ParticipantFactory, ProjectFactory, ServiceFactory, @@ -37,6 +45,178 @@ def call_handler(handler, **available): return handler(**{a: available[a] for a in wanted}) +@pytest.fixture +def dispatch_interaction(): + """Run a Slack interaction payload through the real Bolt app. + + Bolt authorizes every request with a live ``auth.test`` call before any + listener runs, and the app's token is a placeholder -- so that one call has + to be answered locally or nothing reaches a listener. Everything past it, + matcher and middleware included, is the real thing. + """ + from dispatch.plugins.dispatch_slack.bolt import app + + auth_test = SlackResponse( + client=None, + http_verb="POST", + api_url="https://slack.com/api/auth.test", + req_args={}, + data={ + "ok": True, + "url": "https://example.slack.com/", + "team": "Example", + "user": "dispatch", + "team_id": "T123", + "user_id": "UBOT", + "bot_id": "BBOT", + }, + headers={}, + status_code=200, + ) + + # Bolt's listener middleware unwind before the listener body rather than + # wrapping it, so db_middleware's `with get_organization_session(...)` has + # already closed its session by the time a listener uses it. The listener's + # first query silently opens a fresh transaction that nothing then closes. + # A live process gets those backends back when the request is collected; + # this suite holds its requests, so they would sit open and the session + # teardown could not drop the database. + opened: list = [] + real_refetch = database_core.refetch_db_session + + def track(organization_slug: str): + session = real_refetch(organization_slug) + opened.append(session) + return session + + # Action listeners are normally run on a worker thread, so dispatch would + # return before the handler had done anything to assert on. The listener + # itself is unaffected by which thread runs it. + runner = app._listener_runner + was_synchronous = runner.process_before_response + runner.process_before_response = True + + try: + with ( + patch.object(database_core, "refetch_db_session", track), + patch.object(WebClient, "auth_test", return_value=auth_test), + ): + yield lambda body: app.dispatch(BoltRequest(body=body, mode="socket_mode")) + finally: + runner.process_before_response = was_synchronous + for session in opened: + session.close() + + +@pytest.fixture +def single_default_organization(session): + """Leave exactly one organization marked as the default. + + OrganizationFactory randomises the flag, so a suite that has built a few + of them has several defaults -- and the middleware that resolves a request + with no subject yet asks for *the* default organization. + """ + was = {org.id: org.default for org in session.query(Organization).all()} + + for org in session.query(Organization).all(): + org.default = org.slug == "default" + session.commit() + + yield + + for org in session.query(Organization).all(): + org.default = was.get(org.id, org.default) + session.commit() + + +@pytest.fixture +def only_projects(session): + """Make the set of enabled projects exactly what a test asks for. + + The factories commit, so projects leak from one test into the next -- which + is why #86 surfaced as an order-dependent failure rather than a reliable + one. Anything asserting on project select options needs the enabled set to + be its own, so each call parks whatever is already enabled and returns the + rows it created. Rows go in one statement: the boundary cases run to a + thousand projects and a thousand factory commits is not worth the wall + clock. + + Teardown restores every row it touched, `display_name` included -- a test + that blanks one is otherwise committed by the factories and outlives the + rollback. + """ + original: dict[int, tuple[bool, str]] = {} + created: list[int] = [] + + def remember(project: Project) -> None: + if project.id not in created and project.id not in original: + original[project.id] = (project.enabled, project.display_name) + + def make( + count: int = 0, + display_names: list[str] | None = None, + keep: list[Project] = (), + ) -> list[Project]: + """Leave `count` projects enabled and return the ones this created. + + Projects in `keep` stay enabled and count toward `count`, for tests + whose fixtures own a project they still need to be able to select. + """ + if display_names is not None: + assert not keep, "keep= and display_names= together would drop the last name" + count = len(display_names) + count -= len(keep) + + keep_ids = {p.id for p in keep} + for project in session.query(Project).filter(Project.enabled.is_(True)).all(): + if project.id in keep_ids: + continue + remember(project) + project.enabled = False + + for project in keep: + remember(project) + project.enabled = True + + if count <= 0: + session.commit() + return [] + + organization = session.query(Organization).order_by(Organization.id).first() + if organization is None: + organization = OrganizationFactory() + + prefix = uuid4().hex[:8] + rows = [ + { + "name": f"{prefix}-{i:04d}", + "display_name": ( + display_names[i] if display_names is not None else f"{prefix}-{i:04d}" + ), + "organization_id": organization.id, + "enabled": True, + "default": False, + } + for i in range(count) + ] + ids = session.scalars(insert(Project).returning(Project.id), rows).all() + session.commit() + created.extend(ids) + + return session.query(Project).filter(Project.id.in_(ids)).order_by(Project.id).all() + + yield make + + if created: + session.query(Project).filter(Project.id.in_(created)).delete(synchronize_session=False) + for project_id, (enabled, display_name) in original.items(): + project = session.get(Project, project_id) + if project: + project.enabled = enabled + project.display_name = display_name + session.commit() + + @pytest.fixture def incident_with_related_records(session): """An incident whose type, severity and priority belong to its own project. @@ -115,6 +295,8 @@ def _drive(handler, subject_type, subject, session): client=client, context=context, db_session=session, + # Shortcut handlers read the trigger_id off the shortcut, not the body. + shortcut={"trigger_id": "T123"}, ) assert client.views_open.called, f"{handler.__name__} never opened a modal" diff --git a/tests/plugins/dispatch_slack/test_project_modals.py b/tests/plugins/dispatch_slack/test_project_modals.py new file mode 100644 index 000000000..88539b990 --- /dev/null +++ b/tests/plugins/dispatch_slack/test_project_modals.py @@ -0,0 +1,375 @@ +"""Every modal that offers a project, built at project counts either side of +Slack's 100-option limit. + +#86 was not a bug in the selector alone -- it made four incident and case +workflows impossible to open. These build each one for real, so a regression +shows up as the modal it actually breaks. +""" + +import json +from unittest.mock import MagicMock, patch + +import pytest +from slack_sdk.web import WebClient + +from tests.factories import IncidentTypeFactory + +from dispatch.plugins.dispatch_slack.case.enums import CaseEscalateActions, CaseReportActions +from dispatch.plugins.dispatch_slack.case.interactive import ( + escalate_button_click, + handle_escalate_case_command, + report_issue, +) +from dispatch.plugins.dispatch_slack.config import MAX_SELECT_OPTIONS +from dispatch.plugins.dispatch_slack.fields import DefaultBlockIds +from dispatch.plugins.dispatch_slack.incident.enums import ( + IncidentReportActions, + IncidentUpdateActions, +) +from dispatch.plugins.dispatch_slack.incident.interactive import ( + handle_report_incident_command, + report_incident, +) + +# Either side of Slack's limit, plus a deployment far past it. +PROJECT_COUNTS = [1, 99, MAX_SELECT_OPTIONS, MAX_SELECT_OPTIONS + 1, 500] + +CASE_HANDLERS = [handle_escalate_case_command, escalate_button_click, report_issue] +INCIDENT_HANDLERS = [handle_report_incident_command, report_incident] + + +@pytest.fixture +def escalatable_case(session, case_with_related_records): + """Give the case type an incident type from the case's own project. + + ``escalate_button_click`` preselects it, and a select rejects an + initial_option that is not among its options -- which the default factory + graph produces, because it gives the incident type a project of its own. + """ + case = case_with_related_records + case.case_type.incident_type = IncidentTypeFactory(project=case.project) + session.commit() + return case + + +def expected_mode(count: int) -> str: + """Which select a given project count must produce -- not merely one of them.""" + return "static_select" if count <= MAX_SELECT_OPTIONS else "external_select" + + +def project_block(view: dict) -> dict: + blocks = [b for b in view["blocks"] if b.get("block_id") == DefaultBlockIds.project_select] + assert blocks, "modal has no project select" + return blocks[0] + + +@pytest.mark.parametrize("count", PROJECT_COUNTS) +@pytest.mark.parametrize("handler", CASE_HANDLERS, ids=lambda h: h.__name__) +def test_case_modals_open_at_any_project_count( + session, escalatable_case, only_projects, build_case_modal, handler, count +): + only_projects(count, keep=[escalatable_case.project]) + + view = build_case_modal(handler) + + assert view["type"] == "modal" + assert project_block(view)["element"]["type"] == expected_mode(count) + + +@pytest.mark.parametrize("count", PROJECT_COUNTS) +@pytest.mark.parametrize("handler", INCIDENT_HANDLERS, ids=lambda h: h.__name__) +def test_incident_modals_open_at_any_project_count( + session, incident_with_related_records, only_projects, build_incident_modal, handler, count +): + only_projects(count, keep=[incident_with_related_records.project]) + + view = build_incident_modal(handler) + + assert view["type"] == "modal" + assert project_block(view)["element"]["type"] == expected_mode(count) + + +@pytest.mark.parametrize("handler", CASE_HANDLERS, ids=lambda h: h.__name__) +def test_case_modals_open_when_the_project_has_no_display_name( + session, escalatable_case, only_projects, build_case_modal, handler +): + """An empty display_name is the column default, and Slack rejects empty + option text -- including in the initial_option the escalate modals set.""" + project = escalatable_case.project + project.display_name = "" + only_projects(3, keep=[project]) + + view = build_case_modal(handler) + element = project_block(view)["element"] + + labels = [o["text"]["text"] for o in element["options"]] + assert project.name in labels, "the project fell out of its own selector" + if "initial_option" in element: + assert element["initial_option"]["text"]["text"] == project.name + + +def test_escalate_preselects_the_cases_project_past_the_limit( + session, escalatable_case, only_projects, build_case_modal +): + project = escalatable_case.project + only_projects(MAX_SELECT_OPTIONS + 1, keep=[project]) + + view = build_case_modal(handle_escalate_case_command) + element = project_block(view)["element"] + + assert element["type"] == "external_select" + assert element["initial_option"]["value"] == str(project.id) + + +def test_report_issue_selection_reaches_the_case_report_handler( + session, + case_with_related_records, + only_projects, + dispatch_interaction, + single_default_organization, +): + """The whole point of the selector: a picked project drives the next step. + + Runs the real block_actions payload Slack sends when a project is chosen and + checks the modal that comes back, at a project count that forces the + external select. This action's middleware resolve the database session + before the subject, so it lands on the default organization -- hence the + fixture that leaves exactly one. + """ + project = case_with_related_records.project + case_type = case_with_related_records.case_type + only_projects(MAX_SELECT_OPTIONS + 1, keep=[project]) + + selected = { + "text": {"type": "plain_text", "text": project.display_name}, + "value": str(project.id), + } + body = { + "type": "block_actions", + "team": {"id": "T123"}, + "user": {"id": "U123"}, + "api_app_id": "A123", + "trigger_id": "TRIGGER", + "container": {"type": "view", "view_id": "V123"}, + "view": { + "id": "V123", + "hash": "1.0", + "type": "modal", + "private_metadata": json.dumps( + { + "type": "case", + "id": str(case_with_related_records.id), + "organization_slug": "default", + "project_id": str(project.id), + "channel_id": "C123", + } + ), + "state": { + "values": { + DefaultBlockIds.project_select: { + CaseReportActions.project_select: { + "type": "external_select", + "selected_option": selected, + } + } + } + }, + }, + "actions": [ + { + "type": "external_select", + "action_id": CaseReportActions.project_select, + "block_id": DefaultBlockIds.project_select, + "selected_option": selected, + "action_ts": "1.0", + } + ], + } + + views_update = MagicMock(return_value={"view": {"id": "V123"}, "trigger_id": "TRIGGER"}) + with patch.object(WebClient, "views_update", views_update): + response = dispatch_interaction(body) + + assert response.status == 200, response.body + assert views_update.called, "the project selection re-rendered nothing" + + view = views_update.call_args.kwargs["view"] + assert project_block(view)["element"]["initial_option"]["value"] == str(project.id) + + case_types = [ + b for b in view["blocks"] if b.get("block_id") == DefaultBlockIds.case_type_select + ] + assert case_types, "the selected project's case types were not offered" + assert str(case_type.id) in {o["value"] for o in case_types[0]["element"]["options"]}, ( + "the case types offered do not belong to the selected project" + ) + + +def selection_payload(action_id: str, subject: dict, project) -> dict: + """The block_actions payload Slack posts when a project is chosen.""" + selected = { + "text": {"type": "plain_text", "text": project.display_name or project.name}, + "value": str(project.id), + } + return { + "type": "block_actions", + "team": {"id": "T123"}, + "user": {"id": "U123"}, + "api_app_id": "A123", + "trigger_id": "TRIGGER", + "container": {"type": "view", "view_id": "V123"}, + "view": { + "id": "V123", + "hash": "1.0", + "type": "modal", + "private_metadata": json.dumps(subject), + "state": { + "values": { + DefaultBlockIds.project_select: { + action_id: {"type": "external_select", "selected_option": selected} + } + } + }, + }, + "actions": [ + { + "type": "external_select", + "action_id": action_id, + "block_id": DefaultBlockIds.project_select, + "selected_option": selected, + "action_ts": "1.0", + } + ], + } + + +CASE_SELECT_ACTION_IDS = [CaseEscalateActions.project_select, CaseReportActions.project_select] +INCIDENT_SELECT_ACTION_IDS = [ + IncidentReportActions.project_select, + IncidentUpdateActions.project_select, +] + + +def assert_selection_survives(dispatch_interaction, body, project): + views_update = MagicMock(return_value={"view": {"id": "V123"}, "trigger_id": "TRIGGER"}) + with patch.object(WebClient, "views_update", views_update): + response = dispatch_interaction(body) + + assert response.status == 200, response.body + assert views_update.called, "the project selection re-rendered nothing" + + element = project_block(views_update.call_args.kwargs["view"])["element"] + assert "initial_option" in element, "the re-rendered modal lost the selection" + assert element["initial_option"]["value"] == str(project.id) + + +@pytest.mark.parametrize("action_id", CASE_SELECT_ACTION_IDS, ids=str) +def test_a_chosen_project_survives_the_case_re_render( + session, + escalatable_case, + only_projects, + dispatch_interaction, + single_default_organization, + action_id, +): + """Past the limit the menu is a search, so a lost selection costs a re-search.""" + project = escalatable_case.project + only_projects(MAX_SELECT_OPTIONS + 1, keep=[project]) + + subject = { + "type": "case", + "id": str(escalatable_case.id), + "organization_slug": "default", + "project_id": str(project.id), + "channel_id": "C123", + } + + assert_selection_survives( + dispatch_interaction, selection_payload(action_id, subject, project), project + ) + + +@pytest.mark.parametrize("action_id", INCIDENT_SELECT_ACTION_IDS, ids=str) +def test_a_chosen_project_survives_the_incident_re_render( + session, + incident_with_related_records, + only_projects, + dispatch_interaction, + single_default_organization, + action_id, +): + project = incident_with_related_records.project + only_projects(MAX_SELECT_OPTIONS + 1, keep=[project]) + + subject = { + "type": "incident", + "id": str(incident_with_related_records.id), + "organization_slug": "default", + "project_id": str(project.id), + "channel_id": "C123", + } + + assert_selection_survives( + dispatch_interaction, selection_payload(action_id, subject, project), project + ) + + +def test_report_incident_files_against_the_selected_project( + session, only_projects, dispatch_interaction, single_default_organization +): + """The selection has to survive submission, not just re-rendering. + + `display_name` is separately editable in the web UI, so it routinely differs + from `name`. Resolving the submitted project from the option's text sends + the incident to whatever `get_by_name_or_default` falls back to -- the + default project -- with nothing said to the reporter. + """ + from dispatch.plugins.dispatch_slack.incident.interactive import ( + handle_report_incident_submission_event, + ) + + default_project, target = only_projects( + display_names=["", "A Friendly Display Name"], + ) + # get_by_name_or_default falls back to whichever project is the default. + default_project.default = True + target.default = False + session.commit() + + assert target.display_name != target.name, "the test needs the two to differ" + + form_data = { + DefaultBlockIds.title_input: "a title", + DefaultBlockIds.description_input: "a description", + DefaultBlockIds.project_select: { + "name": target.display_name, + "value": str(target.id), + }, + } + + created = {} + + def capture(*, db_session, incident_in): + created["project"] = incident_in.project + raise RuntimeError("stop after the project is resolved") + + client = MagicMock() + client.views_update.return_value = {"view": {"id": "V123"}, "trigger_id": "TRIGGER"} + + with patch( + "dispatch.plugins.dispatch_slack.incident.interactive.incident_service.create", + side_effect=capture, + ): + with pytest.raises(RuntimeError, match="stop after"): + handle_report_incident_submission_event( + ack=MagicMock(), + body={"view": {"id": "V123"}, "trigger_id": "TRIGGER"}, + client=client, + db_session=session, + form_data=form_data, + user=MagicMock(email="reporter@example.com"), + ) + + assert created["project"].name == target.name, ( + "the incident was filed against the wrong project" + ) diff --git a/tests/plugins/dispatch_slack/test_project_options.py b/tests/plugins/dispatch_slack/test_project_options.py new file mode 100644 index 000000000..7c62c26df --- /dev/null +++ b/tests/plugins/dispatch_slack/test_project_options.py @@ -0,0 +1,202 @@ +"""Coverage for the project type-ahead behind the external project select. + +Past Slack's 100-option limit the select carries no options of its own and asks +for them over the block_suggestion route instead (#86). The bug #86 describes +lives at the Slack boundary, so these drive the real Bolt app with real +block_suggestion payloads rather than calling the listener directly. +""" + +import json +from unittest.mock import patch + +import pytest + +# Deliberately the production wiring, not `.options` directly: endpoints.py is +# the only thing that registers the listener in a running deployment, so +# importing the module under test here would let that import be deleted with +# the suite still green. +import dispatch.plugins.dispatch_slack.endpoints # noqa: F401 +from dispatch.plugins.dispatch_slack.case.enums import CaseEscalateActions, CaseReportActions +from dispatch.plugins.dispatch_slack.config import MAX_SELECT_OPTIONS +from dispatch.plugins.dispatch_slack.fields import DefaultActionIds +from dispatch.plugins.dispatch_slack.incident.enums import ( + IncidentReportActions, + IncidentUpdateActions, +) + +PROJECT_SELECT_ACTION_IDS = [ + DefaultActionIds.project_select, + IncidentUpdateActions.project_select, + IncidentReportActions.project_select, + CaseEscalateActions.project_select, + CaseReportActions.project_select, +] + + +def suggestion_payload(query: str = "", action_id: str = DefaultActionIds.project_select) -> dict: + """A block_suggestion payload of the shape Slack posts to /slack/menu.""" + return { + "type": "block_suggestion", + "team": {"id": "T123", "domain": "example"}, + "user": {"id": "U123", "name": "someone"}, + "api_app_id": "A123", + "token": "verification-token", + "container": {"type": "view", "view_id": "V123"}, + "action_id": action_id, + "block_id": "project-select", + "value": query, + "view": { + "id": "V123", + "type": "modal", + "private_metadata": json.dumps( + {"type": "case", "organization_slug": "default", "channel_id": "C123"} + ), + "state": {"values": {}}, + }, + } + + +@pytest.fixture +def load_options(dispatch_interaction): + """Run a block_suggestion through Bolt and return the options it answered with.""" + + def load(query: str = "", action_id: str = DefaultActionIds.project_select) -> list[dict]: + response = dispatch_interaction(suggestion_payload(query, action_id)) + + assert response.status == 200, response.body + return json.loads(response.body)["options"] + + return load + + +def test_an_empty_query_returns_every_project_it_can(session, only_projects, load_options): + projects = only_projects(5) + + options = load_options() + + assert {o["value"] for o in options} == {str(p.id) for p in projects} + + +def test_options_are_bounded_by_slacks_limit(session, only_projects, load_options): + """1000 projects, one query: Slack accepts at most 100 options back.""" + only_projects(1000) + + assert len(load_options()) == MAX_SELECT_OPTIONS + + +def test_a_query_matching_more_than_slack_allows_is_still_bounded( + session, only_projects, load_options +): + only_projects(display_names=[f"security-{i:04d}" for i in range(500)]) + + assert len(load_options("security")) == MAX_SELECT_OPTIONS + + +def test_a_query_narrows_to_matching_projects(session, only_projects, load_options): + only_projects(display_names=["Security Engineering", "Payments", "Security Ops"]) + + labels = [o["text"]["text"] for o in load_options("sec")] + + assert labels == ["Security Engineering", "Security Ops"] + + +def test_search_is_case_insensitive(session, only_projects, load_options): + only_projects(display_names=["Security Engineering", "Payments"]) + + assert load_options("SEC") == load_options("sec") + + +def test_a_project_is_findable_by_name_as_well_as_display_name( + session, only_projects, load_options +): + projects = only_projects(display_names=["Totally Different"]) + + options = load_options(projects[0].name) + + assert [o["value"] for o in options] == [str(projects[0].id)] + + +def test_no_matches_returns_an_empty_option_set(session, only_projects, load_options): + only_projects(display_names=["Payments"]) + + assert load_options("nothing-matches-this") == [] + + +def test_no_projects_returns_an_empty_option_set(session, only_projects, load_options): + only_projects(0) + + assert load_options() == [] + + +def test_disabled_projects_are_not_offered(session, only_projects, load_options): + projects = only_projects(display_names=["Security One", "Security Two"]) + projects[0].enabled = False + session.commit() + + assert [o["value"] for o in load_options("security")] == [str(projects[1].id)] + + +def test_option_values_are_project_ids_not_names(session, only_projects, load_options): + projects = only_projects(display_names=["Security", "Security"]) + + options = load_options("security") + + assert [o["text"]["text"] for o in options] == ["Security", "Security"] + assert {o["value"] for o in options} == {str(p.id) for p in projects} + + +def test_an_empty_display_name_falls_back_to_the_project_name(session, only_projects, load_options): + projects = only_projects(display_names=[""]) + + options = load_options(projects[0].name) + + assert [o["text"]["text"] for o in options] == [projects[0].name] + + +def test_a_long_display_name_is_truncated_to_slacks_limit(session, only_projects, load_options): + only_projects(display_names=["y" * 200]) + + assert load_options("yyy")[0]["text"]["text"] == "y" * 75 + + +def test_results_are_ordered_by_label_case_insensitively(session, only_projects, load_options): + only_projects(display_names=["zulu match", "Alpha match", "mike match"]) + + labels = [o["text"]["text"] for o in load_options("match")] + + assert labels == ["Alpha match", "mike match", "zulu match"] + + +def test_repeating_a_query_returns_the_same_order(session, only_projects, load_options): + only_projects(60) + + assert load_options() == load_options() + + +def test_a_wildcard_in_the_query_is_matched_literally(session, only_projects, load_options): + """An unescaped `%` would match every project instead of none.""" + only_projects(display_names=["Payments", "Security"]) + + assert load_options("%") == [] + + +@pytest.mark.parametrize("action_id", PROJECT_SELECT_ACTION_IDS, ids=str) +def test_every_project_select_action_id_is_served(session, only_projects, load_options, action_id): + """A select whose action id has no options handler opens to an empty menu.""" + projects = only_projects(2) + + options = load_options(action_id=action_id) + + assert {o["value"] for o in options} == {str(p.id) for p in projects} + + +def test_a_failed_lookup_answers_with_no_options_rather_than_an_error( + session, only_projects, load_options +): + """Slack shows the user an empty menu; it must not see a 500 or a traceback.""" + only_projects(3) + + with patch( + "dispatch.project.service.get_all_enabled", side_effect=RuntimeError("database is down") + ): + assert load_options() == [] diff --git a/tests/plugins/dispatch_slack/test_project_select.py b/tests/plugins/dispatch_slack/test_project_select.py new file mode 100644 index 000000000..2e12402a9 --- /dev/null +++ b/tests/plugins/dispatch_slack/test_project_select.py @@ -0,0 +1,202 @@ +"""Regression coverage for the project select behind every incident/case modal. + +Slack caps a static select at 100 options, so a deployment with more enabled +projects than that could not open any modal built through ``project_select`` +(#86). These tests drive the real builder against real rows either side of that +boundary and assert on the Block Kit payload it produces, not merely that it +does not raise. +""" + +import pytest +from blockkit.core import FieldValidationError + +from dispatch.plugins.dispatch_slack.config import MAX_SELECT_OPTIONS +from dispatch.plugins.dispatch_slack.fields import ( + DefaultActionIds, + DefaultBlockIds, + project_select, +) +from dispatch.project.models import Project + + +def build(db_session, **kwargs) -> dict | None: + """Build the project select the way a modal does.""" + block = project_select(db_session=db_session, **kwargs) + return None if block is None else block.build() + + +def element(view_block: dict) -> dict: + return view_block["element"] + + +# One count either side of the boundary, for the properties that must hold in +# both modes. +BOTH_MODES = [2, MAX_SELECT_OPTIONS + 1] + + +def test_no_projects_yields_no_block(session, only_projects): + only_projects(0) + + assert build(session) is None + + +def test_one_project_yields_a_single_static_option(session, only_projects): + only_projects(1) + + block = build(session) + + assert element(block)["type"] == "static_select" + assert len(element(block)["options"]) == 1 + + +@pytest.mark.parametrize("count", [1, 99, MAX_SELECT_OPTIONS]) +def test_static_select_up_to_the_slack_limit(session, only_projects, count): + """At or below Slack's cap the options stay embedded, as they always were.""" + only_projects(count) + + block = build(session) + + assert element(block)["type"] == "static_select" + assert len(element(block)["options"]) == count + + +@pytest.mark.parametrize("count", [MAX_SELECT_OPTIONS + 1, 500, 1000]) +def test_external_select_past_the_slack_limit(session, only_projects, count): + """The original #86 failure: more projects than a static select can hold.""" + only_projects(count) + + block = build(session) + + assert element(block)["type"] == "external_select" + assert "options" not in element(block), "the whole project list is still embedded" + + +def test_the_original_919_project_failure(session, only_projects): + """The count that tripped the limit when #86 was found.""" + only_projects(919) + + view_block = build(session) + + assert element(view_block)["type"] == "external_select" + + +def test_one_option_past_the_limit_is_what_a_static_select_rejects(): + """Why the mode switches where it does -- the constraint being designed around.""" + from dispatch.plugins.dispatch_slack.fields import static_select_block + + options = [{"text": f"p{i}", "value": str(i)} for i in range(MAX_SELECT_OPTIONS + 1)] + + with pytest.raises(FieldValidationError): + static_select_block(options=options, placeholder="Select Project", label="Project").build() + + +@pytest.mark.parametrize("count", BOTH_MODES) +def test_block_and_action_ids_are_unchanged_in_either_mode(session, only_projects, count): + """Every caller reads state back by these ids; they must not move.""" + only_projects(count) + + block = build(session) + + assert block["block_id"] == DefaultBlockIds.project_select + assert element(block)["action_id"] == DefaultActionIds.project_select + + +@pytest.mark.parametrize("count", BOTH_MODES) +def test_dispatch_action_survives_in_either_mode(session, only_projects, count): + """Selecting a project re-renders the modal; that needs dispatch_action.""" + only_projects(count) + + block = build(session, dispatch_action=True) + + assert block["dispatch_action"] is True + + +def test_option_values_are_project_ids(session, only_projects): + projects = only_projects(3) + + options = element(build(session))["options"] + + assert {o["value"] for o in options} == {str(p.id) for p in projects} + # Slack drops an option with a non-string value without saying so. + assert all(isinstance(o["value"], str) for o in options) + + +@pytest.mark.parametrize("count", BOTH_MODES) +def test_initial_option_is_preserved_in_either_mode(session, only_projects, count): + projects = only_projects(count) + chosen = projects[0] + + block = build(session, initial_option={"text": chosen.display_name, "value": chosen.id}) + + assert element(block)["initial_option"]["value"] == str(chosen.id) + + +def test_disabled_projects_are_excluded(session, only_projects): + projects = only_projects(4) + projects[0].enabled = False + projects[1].enabled = False + session.commit() + + options = element(build(session))["options"] + + assert {o["value"] for o in options} == {str(p.id) for p in projects[2:]} + + +def test_options_are_ordered_by_label_case_insensitively(session, only_projects): + only_projects(display_names=["zulu", "Alpha", "mike"]) + + labels = [o["text"]["text"] for o in element(build(session))["options"]] + + assert labels == ["Alpha", "mike", "zulu"] + + +def test_ordering_is_stable_across_repeated_builds(session, only_projects): + only_projects(50) + + first = [o["value"] for o in element(build(session))["options"]] + session.expire_all() + second = [o["value"] for o in element(build(session))["options"]] + + assert first == second + + +def test_duplicate_display_names_keep_distinct_values(session, only_projects): + projects = only_projects(2, display_names=["Security", "Security"]) + + options = element(build(session))["options"] + + assert [o["text"]["text"] for o in options] == ["Security", "Security"] + assert {o["value"] for o in options} == {str(p.id) for p in projects} + + +def test_an_empty_display_name_falls_back_to_the_project_name(session, only_projects): + """``display_name`` defaults to '' at the column level, and Slack rejects + empty option text -- so the fallback is what keeps the modal openable.""" + only_projects(1, display_names=[""]) + project = session.query(Project).filter(Project.enabled.is_(True)).one() + + options = element(build(session))["options"] + + assert options[0]["text"]["text"] == project.name + + +def test_a_long_display_name_is_truncated_to_slacks_limit(session, only_projects): + only_projects(1, display_names=["x" * 200]) + + options = element(build(session))["options"] + + assert options[0]["text"]["text"] == "x" * 75 + + +def test_a_project_with_no_name_at_all_is_still_selectable(session, only_projects): + """`name` is nullable and `display_name` defaults to '', so a label has to + come from somewhere -- an option with empty text is rejected outright.""" + project = only_projects(1)[0] + project.name = None + project.display_name = "" + session.commit() + + options = element(build(session))["options"] + + assert [o["text"]["text"] for o in options] == [f"Project {project.id}"] + assert [o["value"] for o in options] == [str(project.id)] diff --git a/tests/plugins/dispatch_slack/test_slack_live.py b/tests/plugins/dispatch_slack/test_slack_live.py index 9af3d61ac..7bd9c9439 100644 --- a/tests/plugins/dispatch_slack/test_slack_live.py +++ b/tests/plugins/dispatch_slack/test_slack_live.py @@ -70,6 +70,7 @@ from dispatch.plugins.dispatch_slack.case.interactive import ( handle_engage_user_command, + handle_escalate_case_command, handle_update_case_command, ) from dispatch.plugins.dispatch_slack.case.messages import ( @@ -177,3 +178,23 @@ def test_incident_modal_is_accepted(slack, handler, build_incident_modal): @pytest.mark.parametrize("handler", CASE_MODAL_HANDLERS, ids=lambda h: h.__name__) def test_case_modal_is_accepted(slack, handler, build_case_modal): assert publish(slack, handler.__name__, build_case_modal(handler)["blocks"])["ok"] + + +# Only Slack can say whether a static select at exactly its limit and an +# external select with no options of its own are both acceptable -- blockkit +# validates against its own model, and #86 is precisely a case where the two +# disagreed. views.publish puts these through the same server-side validation +# as views.open. +PROJECT_SELECT_COUNTS = [1, 100, 101, 500] + + +@needs_user_id +@pytest.mark.parametrize("count", PROJECT_SELECT_COUNTS) +def test_project_select_is_accepted_at_any_project_count( + slack, session, only_projects, build_case_modal, case_with_related_records, count +): + only_projects(count, keep=[case_with_related_records.project]) + + blocks = build_case_modal(handle_escalate_case_command)["blocks"] + + assert publish(slack, f"escalate_case_{count}_projects", blocks)["ok"] diff --git a/tests/project/test_project_service.py b/tests/project/test_project_service.py index e39f1128f..a5d877563 100644 --- a/tests/project/test_project_service.py +++ b/tests/project/test_project_service.py @@ -111,3 +111,31 @@ def test_get_by_name_or_default__default(session, project, organization): project_in = ProjectRead(name="nonexistent", organization=organization) result = get_by_name_or_default(db_session=session, project_in=project_in) assert result.id == project.id + + +def test_get_all_enabled_limits_in_the_database(session): + """The Slack project type-ahead runs on every keystroke against a table + that can hold thousands of rows, so the limit has to be the database's.""" + from sqlalchemy import event + + from dispatch.database.core import engine + from dispatch.project.service import get_all_enabled + from tests.factories import ProjectFactory + + ProjectFactory.create_batch(3, enabled=True) + statements = [] + + def record(conn, cursor, statement, parameters, context, executemany): + statements.append(statement) + + event.listen(engine, "after_cursor_execute", record) + try: + results = get_all_enabled(db_session=session, query_str="proj", limit=2) + finally: + event.remove(engine, "after_cursor_execute", record) + + assert len(results) <= 2 + assert len(statements) == 1, statements + assert "LIMIT" in statements[0].upper() + assert "ORDER BY" in statements[0].upper() + assert "ILIKE" in statements[0].upper()