Skip to content

Plugin group config in group requests#502

Draft
barborico wants to merge 11 commits into
mainfrom
brynna/plugin_group_config_in_requests
Draft

Plugin group config in group requests#502
barborico wants to merge 11 commits into
mainfrom
brynna/plugin_group_config_in_requests

Conversation

@barborico

Copy link
Copy Markdown
Contributor

Summary

Lets users supply (and resolvers edit) app-group-lifecycle plugin group config when requesting an app group, validates it at submit and approval, and applies it to the group created on approval so the plugin's group_created hook runs against a configured group.

Urgency: 5 days
Expected review effort: MEDIUM

Motivation

App-group-lifecycle plugins can require per-group config (e.g. the Google Group plugin needs an email prefix + display name). Today that config is only collectable in the direct group-create/edit flow. When a user requests a group for such an app, there is nowhere to supply it, so approval creates the group unconfigured and the group_created hook runs against an empty config. This adds a generic "plugin-config-in-requests" capability (not specific to any one plugin).

Description of Changes

  • Model + migration: two JSON columns on GroupRequestrequested_plugin_data and resolved_plugin_data — mirroring the existing requested_*/resolved_* pairs and the sibling *_group_tags columns (JSONB on Postgres, server_default "{}"). Config is stored in the same shape as a group's plugin_data ({plugin_id: {configuration: {...}}}); no status is stored (the hook populates that post-creation).
  • Schemas: requested_plugin_data on the app-group create body; both fields on GroupRequestDetail; resolved_plugin_data on ResolveGroupRequestBody.
  • Create: CreateGroupRequest persists requested_plugin_data; the POST router validates it against the app's plugin (400 on invalid/missing-required).
  • Approve: ApproveGroupRequest resolves resolved_plugin_data or requested_plugin_data, re-validates, and sets it on the new AppGroup before CreateGroup runs, so the group_created hook sees the configured group. Invalid config surfaces as a 400 via the PUT handler.
  • Resolve (PUT): copies resolved_plugin_data from the resolve body so a resolver can edit before approving.
  • Frontend: Create.tsx renders the existing AppGroupLifecyclePluginConfigurationForm for app-group requests whose app has a plugin (fetching AppDetail via useAppById since search summaries omit the plugin id) and submits requested_plugin_data; Read.tsx lets the resolver edit and submits resolved_plugin_data. Small DOM-free helpers (pluginConfig.ts) with unit tests.
  • Generated client: the plugin_data fields were hand-added to src/api/apiSchemas.ts. A full npm run codegen in this environment renames the paginated generics (PageTypeVarCustomized*PageTCustomized*) across the entire client, which would bury this change under hundreds of unrelated lines; the drift-check workflow sanctions hand-editing the generated files.
  • Also fixes a pre-existing one-liner in src/setupTests.ts (jest-dom v6 dropped the /extend-expect subpath), which is required for the frontend test suite to run.

Validation of Changes

  • Backend: tests/test_group_request_plugin_config.py covers column defaults/round-trip, schema parsing, CreateGroupRequest persistence, POST 400-on-invalid + 201-on-valid, approval applying resolved-over-requested onto the created group (asserting the group_created hook fired against it) + fallback-to-requested, and the PUT resolve path end-to-end. Full relevant suite: 134 passed (test_group_request, test_group_request_plugin_config, test_app_group_lifecycle_plugin, test_request_models).
  • Migration verified up + down on a scratch SQLite DB.
  • Frontend: npx tsc --noEmit clean; pluginConfig.test.ts 5/5.
  • Manual UI check: confirm the resolver form in Read.tsx pre-fills with the requester's submitted config (the form seeds via uncontrolled defaultValue).

Guidance for Reviewers

Review commit-by-commit — the history is linear and each commit is one logical layer (model → migration → schemas → create op → POST validation → approve op → PUT → client → Create form → Read form). Most worth a look: ApproveGroupRequest setting new_group.plugin_data before CreateGroup.execute() (so the hook sees it), and the hand-edited TS client (see the codegen-churn rationale in that commit).

🤖 Generated with Claude Code

barborico and others added 11 commits June 27, 2026 06:06
Store requester- and resolver-supplied app-group-lifecycle plugin
config on the group request, mirroring the requested_/resolved_ pair
used for every other field, so plugin config can be collected at
request time and applied when the group is created on approval.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds requested_plugin_data to _AppGroupRequestBody (only app groups
carry a lifecycle plugin) and GroupRequestDetail, and resolved_plugin_data
to ResolveGroupRequestBody and GroupRequestDetail. Both fields default to
empty dict / None to remain backwards-compatible with existing callers.
Both Any and Optional were already imported; no import changes needed.

Tests added for accept/default behavior on create and resolve bodies.
The test inputs include requested_group_description to satisfy the
REQUIRE_DESCRIPTIONS=true env constraint present in the test environment.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…uest

Adds requested_plugin_data as an optional kwarg to CreateGroupRequest.__init__
and threads it through to the GroupRequest constructor so the value is
written to the new DB column. Defaults to {} when not supplied, consistent
with the model-level default.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Reject app-group requests whose plugin group config is missing or
invalid (400) by running the app's configured lifecycle plugin's
group-config validation at request time, so a request can't be filed
with config the plugin would reject. Thread requested_plugin_data into
CreateGroupRequest and the response.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve resolved_plugin_data or requested_plugin_data, re-validate it
against the resolved app's lifecycle plugin, and set it on the AppGroup
before CreateGroup so the group_created hook runs against a configured
group. Invalid config surfaces as a 400 via the PUT handler.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy resolved_plugin_data from the resolve body onto the group request
before approval so a resolver's plugin-config edits are applied to the
group that approval creates.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add requested_plugin_data/resolved_plugin_data to GroupRequestDetail,
requested_plugin_data to the app-group create body, and
resolved_plugin_data to ResolveGroupRequestBody in the generated client.

Hand-edited rather than via `npm run codegen`: the @openapi-codegen
version resolved in this environment renames the paginated generics
(PageTypeVarCustomized* -> PageTCustomized*) across the entire client,
which would bury this change under hundreds of lines of unrelated
churn. The drift-check workflow explicitly supports hand-editing the
generated files; these additions match the spec the backend now emits.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Render AppGroupLifecyclePluginConfigurationForm for app-group requests
whose app has a lifecycle plugin (fetching the app detail to read the
plugin id, since search summaries omit it), and submit the collected
config as requested_plugin_data. Adds DOM-free helpers with unit tests.

Also fixes setupTests.ts to import @testing-library/jest-dom directly
(the /extend-expect sub-path was removed in a newer version), which
unblocked running vitest for DOM-free test files.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Seed the resolve view with the requested plugin config, allow editing,
and submit it as resolved_plugin_data on approval.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wrap a long mocker.patch.object line to the configured width and add
explicit TypeAdapter[Any] annotations the TypeAdapter() calls so mypy
no longer needs a var annotation. No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant