Flagsmith / Flagsmith/flagsmith
Consolidate VCS integration orchestration
- Dominant language
- Python
- Stars
- 6.6k
- Forks
- 567
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 124
Description
The same logical events (link, unlink, feature state changed, feature deleted) dispatch per-provider from multiple unrelated sites today. Adding a new VCS (Bitbucket, say) means touching all of these files and re-learning the conventions each time.
## Current state
After #7306 and #7327, `integrations/vcs/services.py` hosts:
- `dispatch_vcs_on_resource_create(resource)`
- `dispatch_vcs_on_resource_destroy(resource)`
`FeatureExternalResourceViewSet.perform_create` and `.perform_destroy` delegate to these. GitLab side effects (log, webhook register/deregister, initial tag, tag clear, link/unlink comment) all flow through the dispatcher via namespaced calls (`from integrations.gitlab import services as gitlab`), and the GitLab-specific `apply_gitlab_tag` / `deregister_gitlab_webhook` lifecycle hooks on `FeatureExternalResource` are gone.
## Still to do
View-level, outside the dispatcher:
- `FeatureExternalResourceViewSet.create` still owns the GitHub URL preflight regex and inline `label_github_issue_pr` call.
Serializer-level:
- State change: `FeatureStateSerializerBasic.save` calls `call_github_task` (GitHub) and `post_gitlab_state_change_comment_for_feature_state` (GitLab) directly. No `dispatch_vcs_on_feature_state_changed` yet.
- `features/versioning/serializers.py` — same split for the v2 versioning flow.
Model-level hooks still present — consolidate to single VCS hook per model:
- `FeatureExternalResource.notify_github_on_link` (`AFTER_SAVE`, type is `GITHUB_ISSUE`/`GITHUB_PR`) — applies GitHub tag + link comment.
- `FeatureExternalResource.notify_github_on_unlink` (`BEFORE_DELETE`, type is `GITHUB_ISSUE`/`GITHUB_PR`) — unlink comment to GitHub.
- `Feature.create_github_comment` (`AFTER_SAVE`) — flag-deleted comment to GitHub.
- `Feature.create_gitlab_comment` (`AFTER_SAVE`) — flag-deleted comment to GitLab.
## Proposed shape for the rest
Extend `integrations/vcs/services.py` with:
- `dispatch_vcs_on_feature_state_changed(feature_state)`
- `dispatch_vcs_on_feature_deleted(feature_name, feature_id, project_id)`
Port each remaining trigger behind these:
- Move the four model-level hooks above into the dispatcher's create/destroy paths (for the FER hooks) and the feature-deleted dispatcher (for the `Feature` hooks). Model-level triggers should be a last resort; when a side effect is driven by a viewset action, the dispatcher called from the viewset is the honest place for it.
- Move the two serializer dispatch sites behind `dispatch_vcs_on_feature_state_changed`.
- Decide whether the GitHub URL preflight regex stays in the view or moves into a provider-owned validator hook called from `dispatch_vcs_on_resource_create`.
The current implementation uses explicit `if resource.type in GITLAB_RESOURCE_TYPES:` branches rather than a registry protocol (`AppConfig.ready()`). Simple, fine for two providers; worth revisiting if a third joins.
## Not in scope
- Emit-parity between GitHub and GitLab (state-change, delete comments for GitHub). Separate product call.
- Log-event consolidation — falls out of the dispatch model naturally.
Contributor guide
Research direction
Read integrations/vcs/services.py and trace FeatureExternalResourceViewSet.create/perform_create/perform_destroy, FeatureStateSerializerBasic.save, and features/versioning/serializers.py. Then inspect the listed FeatureExternalResource and Feature model hooks. Done means the remaining VCS triggers use the proposed dispatchers, the GitHub URL preflight has a decided home, and the stated out-of-scope behavior remains unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend, backend-api-design
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100