diff --git a/.agents/feature-map/components/connectors.md b/.agents/feature-map/components/connectors.md index de76038f34..3a1246dd4b 100644 --- a/.agents/feature-map/components/connectors.md +++ b/.agents/feature-map/components/connectors.md @@ -315,7 +315,7 @@ range: |---|---|---|---| | Confluence (`confluence/connector.py:ConfluenceConnector`) | `CheckpointedConnector`, `SlimConnector`, `SlimConnectorWithPermSync`, `CredentialsConnector`, `Resolver` | `ConfluenceCheckpoint` (`next_page_url`) | No `LoadConnector`/`PollConnector`; checkpointing is mandatory, not optional. `load_credentials` deliberately raises `NotImplementedError("Use set_credentials_provider with this connector.")`. Has a `source_operations.py` gateway (`ConfluenceSourceOperations`) and registers named capability checks. | | Google Drive (`google_drive/connector.py:GoogleDriveConnector`) | `SlimConnector`, `SlimConnectorWithPermSync`, `CheckpointedConnectorWithPermSync`, `Resolver` | `GoogleDriveCheckpoint` (`google_drive/models.py`): a multi-stage state machine (`DriveRetrievalStage`: `START → OAUTH_FILES → USER_EMAILS → MY_DRIVE_FILES → DRIVE_IDS → SHARED_DRIVE_FILES → DONE`) plus a per-impersonated-user completion map. Not a simple cursor. | Splits source-API logic across `file_retrieval.py`, `doc_conversion.py`, `section_extraction.py` rather than a single `source_operations.py`. | -| Slack (`slack/connector.py:SlackConnector`) | `SlimConnectorWithPermSync`, `CredentialsConnector`, `CheckpointedConnectorWithPermSync` | `SlackCheckpoint`: `channel_ids`, per-channel completion map, `current_channel_access` (carries an in-flight channel's `ExternalAccess` across a checkpoint boundary) | Has a real `source_operations.py` (`SlackSourceOperations`), the enforced single import site for `slack_sdk`. `external_access` is assigned inline on yielded documents, not in a separate pass. | +| Slack (`slack/connector.py:SlackConnector`) | `SlimConnectorWithPermSync`, `CredentialsConnector`, `CheckpointedConnectorWithPermSync` | `SlackCheckpoint`: `channel_ids`, per-channel completion map, `current_channel_access` (carries an in-flight channel's `ExternalAccess` across a checkpoint boundary) | Has a real `source_operations.py` (`SlackSourceOperations`), the enforced single import site for `slack_sdk`. An exact `channels` or `exclude_channels` entry is a channel id or a name (`filter_channels`); the readable-channel listing check offers ids as values with names as labels, so a picked channel survives a rename. `external_access` is assigned inline on yielded documents, not in a separate pass. | ### 4.6.1 Outlook: one document per mail thread diff --git a/backend/ee/onyx/external_permissions/slack/doc_sync.py b/backend/ee/onyx/external_permissions/slack/doc_sync.py index 478a88ea12..fbfe2a7d4e 100644 --- a/backend/ee/onyx/external_permissions/slack/doc_sync.py +++ b/backend/ee/onyx/external_permissions/slack/doc_sync.py @@ -17,10 +17,12 @@ from onyx.connectors.interfaces import SecondsSinceUnixEpoch from onyx.connectors.models import HierarchyNode from onyx.connectors.slack.connector import ( SlackConnector, + channel_keys, filter_channels, get_channels, get_channels_across_teams, list_grid_team_ids, + normalize_channel_entries, ) from onyx.connectors.slack.models import ChannelType from onyx.connectors.slack.source_operations import SlackSourceOperations @@ -54,11 +56,10 @@ def _filter_channels_for_permissions( exclude_regex_enabled: bool = False, ) -> list[ChannelType]: if channels_to_include and not include_regex_enabled: - available_channel_names = {channel["name"] for channel in all_channels} + channels_to_include = normalize_channel_entries(channels_to_include) + available_channels = channel_keys(all_channels) available_channels_to_include = [ - channel - for channel in channels_to_include - if channel in available_channel_names + channel for channel in channels_to_include if channel in available_channels ] missing_channels = sorted( set(channels_to_include) - set(available_channels_to_include) diff --git a/backend/onyx/connectors/slack/capability_checks.py b/backend/onyx/connectors/slack/capability_checks.py index 3c6851e0df..d9e2cc39e0 100644 --- a/backend/onyx/connectors/slack/capability_checks.py +++ b/backend/onyx/connectors/slack/capability_checks.py @@ -32,6 +32,8 @@ from collections.abc import Iterator from itertools import islice from typing import Any, NoReturn +from pydantic import BaseModel + from onyx.connectors.capability_checks.findings import bounded_options from onyx.connectors.capability_checks.models import ( CapabilityCheck, @@ -52,9 +54,11 @@ from onyx.connectors.exceptions import ( ) from onyx.connectors.slack.config import SlackConnectorConfig from onyx.connectors.slack.connector import ( + channel_keys, get_channels, get_channels_across_teams, list_grid_team_ids, + normalize_channel_entries, validate_channel_regexes, ) from onyx.connectors.slack.source_operations import ( @@ -661,7 +665,7 @@ class _ChannelPatternsCheck(CapabilityCheck[SlackConnectorConfig]): class _ConfiguredChannelsVisibleCheck(CapabilityCheck[SlackConnectorConfig]): - """Verifies every configured channel name is visible to the bot. + """Verifies every configured channel id or name is visible to the bot. Existed only as commented-out code in ``validate_connector_settings`` (removed as too slow for a synchronous request); resurrected here where slow @@ -713,9 +717,9 @@ class _ConfiguredChannelsVisibleCheck(CapabilityCheck[SlackConnectorConfig]): "(`conversations.list`), so the configured channels could not " "be verified.", ) - visible_names = {channel["name"] for channel in all_channels} - configured_names = [name.removeprefix("#") for name in channels_to_include] - missing = sorted(set(configured_names) - visible_names) + visible_channels = channel_keys(all_channels) + configured = normalize_channel_entries(channels_to_include) + missing = sorted(set(configured) - visible_channels) if missing: raise ConnectorValidationError( f"Configured channels are not visible to the bot: {missing}. " @@ -726,20 +730,36 @@ class _ConfiguredChannelsVisibleCheck(CapabilityCheck[SlackConnectorConfig]): return None +class _ListedChannel(BaseModel): + channel: dict[str, Any] + # The Grid workspace the channel was listed in, which tells two + # same-named channels apart. None off Grid. + workspace: str | None + + +def _grid_workspaces(slack_client: SlackSourceOperations) -> dict[str, str | None]: + """Grid team id to workspace name via ``auth.teams.list``.""" + return { + team["id"]: team.get("name") + for page in slack_client.list_teams() + for team in page.teams + if team.get("id") + } + + def _iter_public_channels( slack_client: SlackSourceOperations, -) -> Iterator[dict[str, Any]]: +) -> Iterator[_ListedChannel]: """Every unarchived public channel, read page by page. On Enterprise Grid, - lists each workspace in turn, as the connector does. The connector matches - ``channels`` by name, so channels that share a name across workspaces are - one option.""" - team_ids: list[str | None] = ( - [*list_grid_team_ids(slack_client)] + lists each workspace in turn, as the connector does. A channel shared into + several workspaces is listed once, by id.""" + workspaces: dict[str | None, str | None] = ( + {**_grid_workspaces(slack_client)} if _auth_test(slack_client).enterprise_id - else [None] + else {None: None} ) - seen_names: set[str] = set() - for team_id in team_ids: + seen_ids: set[str] = set() + for team_id, workspace in workspaces.items(): for page in slack_client.list_channels( variant=SlackChannelVariant.PUBLIC, channel_types=["public_channel"], @@ -747,10 +767,10 @@ def _iter_public_channels( team_id=team_id, ): for channel in page.channels: - if channel["name"] in seen_names: + if channel["id"] in seen_ids: continue - seen_names.add(channel["name"]) - yield channel + seen_ids.add(channel["id"]) + yield _ListedChannel(channel=channel, workspace=workspace) class _ReadableChannelListingCheck(CapabilityCheck): @@ -771,13 +791,14 @@ class _ReadableChannelListingCheck(CapabilityCheck): def run(self, context: CapabilityCheckContext) -> CheckOutput: try: - # The connector matches ``channels`` against bare channel names. + # The id survives a rename. The label is the name the admin reads. finding: OptionsFinding = bounded_options( _iter_public_channels(_slack_client(context)), key=CheckFindingKey.READABLE_CHANNELS, field_name=_CHANNELS_FIELD, - value=lambda channel: channel["name"], - label=lambda channel: channel["name"], + value=lambda listed: listed.channel["id"], + label=lambda listed: listed.channel["name"], + detail=lambda listed: listed.workspace, ) except SlackApiError as e: _raise_for_slack_api_error( diff --git a/backend/onyx/connectors/slack/connector.py b/backend/onyx/connectors/slack/connector.py index 97aa97a983..6f1ff687e4 100644 --- a/backend/onyx/connectors/slack/connector.py +++ b/backend/onyx/connectors/slack/connector.py @@ -468,34 +468,56 @@ def filter_channels( if channels_to_include: if not include_regex_enabled: + channels_to_include = normalize_channel_entries(channels_to_include) _validate_channels_exist(all_channels, channels_to_include) filtered_channels = [ channel for channel in filtered_channels - if _channel_name_matches( - channel["name"], channels_to_include, include_regex_enabled - ) + if _channel_matches(channel, channels_to_include, include_regex_enabled) ] - # unlike includes, exclude names aren't validated — excluding a missing channel is harmless + # unlike includes, exclude entries aren't validated: excluding a missing channel is harmless if channels_to_exclude: + if not exclude_regex_enabled: + channels_to_exclude = normalize_channel_entries(channels_to_exclude) filtered_channels = [ channel for channel in filtered_channels - if not _channel_name_matches( - channel["name"], channels_to_exclude, exclude_regex_enabled - ) + if not _channel_matches(channel, channels_to_exclude, exclude_regex_enabled) ] return filtered_channels -def _channel_name_matches( - channel_name: str, patterns: list[str], regex_enabled: bool +# A channel id: a type letter, then uppercase letters and digits. A channel +# name is lowercase, so the two never collide. +_CHANNEL_ID_PATTERN = re.compile(r"[CGD][A-Z0-9]{8,}") + + +def normalize_channel_entries(entries: list[str]) -> list[str]: + """Exact config entries as Slack stores them: an id as given, a typed name + lowercase without its leading ``#``.""" + return [ + entry + if _CHANNEL_ID_PATTERN.fullmatch(entry) + else entry.removeprefix("#").lower() + for entry in entries + ] + + +def _channel_matches( + channel: ChannelType, patterns: list[str], regex_enabled: bool ) -> bool: + """An exact entry names the channel by id, which survives a rename, or by + name. A regex matches the name only.""" if regex_enabled: - return any(re.fullmatch(pattern, channel_name) for pattern in patterns) - return channel_name in patterns + return any(re.fullmatch(pattern, channel["name"]) for pattern in patterns) + return channel["id"] in patterns or channel["name"] in patterns + + +def channel_keys(channels: list[ChannelType]) -> set[str]: + """Every string an exact config entry may use for one of ``channels``.""" + return {key for channel in channels for key in (channel["id"], channel["name"])} def _validate_channels_exist( @@ -503,9 +525,10 @@ def _validate_channels_exist( ) -> None: # fail loudly on an unknown channel so the user knows one of the # channels they've specified is typo'd or private + known_channels = channel_keys(all_channels) all_channel_names = {channel["name"] for channel in all_channels} for channel in channels_to_include: - if channel not in all_channel_names: + if channel not in known_channels: raise ValueError( f"Channel '{channel}' not found in workspace. " f"Available channels (Showing {len(all_channel_names)} of " diff --git a/backend/tests/unit/ee/onyx/external_permissions/slack/test_grid_perm_sync.py b/backend/tests/unit/ee/onyx/external_permissions/slack/test_grid_perm_sync.py index 107aec8748..33e85722b3 100644 --- a/backend/tests/unit/ee/onyx/external_permissions/slack/test_grid_perm_sync.py +++ b/backend/tests/unit/ee/onyx/external_permissions/slack/test_grid_perm_sync.py @@ -287,6 +287,33 @@ class TestFetchChannelPermissionsGrid: == "C_INCLUDED" ) + def test_channel_filter_matches_channel_ids(self) -> None: + slack_client = MagicMock() + included = _channel("CINCLUDED01", name="renamed", is_private=True) + excluded = _channel("CEXCLUDED01", name="excluded", is_private=True) + with patch( + "ee.onyx.external_permissions.slack.doc_sync.get_channels" + ) as mock_get: + mock_get.side_effect = [[], [included, excluded]] # public, private + slack_client.list_channel_members.return_value = iter( + [SlackChannelMembersPage(members=["U1"])] + ) + workspace_perm = _fetch_workspace_permissions({"U1": "u1@x.com"}) + # The typed name keeps its case: an id is matched as given. + result = _fetch_channel_permissions( + slack_client=slack_client, + workspace_permissions=workspace_perm, + user_id_to_email_map={"U1": "u1@x.com"}, + channels_to_include=["CINCLUDED01"], + ) + + assert set(result) == {"CINCLUDED01"} + slack_client.list_channel_members.assert_called_once() + assert ( + slack_client.list_channel_members.call_args.kwargs["channel_id"] + == "CINCLUDED01" + ) + def test_channel_filter_skips_missing_included_channels(self) -> None: slack_client = MagicMock() included = _channel("C_INCLUDED", name="included", is_private=True) diff --git a/backend/tests/unit/onyx/connectors/slack/test_slack_capability_checks.py b/backend/tests/unit/onyx/connectors/slack/test_slack_capability_checks.py index b334ed93f5..b77f402f8b 100644 --- a/backend/tests/unit/onyx/connectors/slack/test_slack_capability_checks.py +++ b/backend/tests/unit/onyx/connectors/slack/test_slack_capability_checks.py @@ -969,9 +969,22 @@ def test_configured_channels_all_visible_passes() -> None: gateway.list_channels.side_effect = _pages( SlackChannelsPage(channels=[{"id": "C1", "name": "general"}]) ) - # The ``#`` prefix must be stripped, mirroring the connector's ``channels`` - # setter. - context = _context(gateway, connector_specific_config={"channels": ["#general"]}) + # A typed name is matched without its ``#`` and in any case, as the + # connector normalizes it. + context = _context(gateway, connector_specific_config={"channels": ["#General"]}) + + # Under test and postcondition. + _run("slack_configured_channels_visible", context) + + +def test_configured_channels_by_id_pass_after_a_rename() -> None: + """Verifies the config check accepts a channel id whose name changed.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C0GENERAL1", "name": "general-renamed"}]) + ) + context = _context(gateway, connector_specific_config={"channels": ["C0GENERAL1"]}) # Under test and postcondition. _run("slack_configured_channels_visible", context) @@ -1017,7 +1030,7 @@ def test_configured_channels_regex_mode_skips_probe() -> None: # slack_readable_channel_listing -def test_readable_channel_listing_finds_channel_names_as_channel_options() -> None: +def test_readable_channel_listing_finds_channel_ids_as_channel_options() -> None: # Precondition. gateway = _gateway() gateway.list_channels.side_effect = lambda **_: iter( @@ -1038,8 +1051,8 @@ def test_readable_channel_listing_finds_channel_names_as_channel_options() -> No key=CheckFindingKey.READABLE_CHANNELS, field_name="channels", options=[ - FindingOption(value="general", label="general"), - FindingOption(value="random", label="random"), + FindingOption(value="C1", label="general"), + FindingOption(value="C2", label="random"), ], ) ] @@ -1056,12 +1069,15 @@ def test_readable_channel_listing_lists_each_grid_workspace() -> None: gateway = _gateway() gateway.check_auth.return_value = _auth_response(enterprise_id="E1") gateway.list_teams.side_effect = _pages( - SlackTeamsPage(teams=[{"id": "T1"}, {"id": "T2"}]) + SlackTeamsPage( + teams=[{"id": "T1", "name": "Acme"}, {"id": "T2", "name": "Labs"}] + ) ) channels_by_team = { "T1": [{"id": "C1", "name": "general"}], - # A shared channel appears in both workspaces, and another workspace - # has its own channel with the same name: both are one option. + # A shared channel appears in both workspaces and is one option. Another + # workspace's own channel with the same name is a second option, told + # apart by its workspace. "T2": [ {"id": "C1", "name": "general"}, {"id": "C3", "name": "general"}, @@ -1080,7 +1096,13 @@ def test_readable_channel_listing_lists_each_grid_workspace() -> None: # Postcondition. listing: CheckFinding = findings[0] assert isinstance(listing, OptionsFinding) - assert [option.value for option in listing.options] == ["general", "eng"] + assert [ + (option.value, option.label, option.detail) for option in listing.options + ] == [ + ("C1", "general", "Acme"), + ("C3", "general", "Labs"), + ("C2", "eng", "Labs"), + ] def test_readable_channel_listing_stops_at_the_option_cap() -> None: diff --git a/backend/tests/unit/onyx/connectors/slack/test_slack_filter_channels.py b/backend/tests/unit/onyx/connectors/slack/test_slack_filter_channels.py index 68cef77628..29ebc3ea81 100644 --- a/backend/tests/unit/onyx/connectors/slack/test_slack_filter_channels.py +++ b/backend/tests/unit/onyx/connectors/slack/test_slack_filter_channels.py @@ -1,6 +1,7 @@ """Channel include/exclude filtering for the Slack connector: excludes apply -after includes, support exact-name and full-match regex modes, and never fail -on names absent from the workspace.""" +after includes, exact entries match a channel id or name, regex entries match +the name in full, and excludes never fail on entries absent from the +workspace.""" from typing import Any, cast @@ -12,7 +13,7 @@ from onyx.connectors.slack.models import ChannelType def _channel(name: str) -> ChannelType: - base: dict[str, Any] = {"id": f"C-{name}", "name": name} + base: dict[str, Any] = {"id": f"C0{name.upper().replace('-', '')}", "name": name} return cast(ChannelType, base) @@ -116,6 +117,48 @@ def test_exclude_regex_is_full_match() -> None: assert result == CHANNELS +def test_include_exact_matches_channel_id() -> None: + result = filter_channels(CHANNELS, ["C0SUPPORT", "C0BILLINGALERTS"], False) + assert _names(result) == ["support", "billing-alerts"] + + +def test_include_exact_mixes_ids_and_names() -> None: + result = filter_channels(CHANNELS, ["C0SUPPORT", "general"], False) + assert _names(result) == ["general", "support"] + + +def test_exact_entries_take_typed_names_in_any_case_with_a_hash() -> None: + result = filter_channels( + CHANNELS, + ["#Support", "C0GENERAL"], + False, + channels_to_exclude=["General"], + exclude_regex_enabled=False, + ) + assert _names(result) == ["support"] + + +def test_include_regex_never_matches_channel_id() -> None: + result = filter_channels(CHANNELS, ["C0.*"], True) + assert result == [] + + +def test_exclude_exact_matches_channel_id() -> None: + result = filter_channels( + CHANNELS, + None, + False, + channels_to_exclude=["C0SUPPORT", "infra-alerts"], + exclude_regex_enabled=False, + ) + assert _names(result) == ["general", "billing-alerts", "deploy-notifications"] + + +def test_include_validation_raises_for_unknown_channel_id() -> None: + with pytest.raises(ValueError, match="C0RENAMED.*not found in workspace"): + filter_channels(CHANNELS, ["C0SUPPORT", "C0RENAMED"], False) + + def test_include_validation_still_raises_for_unknown_channel() -> None: with pytest.raises(ValueError, match="not found in workspace"): filter_channels(CHANNELS, ["typo-channel"], False) diff --git a/web/src/i18n/messages/ar.json b/web/src/i18n/messages/ar.json index a17e21f6a4..ee82e44b00 100644 --- a/web/src/i18n/messages/ar.json +++ b/web/src/i18n/messages/ar.json @@ -3134,6 +3134,9 @@ "title": "صلاحيات إدارة الموصل" }, "title": "إعدادات الموصل" + }, + "optionsListInput": { + "placeholder": "اختر {label} من القائمة أو أدخله" } }, "costOverrides": { diff --git a/web/src/i18n/messages/de.json b/web/src/i18n/messages/de.json index db7a4ce92f..ce6e94e098 100644 --- a/web/src/i18n/messages/de.json +++ b/web/src/i18n/messages/de.json @@ -3134,6 +3134,9 @@ "title": "Verwaltungszugriff für den Connector" }, "title": "Connector-Einstellungen" + }, + "optionsListInput": { + "placeholder": "{label} aus der Liste wählen oder eingeben" } }, "costOverrides": { diff --git a/web/src/i18n/messages/en.json b/web/src/i18n/messages/en.json index 25a8038ac1..72786d9f8d 100644 --- a/web/src/i18n/messages/en.json +++ b/web/src/i18n/messages/en.json @@ -3134,6 +3134,9 @@ "title": "Connector Manage Access" }, "title": "Connector Settings" + }, + "optionsListInput": { + "placeholder": "Pick {label} from the list, or type one" } }, "costOverrides": { diff --git a/web/src/i18n/messages/es.json b/web/src/i18n/messages/es.json index 87f9e58e4d..778cbd13c3 100644 --- a/web/src/i18n/messages/es.json +++ b/web/src/i18n/messages/es.json @@ -3134,6 +3134,9 @@ "title": "Acceso de gestión del conector" }, "title": "Configuración del conector" + }, + "optionsListInput": { + "placeholder": "Elige {label} de la lista o escríbelo" } }, "costOverrides": { diff --git a/web/src/i18n/messages/fr.json b/web/src/i18n/messages/fr.json index df4846bab0..08a1b321e9 100644 --- a/web/src/i18n/messages/fr.json +++ b/web/src/i18n/messages/fr.json @@ -3134,6 +3134,9 @@ "title": "Accès de gestion du connecteur" }, "title": "Paramètres du connecteur" + }, + "optionsListInput": { + "placeholder": "Choisissez {label} dans la liste ou saisissez-en" } }, "costOverrides": { diff --git a/web/src/i18n/messages/ja.json b/web/src/i18n/messages/ja.json index 4e9066e777..9f9a6e3627 100644 --- a/web/src/i18n/messages/ja.json +++ b/web/src/i18n/messages/ja.json @@ -3134,6 +3134,9 @@ "title": "コネクタの管理アクセス" }, "title": "コネクタ設定" + }, + "optionsListInput": { + "placeholder": "リストから {label} を選択するか入力" } }, "costOverrides": { diff --git a/web/src/i18n/messages/ko.json b/web/src/i18n/messages/ko.json index fecd5cc300..21dd0c9d90 100644 --- a/web/src/i18n/messages/ko.json +++ b/web/src/i18n/messages/ko.json @@ -3134,6 +3134,9 @@ "title": "커넥터 관리 액세스" }, "title": "커넥터 설정" + }, + "optionsListInput": { + "placeholder": "목록에서 {label} 선택 또는 입력" } }, "costOverrides": { diff --git a/web/src/i18n/messages/pt.json b/web/src/i18n/messages/pt.json index 9447d0f6f5..a6ed4a96f1 100644 --- a/web/src/i18n/messages/pt.json +++ b/web/src/i18n/messages/pt.json @@ -3134,6 +3134,9 @@ "title": "Acesso de gerenciamento do conector" }, "title": "Configurações do conector" + }, + "optionsListInput": { + "placeholder": "Escolha {label} na lista ou digite" } }, "costOverrides": { diff --git a/web/src/i18n/messages/zh.json b/web/src/i18n/messages/zh.json index b0ac7363b6..02a270d6b7 100644 --- a/web/src/i18n/messages/zh.json +++ b/web/src/i18n/messages/zh.json @@ -3134,6 +3134,9 @@ "title": "连接器管理权限" }, "title": "连接器设置" + }, + "optionsListInput": { + "placeholder": "从列表中选择{label}或输入" } }, "costOverrides": { diff --git a/web/src/lib/connectors/checks/findings.test.ts b/web/src/lib/connectors/checks/findings.test.ts new file mode 100644 index 0000000000..5b0de54362 --- /dev/null +++ b/web/src/lib/connectors/checks/findings.test.ts @@ -0,0 +1,119 @@ +/** Listing-backed config fields: the first non-empty listing wins, stored ids + * show as their labels, and the pair's report is read before the credential's. */ +import { + findingOptions, + labelFindingValues, + storedCheckResults, +} from "@/lib/connectors/checks/findings"; +import type { + CapabilityCheckResult, + CheckFinding, +} from "@/lib/connectors/checks/types"; +import type { ConnectionConfiguration } from "@/lib/connectors/types"; +import type { CredentialCheckReport } from "@/lib/credentials/types"; + +function result(findings: CheckFinding[]): CapabilityCheckResult { + return { + capability: "indexing", + check_id: "listing", + display_name: "Listing", + required: false, + status: "passed", + message: "", + remediation: null, + docs_link: null, + duration_ms: null, + findings, + }; +} + +function channels(options: { value: string; label: string }[]): CheckFinding { + return { + type: "options", + key: "readable_channels", + field_name: "channels", + options: options.map((option) => ({ ...option, detail: null })), + truncated: false, + }; +} + +const GENERAL = { value: "C1", label: "general" }; + +test("findingOptions skips an empty listing for a later full one", () => { + const checks = [result([channels([])]), result([channels([GENERAL])])]; + expect(findingOptions(checks, "readable_channels")).toEqual([ + { ...GENERAL, detail: null }, + ]); + expect(findingOptions(checks, "shared_drives")).toEqual([]); +}); + +test("labelFindingValues shows ids by label and leaves the rest", () => { + const configuration: ConnectionConfiguration = { + description: "", + values: [], + advanced_values: [ + { + type: "tab", + name: "mode", + tabs: [ + { + label: "Channels", + value: "channels", + fields: [ + { + type: "list", + label: "Channels", + name: "channels", + optionsFinding: "readable_channels", + }, + ], + }, + ], + }, + { type: "list", label: "Patterns", name: "patterns" }, + ], + }; + const labeled = labelFindingValues( + configuration, + { channels: ["C1", "typed-name"], patterns: ["C1"], flag: true }, + [result([channels([GENERAL])])] + ); + expect(labeled).toEqual({ + channels: ["general", "typed-name"], + patterns: ["C1"], + flag: true, + }); +}); + +test("storedCheckResults reads the pair's report before the credential's", () => { + const report = ( + connectorId: number | null, + label: string + ): CredentialCheckReport => ({ + credential_id: 7, + connector_id: connectorId, + run_status: "completed", + run_started_at: null, + report: { + checked_at: "2026-10-13T00:00:00Z", + check_results: [result([channels([{ value: "C1", label }])])], + }, + time_updated: "2026-10-13T00:00:00Z", + }); + const reports = [report(3, "from-pair"), report(null, "from-credential")]; + const reportFor = (credentialId: number, connectorId: number | null = null) => + reports.find( + (candidate) => + candidate.credential_id === credentialId && + candidate.connector_id === connectorId + ) ?? null; + + const labels = (connectorId: number | null) => + findingOptions( + storedCheckResults(reportFor, { credentialId: 7, connectorId }), + "readable_channels" + ).map((option) => option.label); + expect(labels(3)).toEqual(["from-pair"]); + expect(labels(null)).toEqual(["from-credential"]); + expect(labels(9)).toEqual(["from-credential"]); +}); diff --git a/web/src/lib/connectors/checks/findings.ts b/web/src/lib/connectors/checks/findings.ts new file mode 100644 index 0000000000..cac5f2f934 --- /dev/null +++ b/web/src/lib/connectors/checks/findings.ts @@ -0,0 +1,81 @@ +import type { + CapabilityCheckResult, + CheckFinding, + CheckFindingKey, + FindingOption, +} from "@/lib/connectors/checks/types"; +import { configFields } from "@/lib/connectors/checks/formState"; +import type { ConnectionConfiguration } from "@/lib/connectors/types"; +import type { CredentialCheckReports } from "@/lib/credentials/hooks"; + +/** Anything that carries findings: a draft check state or a stored result. */ +interface WithFindings { + findings: CheckFinding[]; +} + +/** The first non-empty options the checks listed under `key`, else empty. */ +export function findingOptions( + checks: readonly WithFindings[], + key: CheckFindingKey +): FindingOption[] { + for (const check of checks) { + for (const finding of check.findings) { + if ( + finding.type === "options" && + finding.key === key && + finding.options.length > 0 + ) { + return finding.options; + } + } + } + return []; +} + +/** The stored reports a form or page may read listings from. */ +export interface StoredReportScope { + credentialId: number; + /** The pair being edited or shown, whose own report is read first. */ + connectorId: number | null; +} + +/** The pair's stored check results, then the credential's own. */ +export function storedCheckResults( + reportFor: CredentialCheckReports["reportFor"], + scope: StoredReportScope +): CapabilityCheckResult[] { + const reports = [ + scope.connectorId === null + ? null + : reportFor(scope.credentialId, scope.connectorId), + reportFor(scope.credentialId), + ]; + return reports.flatMap((report) => report?.report?.check_results ?? []); +} + +/** The config with listing-backed entries shown by option label. Entries the listing lacks stay as stored. */ +export function labelFindingValues( + configuration: ConnectionConfiguration, + config: Record, + checks: readonly WithFindings[] +): Record { + const labeled: Record = { ...config }; + for (const field of configFields([ + ...configuration.values, + ...configuration.advanced_values, + ])) { + if (field.type !== "list" || field.optionsFinding === undefined) continue; + const stored = config[field.name]; + if (!Array.isArray(stored)) continue; + const labels = new Map( + findingOptions(checks, field.optionsFinding).map((option) => [ + option.value, + option.label, + ]) + ); + labeled[field.name] = stored.map((entry) => + typeof entry === "string" ? (labels.get(entry) ?? entry) : entry + ); + } + return labeled; +} diff --git a/web/src/lib/connectors/checks/formState.ts b/web/src/lib/connectors/checks/formState.ts index 2567c55643..0e1d368162 100644 --- a/web/src/lib/connectors/checks/formState.ts +++ b/web/src/lib/connectors/checks/formState.ts @@ -3,17 +3,22 @@ import type { ValidSources } from "@/lib/connectors/types/source"; import { withoutFormOnlyFields } from "@/lib/connectors/utils"; type ConfigField = ConnectionConfiguration["values"][number]; +type ConfigValueField = Exclude; -/** Every config field name, tab contents included; tab selectors are form +/** Every config field, tab contents included. Tab selectors are form * controls, not config. */ -function configFieldNames(fields: ConfigField[]): string[] { +export function configFields(fields: ConfigField[]): ConfigValueField[] { return fields.flatMap((field) => field.type === "tab" - ? field.tabs.flatMap((tab) => configFieldNames(tab.fields)) - : [field.name] + ? field.tabs.flatMap((tab) => configFields(tab.fields)) + : [field] ); } +function configFieldNames(fields: ConfigField[]): string[] { + return configFields(fields).map((field) => field.name); +} + /** * The connector config an unsaved form holds, for a draft check run or plan. * It takes only the configuration's own fields without the form-only ones diff --git a/web/src/lib/connectors/checks/hooks.test.tsx b/web/src/lib/connectors/checks/hooks.test.tsx index ce5196a226..880f2d7684 100644 --- a/web/src/lib/connectors/checks/hooks.test.tsx +++ b/web/src/lib/connectors/checks/hooks.test.tsx @@ -77,6 +77,7 @@ function check( duration_ms: null, from_cache: false, validates_binding: false, + findings: [], ...overrides, }; } diff --git a/web/src/lib/connectors/checks/hooks.ts b/web/src/lib/connectors/checks/hooks.ts index 1262a0f26f..37b2b58f6c 100644 --- a/web/src/lib/connectors/checks/hooks.ts +++ b/web/src/lib/connectors/checks/hooks.ts @@ -28,18 +28,28 @@ import { useConnectorConfiguration } from "@/lib/connectors/connectors"; import { splitCredentialBoundFields } from "@/lib/connectors/utils"; import { toWireAccess } from "@/lib/connectors/accessType"; import { checksProgress } from "@/lib/connectors/checks/progress"; +import { + findingOptions, + storedCheckResults, + type StoredReportScope, +} from "@/lib/connectors/checks/findings"; import type { CredentialRef } from "@/lib/credentials/types"; -import { useSavedDraftCredential } from "@/lib/credentials/hooks"; +import { + useCredentialCheckReports, + useSavedDraftCredential, +} from "@/lib/credentials/hooks"; import { toCredentialRequest } from "@/lib/credentials/utils"; import type { CapabilityCheckResult, CapabilityCheckStatus, + CheckFindingKey, ConnectorChecksStatus, DraftCheckPlan, DraftCheckRunSnapshot, DraftCheckState, DraftCheckStateKind, DraftRerunMode, + FindingOption, } from "@/lib/connectors/checks/types"; import type { ConfigurableSources } from "@/lib/connectors/types/source"; import type { AccessType } from "@/lib/types"; @@ -114,6 +124,7 @@ function toCheckResult( remediation: check.remediation, docs_link: check.docs_link, duration_ms: check.duration_ms, + findings: check.findings, }; } @@ -510,6 +521,37 @@ export function useResetChecksOnCredentialChange({ }, [stale, staleRunId, credentialKey, session?.request, sessionKey, mutate]); } +/** Options a discovery check listed under `key`: the form's latest run, else the stored reports in `scope`. Starts no run. */ +export function useFindingOptions( + source: ConfigurableSources, + key: CheckFindingKey, + scope: StoredReportScope | null +): FindingOption[] { + const { data: session } = useSWR( + SWR_KEYS.connectorCheckSession(source), + null + ); + const runId: string | null = session?.runId ?? null; + const { data: run } = useSWR( + runId ? SWR_KEYS.connectorCheckRun(runId) : null, + null + ); + const { reportFor } = useCredentialCheckReports(source); + const { savedDraft } = useSavedDraftCredential(source); + // The form's account: a saved credential, else the draft a run saved the + // typed account as. A run of another account is stale until the reset. + const accountId: number | null = + scope?.credentialId ?? savedDraft?.credential_id ?? null; + return useMemo(() => { + const fromRun: FindingOption[] = + run && run.credential_id === accountId + ? findingOptions(run.checks, key) + : []; + if (fromRun.length > 0 || scope === null) return fromRun; + return findingOptions(storedCheckResults(reportFor, scope), key); + }, [run, accountId, key, scope, reportFor]); +} + const PLAN_DEBOUNCE_MS = 300; /** `value`, once it has held still for `delayMs`. */ diff --git a/web/src/lib/connectors/checks/types.ts b/web/src/lib/connectors/checks/types.ts index cd42f5705e..8881343e04 100644 --- a/web/src/lib/connectors/checks/types.ts +++ b/web/src/lib/connectors/checks/types.ts @@ -24,6 +24,45 @@ export type CapabilityCheckStatus = | "indeterminate" | "skipped"; +/** What a discovery check lists, named by `CheckFindingKey` on the backend. */ +export type CheckFindingKey = + | "shared_drives" + | "visible_organization_users" + | "visible_spaces" + | "readable_channels" + | "readable_mailboxes" + | "readable_drives"; + +/** One choice a listing offers a config field. */ +export interface FindingOption { + /** What the config field stores: an id or a URL. */ + value: string; + /** What the admin reads. */ + label: string; + detail: string | null; +} + +export interface OptionsFinding { + type: "options"; + key: CheckFindingKey; + /** The config field the options are for. */ + field_name: string | null; + options: FindingOption[]; + /** The listing stopped at the backend's option cap. */ + truncated: boolean; +} + +export interface CountFinding { + type: "count"; + key: CheckFindingKey; + field_name: string | null; + count: number; + /** The listing stopped at a cap, so the real count is at least `count`. */ + at_least: boolean; +} + +export type CheckFinding = OptionsFinding | CountFinding; + /** One finished check, as the checks card shows it. */ export interface CapabilityCheckResult { capability: CredentialCapability; @@ -37,6 +76,8 @@ export interface CapabilityCheckResult { remediation: string | null; docs_link: string | null; duration_ms: number | null; + /** What a passing discovery check listed. Empty for every other check. */ + findings: CheckFinding[]; } /** A check's state in a draft run. */ @@ -90,6 +131,7 @@ export interface DraftCheckState { from_cache: boolean; /** The check proves the credential works with the credential-bound fields. */ validates_binding: boolean; + findings: CheckFinding[]; } export interface DraftCheckRunSnapshot { diff --git a/web/src/lib/connectors/checks/useFindingOptions.test.tsx b/web/src/lib/connectors/checks/useFindingOptions.test.tsx new file mode 100644 index 0000000000..c6d5330f03 --- /dev/null +++ b/web/src/lib/connectors/checks/useFindingOptions.test.tsx @@ -0,0 +1,151 @@ +/** The listing a picker offers follows the form's account: the account's own + * run first, else its stored report, never another account's run. */ +import { act, renderHook } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { SWRConfig, useSWRConfig } from "swr"; +import { useFindingOptions } from "@/lib/connectors/checks/hooks"; +import type { StoredReportScope } from "@/lib/connectors/checks/findings"; +import type { + CapabilityCheckResult, + CheckFinding, + DraftCheckRunSnapshot, + DraftCheckState, +} from "@/lib/connectors/checks/types"; +import { ValidSources } from "@/lib/connectors/types/source"; +import type { CredentialCheckReport } from "@/lib/credentials/types"; +import { SWR_KEYS } from "@/lib/swr-keys"; + +const SOURCE = ValidSources.Slack; + +function listing(label: string): CheckFinding { + return { + type: "options", + key: "readable_channels", + field_name: "channels", + options: [{ value: "C1", label, detail: null }], + truncated: false, + }; +} + +function checkState(findings: CheckFinding[]): DraftCheckState { + return { + check_id: "listing", + display_name: "Listing", + capability: "indexing", + required: false, + state: "passed", + message: "", + missing_fields: [], + invalid_fields: [], + remediation: null, + docs_link: null, + duration_ms: 1, + from_cache: false, + validates_binding: false, + findings, + }; +} + +function checkResult(findings: CheckFinding[]): CapabilityCheckResult { + return { + capability: "indexing", + check_id: "listing", + display_name: "Listing", + required: false, + status: "passed", + message: "", + remediation: null, + docs_link: null, + duration_ms: 1, + findings, + }; +} + +function report(credentialId: number, label: string): CredentialCheckReport { + return { + credential_id: credentialId, + connector_id: null, + run_status: "completed", + run_started_at: null, + report: { + checked_at: "2026-10-13T00:00:00Z", + check_results: [checkResult([listing(label)])], + }, + time_updated: "2026-10-13T00:00:00Z", + }; +} + +const REPORTS: CredentialCheckReport[] = [ + report(1, "from-report-1"), + report(2, "from-report-2"), +]; + +jest.mock("@/lib/credentials/hooks", () => ({ + useCredentialCheckReports: () => ({ + reportFor: (credentialId: number, connectorId: number | null = null) => + REPORTS.find( + (candidate) => + candidate.credential_id === credentialId && + candidate.connector_id === connectorId + ) ?? null, + rerun: async () => undefined, + }), + useSavedDraftCredential: () => ({ + savedDraft: null, + rememberDraft: async () => undefined, + }), +})); + +const RUN: DraftCheckRunSnapshot = { + run_id: "run-1", + draft_key: "draft", + source: SOURCE, + credential_id: 1, + access_type: "public", + status: "completed", + form_errors: {}, + unknown_fields: [], + checks: [checkState([listing("from-run-1")])], +}; + +function wrapper({ children }: { children: ReactNode }) { + return ( + new Map() }}>{children} + ); +} + +function scopeFor(credentialId: number): StoredReportScope { + return { credentialId, connectorId: null }; +} + +test("the account's run wins, and another account's run is ignored", async () => { + const { result, rerender } = renderHook( + (props: { scope: StoredReportScope }) => ({ + labels: useFindingOptions(SOURCE, "readable_channels", props.scope).map( + (option) => option.label + ), + swr: useSWRConfig(), + }), + { initialProps: { scope: scopeFor(1) }, wrapper } + ); + expect(result.current.labels).toEqual(["from-report-1"]); + + // A run of account 1 seeds the session and run keys as a check start does. + await act(async () => { + await result.current.swr.mutate( + SWR_KEYS.connectorCheckSession(SOURCE), + { runId: RUN.run_id }, + { revalidate: false } + ); + await result.current.swr.mutate( + SWR_KEYS.connectorCheckRun(RUN.run_id), + RUN, + { revalidate: false } + ); + }); + expect(result.current.labels).toEqual(["from-run-1"]); + + // Switching to account 2 before the session resets reads its own report. + rerender({ scope: scopeFor(2) }); + expect(result.current.labels).toEqual(["from-report-2"]); +}); diff --git a/web/src/lib/connectors/connectorConfigFields.test.tsx b/web/src/lib/connectors/connectorConfigFields.test.tsx index 3dbabe0181..618eab9a5f 100644 --- a/web/src/lib/connectors/connectorConfigFields.test.tsx +++ b/web/src/lib/connectors/connectorConfigFields.test.tsx @@ -26,6 +26,11 @@ function renderDriveFields(currentCredential: Credential | null) { values={values} connector={ValidSources.GoogleDrive} currentCredential={currentCredential} + reportScope={ + currentCredential + ? { credentialId: currentCredential.id, connectorId: null } + : null + } /> ); diff --git a/web/src/lib/connectors/connectors.tsx b/web/src/lib/connectors/connectors.tsx index 07e66a4fa3..2572a32149 100644 --- a/web/src/lib/connectors/connectors.tsx +++ b/web/src/lib/connectors/connectors.tsx @@ -1484,10 +1484,10 @@ export const connectorConfigs: Record< query: "Enter channels to include:", label: "Channels", name: "channels", - description: `Specify 0 or more channels to index. For example, specifying the channel "support" will cause us to only index all content within the "#support" channel. If no channels are specified, all channels in your workspace will be indexed.`, + description: `Specify 0 or more channels to index. Pick a channel from the list to follow it by id, so a rename does not break indexing, or type the name of a channel the list does not show, such as a private channel the bot was invited to. If no channels are specified, all channels in your workspace will be indexed.`, optional: true, - // Slack Channels can only be lowercase - transform: (values) => values.map((value) => value.toLowerCase()), + optionsFinding: "readable_channels", + plainListCondition: (values) => values.channel_regex_enabled === true, }, { type: "checkbox", @@ -1506,8 +1506,9 @@ For example, specifying \`.*-support.*\` as a "channel" will cause the connector name: "exclude_channels", description: `Specify 0 or more channels to exclude. Exclusions are applied after the "Channels" filter above, so a channel matched by both is excluded. If no channels are specified, nothing is excluded.`, optional: true, - // Slack Channels can only be lowercase - transform: (values) => values.map((value) => value.toLowerCase()), + optionsFinding: "readable_channels", + plainListCondition: (values) => + values.exclude_channel_regex_enabled === true, }, { type: "checkbox", diff --git a/web/src/lib/connectors/types/form.ts b/web/src/lib/connectors/types/form.ts index 4e9b7a7a38..f083defee3 100644 --- a/web/src/lib/connectors/types/form.ts +++ b/web/src/lib/connectors/types/form.ts @@ -1,4 +1,5 @@ /** The schema a connector's creation form is rendered from. */ +import type { CheckFindingKey } from "@/lib/connectors/checks/types"; import type { CredentialValues } from "@/lib/credentials/types"; export type InputType = @@ -51,6 +52,10 @@ export interface ListOption extends Option { type: "list"; default?: string[]; transform?: (values: string[]) => string[]; + /** Discovery finding whose options the field offers: stores the value, shows the label. Typed entries stay allowed. */ + optionsFinding?: CheckFindingKey; + /** When true the field takes typed entries only, such as regex patterns. */ + plainListCondition?: (values: Record) => boolean; } export interface StringPairListOption extends Option { diff --git a/web/src/lib/connectors/zoomConnectorConfig.test.tsx b/web/src/lib/connectors/zoomConnectorConfig.test.tsx index b2d476a860..62e0437998 100644 --- a/web/src/lib/connectors/zoomConnectorConfig.test.tsx +++ b/web/src/lib/connectors/zoomConnectorConfig.test.tsx @@ -32,6 +32,7 @@ function ZoomForm() { values={values} connector={ValidSources.Zoom} currentCredential={null} + reportScope={null} /> ))} diff --git a/web/src/lib/credentials/hooks.ts b/web/src/lib/credentials/hooks.ts index 44958171c6..fb4b23d503 100644 --- a/web/src/lib/credentials/hooks.ts +++ b/web/src/lib/credentials/hooks.ts @@ -406,8 +406,11 @@ const CREDENTIAL_CHECK_POLL_MS = 2000; /** What `useCredentialCheckReports` returns. */ export interface CredentialCheckReports { - /** A credential's own report (not a connector's), or null if it has none. */ - reportFor: (credentialId: number) => CredentialCheckReport | null; + /** The credential's report for `connectorId`, or its own for null. */ + reportFor: ( + credentialId: number, + connectorId?: number | null + ) => CredentialCheckReport | null; /** Starts the checks on one credential; resolves once the run is queued. */ rerun: (credentialId: number) => Promise; } @@ -431,12 +434,18 @@ export function useCredentialCheckReports( } ); - return { - reportFor: (credentialId) => + // Stable per load, so a consumer can memoize on it. + const reportFor = useCallback( + (credentialId: number, connectorId: number | null = null) => data?.find( (report) => - report.credential_id === credentialId && report.connector_id === null + report.credential_id === credentialId && + report.connector_id === connectorId ) ?? null, + [data] + ); + return { + reportFor, rerun: async (credentialId) => { await startCredentialCheckRun(credentialId); await refresh(); diff --git a/web/src/lib/credentials/types/credential.ts b/web/src/lib/credentials/types/credential.ts index a0a12a6760..70868a782b 100644 --- a/web/src/lib/credentials/types/credential.ts +++ b/web/src/lib/credentials/types/credential.ts @@ -1,4 +1,5 @@ import type { SWRResponse } from "swr"; +import type { CapabilityCheckResult } from "@/lib/connectors/checks/types"; import type { CredentialSpec } from "@/lib/credentials/types/spec"; import type { ValidSources } from "@/lib/connectors/types/source"; import type { TypedFile } from "@/lib/connectors/fileTypes"; @@ -176,7 +177,10 @@ export interface CredentialCheckReport { connector_id: number | null; run_status: CredentialCheckRunStatus; run_started_at: string | null; - report: { checked_at: string } | null; + report: { + checked_at: string; + check_results: CapabilityCheckResult[]; + } | null; time_updated: string; } diff --git a/web/src/views/admin/connectors/AddConnectorPage/components/AddConnectorWrapper.tsx b/web/src/views/admin/connectors/AddConnectorPage/components/AddConnectorWrapper.tsx index c96af494d7..3e1d1ba4fa 100644 --- a/web/src/views/admin/connectors/AddConnectorPage/components/AddConnectorWrapper.tsx +++ b/web/src/views/admin/connectors/AddConnectorPage/components/AddConnectorWrapper.tsx @@ -707,6 +707,10 @@ export function AddConnectorForm({ connector, edit }: AddConnectorFormProps) { {(formikProps) => { const busy = uploading || creatingConnector || planning; const formCredential = accountFor(formikProps.values); + const formCredentialId: number | null = + formCredential !== null && !isDraftCredential(formCredential) + ? formCredential.id + : null; const canCreate = noCredentials || formCredential !== null; const newAccountReady = typedDraft(connector, accountSchema, formikProps.values) !== null; @@ -730,10 +734,7 @@ export function AddConnectorForm({ connector, edit }: AddConnectorFormProps) { const credentialSwapped: boolean = edit !== undefined && formCredential !== null && - !( - "id" in formCredential && - formCredential.id === edit.ccPair.credential.id - ); + formCredentialId !== edit.ccPair.credential.id; return ( {edit && editsFileList && ( diff --git a/web/src/views/admin/connectors/AddConnectorPage/form/ConnectorConfigFields.tsx b/web/src/views/admin/connectors/AddConnectorPage/form/ConnectorConfigFields.tsx index 5073088cbc..9e75e2ed70 100644 --- a/web/src/views/admin/connectors/AddConnectorPage/form/ConnectorConfigFields.tsx +++ b/web/src/views/admin/connectors/AddConnectorPage/form/ConnectorConfigFields.tsx @@ -3,6 +3,7 @@ import CredentialSubText from "@/lib/credentials/components/CredentialFields"; import type { ConnectionConfiguration } from "@/lib/connectors/types"; import type { ConfigurableSources } from "@/lib/connectors/types/source"; import type { CredentialValues } from "@/lib/credentials/types"; +import type { StoredReportScope } from "@/lib/connectors/checks/findings"; import { RenderField } from "./FieldRendering"; import { useFormikContext } from "formik"; @@ -13,6 +14,8 @@ export interface ConnectorConfigFieldsProps { values: any; connector: ConfigurableSources; currentCredential: CredentialValues | null; + /** The stored check reports that can list a field's options. */ + reportScope: StoredReportScope | null; } export default function ConnectorConfigFields({ @@ -20,6 +23,7 @@ export default function ConnectorConfigFields({ values, connector, currentCredential, + reportScope, }: ConnectorConfigFieldsProps) { const { setFieldValue } = useFormikContext(); // Get Formik's context functions @@ -68,6 +72,7 @@ export default function ConnectorConfigFields({ values={values} connector={connector} currentCredential={currentCredential} + reportScope={reportScope} /> ))} diff --git a/web/src/views/admin/connectors/AddConnectorPage/form/CredentialBoundFields.tsx b/web/src/views/admin/connectors/AddConnectorPage/form/CredentialBoundFields.tsx index 9d1006fe68..b7609ef038 100644 --- a/web/src/views/admin/connectors/AddConnectorPage/form/CredentialBoundFields.tsx +++ b/web/src/views/admin/connectors/AddConnectorPage/form/CredentialBoundFields.tsx @@ -61,6 +61,8 @@ export default function CredentialBoundFields({ values={values} connector={connector} currentCredential={currentCredential} + // A bound field is read before the checks run, so no listing. + reportScope={null} /> {error && ( diff --git a/web/src/views/admin/connectors/AddConnectorPage/form/FieldRendering.tsx b/web/src/views/admin/connectors/AddConnectorPage/form/FieldRendering.tsx index 6b3ec18ee3..7d11dae519 100644 --- a/web/src/views/admin/connectors/AddConnectorPage/form/FieldRendering.tsx +++ b/web/src/views/admin/connectors/AddConnectorPage/form/FieldRendering.tsx @@ -26,6 +26,8 @@ import FileDropzoneField from "@/refresh-components/form/FileDropzoneField"; import { FormikField } from "@/refresh-components/form/FormikField"; import InputTextAreaField from "@/refresh-components/form/InputTextAreaField"; import InputTypeInField from "@/refresh-components/form/InputTypeInField"; +import FindingListField from "@/views/admin/connectors/AddConnectorPage/form/FindingListField"; +import type { StoredReportScope } from "@/lib/connectors/checks/findings"; import { getSourceDisplayName } from "@/lib/sources"; import { markdown } from "@opal/utils"; @@ -37,6 +39,7 @@ interface TabsFieldProps { values: any; connector: ConfigurableSources; currentCredential: CredentialValues | null; + reportScope: StoredReportScope | null; } const TabsField: FC = ({ @@ -44,6 +47,7 @@ const TabsField: FC = ({ values, connector, currentCredential, + reportScope, }) => { const t = useTranslations("admin.connectorsList"); const { setFieldTouched, setFieldValue } = useFormikContext(); @@ -126,6 +130,7 @@ const TabsField: FC = ({ values={values} connector={connector} currentCredential={currentCredential} + reportScope={reportScope} /> ); })} @@ -257,6 +262,8 @@ interface RenderFieldProps { values: any; connector: ConfigurableSources; currentCredential: CredentialValues | null; + /** The stored check reports that can list a field's options. */ + reportScope: StoredReportScope | null; } export const RenderField: FC = ({ @@ -264,6 +271,7 @@ export const RenderField: FC = ({ values, connector, currentCredential, + reportScope, }) => { const t = useTranslations("admin.connectorsList"); const { setFieldValue } = useFormikContext(); // Get Formik's context functions @@ -315,6 +323,7 @@ export const RenderField: FC = ({ values={values} connector={connector} currentCredential={currentCredential} + reportScope={reportScope} /> ); } @@ -345,13 +354,30 @@ export const RenderField: FC = ({ subDescription={richDescription} suffix={optionalSuffix} > - + {field.optionsFinding !== undefined ? ( + + ) : ( + + )} ) : field.type === "string_pair_list" ? ( (name); + const options = useMemo( + () => + findingOptions.map((option) => ({ + value: option.value, + title: option.label, + description: option.detail ?? undefined, + // A search by the stored value finds the row too. + keywords: [option.value], + })), + [findingOptions] + ); + + // A plain list takes names, so a picked id becomes its label. + useEffect(() => { + if (!plain) return; + const labels = new Map( + findingOptions.map((option) => [option.value, option.label]) + ); + const entries: string[] = field.value ?? []; + if (!entries.some((entry) => labels.has(entry))) return; + void helpers.setValue(entries.map((entry) => labels.get(entry) ?? entry)); + }, [plain, findingOptions, field.value, helpers]); + + if (plain) { + return ( + + ); + } + return ( + + ); +} diff --git a/web/src/views/admin/connectors/AddConnectorPage/sections/ConnectorContentSection.tsx b/web/src/views/admin/connectors/AddConnectorPage/sections/ConnectorContentSection.tsx index a9429502f5..82effdd2b1 100644 --- a/web/src/views/admin/connectors/AddConnectorPage/sections/ConnectorContentSection.tsx +++ b/web/src/views/admin/connectors/AddConnectorPage/sections/ConnectorContentSection.tsx @@ -6,6 +6,7 @@ import ConnectorConfigFields from "@/views/admin/connectors/AddConnectorPage/for import type { ConnectionConfiguration } from "@/lib/connectors/types"; import type { ConfigurableSources } from "@/lib/connectors/types/source"; import type { CredentialValues } from "@/lib/credentials/types"; +import type { StoredReportScope } from "@/lib/connectors/checks/findings"; interface ConnectorContentSectionProps { /** The connector's fields that are not bound to the credential. */ @@ -13,6 +14,8 @@ interface ConnectorContentSectionProps { values: Record; connector: ConfigurableSources; currentCredential: CredentialValues | null; + /** The stored check reports that can list a field's options. */ + reportScope: StoredReportScope | null; /** Freezes the section while the page locks the configuration. */ disabled?: boolean; /** Extra content below the fields, such as an edit's file list. */ @@ -25,6 +28,7 @@ export default function ConnectorContentSection({ values, connector, currentCredential, + reportScope, disabled, children, }: ConnectorContentSectionProps) { @@ -43,6 +47,7 @@ export default function ConnectorContentSection({ config={config} connector={connector} currentCredential={currentCredential} + reportScope={reportScope} /> {children} diff --git a/web/src/views/admin/connectors/ConnectorDetailPage/ConfigDisplay.tsx b/web/src/views/admin/connectors/ConnectorDetailPage/ConfigDisplay.tsx index 2a13a9c799..9e4a16cbb1 100644 --- a/web/src/views/admin/connectors/ConnectorDetailPage/ConfigDisplay.tsx +++ b/web/src/views/admin/connectors/ConnectorDetailPage/ConfigDisplay.tsx @@ -1,9 +1,16 @@ "use client"; -import { useState } from "react"; +import { useMemo, useState } from "react"; import { useLocale, useTranslations } from "next-intl"; import { ValidSources } from "@/lib/connectors/types/source"; +import { connectorConfigs } from "@/lib/connectors/connectors"; +import { isEditableSource } from "@/lib/connectors/edit/editable"; +import { + labelFindingValues, + storedCheckResults, +} from "@/lib/connectors/checks/findings"; +import { useCredentialCheckReports } from "@/lib/credentials/hooks"; import { Section } from "@/layouts/general-layouts"; import Text from "@/refresh-components/texts/Text"; import { Button, Divider } from "@opal/components"; @@ -27,7 +34,7 @@ function convertObjectToString(obj: any): string | any { return obj; } -export function buildConfigEntries( +function buildConfigEntries( obj: any, sourceType: ValidSources ): { [key: string]: string } { @@ -246,7 +253,32 @@ export function AdvancedConfigDisplay({ ); } -export function ConfigDisplay({ +interface ConnectorConfigDisplayProps { + source: ValidSources; + config: Record; + credentialId: number; + connectorId: number; +} + +/** A pair's config with listing-backed entries shown by name. The pair's report is read before the credential's. */ +export function ConnectorConfigDisplay({ + source, + config, + credentialId, + connectorId, +}: ConnectorConfigDisplayProps) { + const { reportFor } = useCredentialCheckReports(source); + const configEntries = useMemo(() => { + const checks = storedCheckResults(reportFor, { credentialId, connectorId }); + const labeled = isEditableSource(source) + ? labelFindingValues(connectorConfigs[source], config, checks) + : config; + return buildConfigEntries(labeled, source); + }, [source, config, credentialId, connectorId, reportFor]); + return ; +} + +function ConfigDisplay({ configEntries, onEdit, }: { diff --git a/web/src/views/admin/connectors/ConnectorDetailPage/index.tsx b/web/src/views/admin/connectors/ConnectorDetailPage/index.tsx index 489157b260..2960ce1b6c 100644 --- a/web/src/views/admin/connectors/ConnectorDetailPage/index.tsx +++ b/web/src/views/admin/connectors/ConnectorDetailPage/index.tsx @@ -20,11 +20,7 @@ import { useRouter } from "next/navigation"; import { useLocale, useTranslations } from "next-intl"; import { useCallback, useEffect, useRef, useState, use } from "react"; import useSWR, { mutate } from "swr"; -import { - AdvancedConfigDisplay, - buildConfigEntries, - ConfigDisplay, -} from "./ConfigDisplay"; +import { AdvancedConfigDisplay, ConnectorConfigDisplay } from "./ConfigDisplay"; import DeletionErrorStatus from "./DeletionErrorStatus"; import { IndexAttemptsTable } from "./IndexAttemptsTable"; import InlineFileManagement from "./InlineFileManagement"; @@ -734,11 +730,11 @@ function Main({ ccPairId }: { ccPairId: number }) { - {/* Inline file management for file connectors */} diff --git a/web/tests/e2e/utils/connectorChecks.ts b/web/tests/e2e/utils/connectorChecks.ts index b151cb0380..9076d676c3 100644 --- a/web/tests/e2e/utils/connectorChecks.ts +++ b/web/tests/e2e/utils/connectorChecks.ts @@ -30,6 +30,7 @@ function signInCheck(state: DraftCheckState["state"]): DraftCheckState { duration_ms: state === "passed" ? 1 : null, from_cache: false, validates_binding: true, + findings: [], }; }