From 90239e1d955f9f8aa9b64a89f01de74d621803df Mon Sep 17 00:00:00 2001 From: Yotam Soudry Date: Thu, 30 Jul 2026 22:52:31 +0300 Subject: [PATCH 1/2] feat(jira): expose DevLake project name as ExtraJQL template variable Users often want to filter a shared Jira board per Devlake project using the devlake project name. The ExtraJQL field supports Go text/templates, but only BoardId and BoardName are available. Look up the Devlake project name from the project_mapping table at task setup time and expose it as {{.ProjectName}} in ExtraJQL templates. Example scope config ExtraJQL: component = {{.ProjectName}} --- backend/plugins/jira/impl/impl.go | 15 +++++++++++++++ backend/plugins/jira/tasks/issue_collector.go | 8 +++++--- backend/plugins/jira/tasks/task_data.go | 1 + 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/backend/plugins/jira/impl/impl.go b/backend/plugins/jira/impl/impl.go index b27ea9daa66..22d1612d690 100644 --- a/backend/plugins/jira/impl/impl.go +++ b/backend/plugins/jira/impl/impl.go @@ -25,6 +25,8 @@ import ( "github.com/apache/incubator-devlake/core/dal" "github.com/apache/incubator-devlake/core/errors" coreModels "github.com/apache/incubator-devlake/core/models" + "github.com/apache/incubator-devlake/core/models/domainlayer/crossdomain" + "github.com/apache/incubator-devlake/core/models/domainlayer/didgen" "github.com/apache/incubator-devlake/core/plugin" helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api" "github.com/apache/incubator-devlake/plugins/jira/api" @@ -257,6 +259,19 @@ func (p Jira) PrepareTaskData(taskCtx plugin.TaskContext, options map[string]int Board: scope, } + // Resolve the Devlake project name from the project_mapping table so it can + // be used as a template variable in ExtraJQL + if scope != nil { + rowId := didgen.NewDomainIdGenerator(&models.JiraBoard{}).Generate(scope.ConnectionId, scope.BoardId) + var mapping crossdomain.ProjectMapping + err = db.First(&mapping, dal.Where("`table` = ? AND `row_id` = ?", "boards", rowId)) + if err == nil { + taskData.ProjectName = mapping.ProjectName + } else if !db.IsErrorNotFound(err) { + logger.Error(err, "failed to load project mapping for board %s", rowId) + } + } + return taskData, nil } diff --git a/backend/plugins/jira/tasks/issue_collector.go b/backend/plugins/jira/tasks/issue_collector.go index fe1939b721b..5b2638dc74b 100644 --- a/backend/plugins/jira/tasks/issue_collector.go +++ b/backend/plugins/jira/tasks/issue_collector.go @@ -112,8 +112,9 @@ func CollectIssues(taskCtx plugin.SubTaskContext) errors.Error { // JqlTemplateData holds the variables available inside an ExtraJQL template. // Users reference these with Go template syntax, e.g. `{{.BoardName}}`. type JqlTemplateData struct { - BoardId uint64 // numeric ID of the connected Jira board - BoardName string // display name of the connected Jira board + BoardId uint64 // numeric ID of the connected Jira board + BoardName string // display name of the connected Jira board + ProjectName string // Devlake project name associated with the Jira board scope } // renderExtraJQL executes the ExtraJQL scope-config field as a Go text/template, @@ -133,7 +134,8 @@ func renderExtraJQL(tmplStr string, data *JiraTaskData) (string, errors.Error) { } vars := JqlTemplateData{ - BoardId: data.Options.BoardId, + BoardId: data.Options.BoardId, + ProjectName: data.ProjectName, } if data.Board != nil { vars.BoardName = data.Board.Name diff --git a/backend/plugins/jira/tasks/task_data.go b/backend/plugins/jira/tasks/task_data.go index 3505bf162e8..67044810e66 100644 --- a/backend/plugins/jira/tasks/task_data.go +++ b/backend/plugins/jira/tasks/task_data.go @@ -39,6 +39,7 @@ type JiraTaskData struct { JiraServerInfo models.JiraServerInfo FilterId string Board *models.JiraBoard + ProjectName string } type JiraApiParams models.JiraApiParams From 9b99a8d4c30a9f9fee5273a445bbe0ef83e57cde Mon Sep 17 00:00:00 2001 From: Yotam Soudry Date: Thu, 30 Jul 2026 23:35:30 +0300 Subject: [PATCH 2/2] fix(jira): resolve ExtraJQL ProjectName per-run instead of via ambiguous reverse lookup The original ProjectName resolution reverse-looked-up project_mapping by board, which can't disambiguate when the same board is attached to more than one Devlake project (project_mapping's PK is project_name+table+ row_id) -- exactly the scenario this feature is meant to support. Instead, GeneratePlanJsonV200 now injects "projectName" generically into every data-source task's options at blueprint-plan-build time, the one place in the framework where the running project and its scopes are known together unambiguously. Jira's PrepareTaskData just reads it back, removing the project_mapping/crossdomain/didgen lookup entirely. Also: gofmt the JqlTemplateData struct/literal, wire the previously-unused projectName param in Test_renderExtraJQL's test helper and add coverage for {{.ProjectName}}, add TestMakePlanV200InjectsProjectNameIntoDataSourceTaskOptions, and document {{.ProjectName}} in the config-ui ExtraJQL tooltip. Co-Authored-By: Claude Sonnet 5 --- CODE_REVIEW_NOTES.md | 137 ++++++++++++++++++ backend/plugins/jira/impl/impl.go | 19 +-- backend/plugins/jira/tasks/issue_collector.go | 8 +- .../jira/tasks/issue_collector_test.go | 19 ++- backend/plugins/jira/tasks/task_data.go | 7 + .../services/blueprint_makeplan_v200.go | 17 +++ .../services/blueprint_makeplan_v200_test.go | 43 ++++++ .../plugins/register/jira/transformation.tsx | 2 +- 8 files changed, 229 insertions(+), 23 deletions(-) create mode 100644 CODE_REVIEW_NOTES.md diff --git a/CODE_REVIEW_NOTES.md b/CODE_REVIEW_NOTES.md new file mode 100644 index 00000000000..8bb4370e24c --- /dev/null +++ b/CODE_REVIEW_NOTES.md @@ -0,0 +1,137 @@ +# Code Review Notes: jira ScopeConfigId fix / ExtraJQL ProjectName feature + +Review date: 2026-07-30. Scope: two branches based on `main` @ `14a4e5bcd`: + +- `fix/jira-blueprint-scope-config` (commit `1a07d1062`) +- `feature/expose-devlake-project-extrajql-variable` (commit `90239e1d9`) + +This file records what was found and fixed, so future agents don't have to +re-derive the same context from scratch. All fixes described below were +applied as uncommitted working-tree changes on their respective branch as of +this review; check `git log`/`git status` on each branch to see whether +they've since been committed. + +## fix/jira-blueprint-scope-config + +**What it does:** `makeDataSourcePipelinePlanV200` in +`backend/plugins/jira/api/blueprint_v200.go` now threads the resolved +`ScopeConfigId` into `JiraTaskOptions`, the way `ArgoCD`/`Linear` already do +in their own `blueprint_v200.go`. Previously Jira relied solely on a runtime +fallback in `impl.go`'s `PrepareTaskData` that re-looks-up the board by +`connection_id + board_id` and copies `scope.ScopeConfigId` if the task +option was zero. That fallback is fragile (e.g. it does nothing if +`op.BoardId == 0`, and is generally an indirect way to get a value that's +already known at plan-build time), so passing it explicitly is a real, +justified fix and matches established plugin convention. + +**Findings (fixed):** +1. `gofmt` failure — the new `ScopeConfigId` struct field broke alignment + of the `JiraTaskOptions{}` literal. The repo's `.golangci.yaml` enables + the `gofmt` formatter and CI runs `golangci-lint run`, so this would have + failed CI. Fixed by running `gofmt -w`. +2. Missing regression test — sibling plugins (`linear`, `bitbucket`, + `gitlab`, `azuredevops_go`) all have a `blueprint_v200_test.go`; Linear's + even has `TestMakePipelinePlanV200PassesScopeConfigId`, testing exactly + this scenario. Jira had no such test. Added + `backend/plugins/jira/api/blueprint_v200_test.go` modeled on Linear's, + covering: ScopeConfig.ID takes priority over scope.ScopeConfigId, and + scope.ScopeConfigId is used when ScopeConfig is nil/unconfigured. + +## feature/expose-devlake-project-extrajql-variable + +**What it does:** Exposes the DevLake *project* name (the logical grouping +of scopes across data sources) as `{{.ProjectName}}` inside the `ExtraJQL` +scope-config template, alongside the existing `{{.BoardName}}` / +`{{.BoardId}}`. + +**Important design change made during this review — read before touching +`ProjectName` resolution again:** + +The original implementation resolved `ProjectName` in `impl.go`'s +`PrepareTaskData` by reverse-looking-up `project_mapping WHERE +table='boards' AND row_id=`. This is broken for the +feature's actual stated purpose (per the requester): **the same Jira board +attached to multiple Devlake projects, each project filtering its own +tickets via ExtraJQL.** `project_mapping`'s primary key is +`(project_name, table, row_id)`, so a shared board has one row *per +project*, and the reverse lookup can't tell which project the *current +pipeline run* belongs to — it just grabs an arbitrary/first row. That means +every project sharing a board would get the same (wrong, for all but one of +them) `{{.ProjectName}}` value, silently. + +We looked at fixing this "properly" by threading `projectName` through +`plugin.DataSourcePluginBlueprintV200.MakeDataSourcePipelinePlanV200(...)`, +the interface every datasource plugin implements — but that's a framework +interface change with a large blast radius (~15+ plugins), and was +rejected as too big for this fix. + +**Actual fix implemented (small blast radius, no interface changes):** +`backend/server/services/blueprint_makeplan_v200.go`'s `GeneratePlanJsonV200` +already receives `projectName` as a parameter — it's the only place in the +framework where "this pipeline run belongs to project X" and "these are its +scopes" are known together unambiguously. After building `sourcePlans` (the +per-connection plans returned by each plugin's +`MakeDataSourcePipelinePlanV200`), it now generically injects +`task.Options["projectName"] = projectName` into every task, for every +plugin, when `projectName != ""`. This is safe because the mapstructure +decoding used everywhere (`Decode`/`DecodeMapStruct`) silently ignores +unknown map keys by default — plugins that don't declare a matching struct +field simply never see it. (Note: `dora`/`refdiff` already manually put a +`"projectName"` key into their own task options today, via +`MakeMetricPluginPipelinePlanV200`'s own `projectName` parameter — this +generic injection follows the same naming convention, just for +`DataSourcePluginBlueprintV200` plugins that don't get `projectName` on +their interface.) + +On the Jira side this actually *simplified* the implementation: +- `backend/plugins/jira/tasks/task_data.go`: `JiraOptions` gained a + `ProjectName string` field (mapstructure tag `projectName,omitempty`), + populated purely by decoding task options — no plugin-side logic needed. +- `backend/plugins/jira/impl/impl.go`: `PrepareTaskData` now just does + `ProjectName: op.ProjectName` when building `JiraTaskData`. The + `project_mapping`/`crossdomain`/`didgen` reverse-lookup code and its + imports were deleted entirely — there's no more ambiguity to handle. +- Test coverage: `backend/server/services/blueprint_makeplan_v200_test.go` + gained `TestMakePlanV200InjectsProjectNameIntoDataSourceTaskOptions`, + asserting the injected key lands in a data-source plugin's task options + alongside its own options, using `mock.Anything` for the "org" plugin's + `MapProject` call (re-registering "org" per-test to avoid cross-test mock + state leaking via the global `plugin` registry — see that file for why). + +**Other findings (fixed):** +1. `gofmt` failure — `JqlTemplateData` struct and the `JqlTemplateData{}` + literal in `issue_collector.go` used tabs for alignment instead of + spaces; new `ProjectName` field wasn't aligned either. Fixed with + `gofmt -w`. +2. Zero test coverage for the new variable — `issue_collector_test.go`'s + existing `Test_renderExtraJQL` test has a `makeData` helper whose third + parameter (`_ string`) was unused and looked purpose-built for exactly + this addition, but nothing wired it up. Fixed: the helper now sets + `ProjectName` from that parameter, and new subtests exercise + `{{.ProjectName}}` substitution (present and empty/unmapped board). This + part of the test is still valid after the `ProjectName`-resolution + redesign above, since it tests `renderExtraJQL` given an already-built + `JiraTaskData`, independent of how `ProjectName` gets populated upstream. +3. Config-UI help tooltip + (`config-ui/src/plugins/register/jira/transformation.tsx`) documented + only `{{.BoardName}}`/`{{.BoardId}}`. Updated to also mention + `{{.ProjectName}}` so the feature is discoverable from the UI. + +## Verification performed + +- `go build ./plugins/jira/...`, `./server/services/...` on both branches + (fix branch built in an isolated git worktree at the time of review). +- `go test ./plugins/jira/api/... ./plugins/jira/tasks/... ./plugins/jira/impl/... ./server/services/...` + on both branches. Pre-existing `plugins/jira/e2e` requires `E2E_DB_URL` + and is expected to fail/skip outside a real DB — unrelated to these + changes. `go build ./...` at the repo root also fails on unrelated + pre-existing issues in this sandbox (no `libgit2` for `gitextractor`; + `plugins/org` and other plugin packages are `main` packages meant to be + built with `-buildmode=plugin`, not as ordinary binaries) — neither is a + regression from these branches. +- `gofmt -l` on all touched files, before and after fixes, on both + branches. +- Generating `backend/mocks/{core,helpers}` via `mockery` was required to + run `server/services` tests locally (`make mock`, or the two `mockery + --recursive ...` commands in `backend/Makefile`); that directory is + gitignored and not checked in. diff --git a/backend/plugins/jira/impl/impl.go b/backend/plugins/jira/impl/impl.go index 22d1612d690..ac07653fd57 100644 --- a/backend/plugins/jira/impl/impl.go +++ b/backend/plugins/jira/impl/impl.go @@ -25,8 +25,6 @@ import ( "github.com/apache/incubator-devlake/core/dal" "github.com/apache/incubator-devlake/core/errors" coreModels "github.com/apache/incubator-devlake/core/models" - "github.com/apache/incubator-devlake/core/models/domainlayer/crossdomain" - "github.com/apache/incubator-devlake/core/models/domainlayer/didgen" "github.com/apache/incubator-devlake/core/plugin" helper "github.com/apache/incubator-devlake/helpers/pluginhelper/api" "github.com/apache/incubator-devlake/plugins/jira/api" @@ -257,19 +255,10 @@ func (p Jira) PrepareTaskData(taskCtx plugin.TaskContext, options map[string]int ApiClient: jiraApiClient, JiraServerInfo: *info, Board: scope, - } - - // Resolve the Devlake project name from the project_mapping table so it can - // be used as a template variable in ExtraJQL - if scope != nil { - rowId := didgen.NewDomainIdGenerator(&models.JiraBoard{}).Generate(scope.ConnectionId, scope.BoardId) - var mapping crossdomain.ProjectMapping - err = db.First(&mapping, dal.Where("`table` = ? AND `row_id` = ?", "boards", rowId)) - if err == nil { - taskData.ProjectName = mapping.ProjectName - } else if !db.IsErrorNotFound(err) { - logger.Error(err, "failed to load project mapping for board %s", rowId) - } + // op.ProjectName is injected by services.GeneratePlanJsonV200 at + // blueprint-plan-generation time, when the running Devlake project is + // known unambiguously. + ProjectName: op.ProjectName, } return taskData, nil diff --git a/backend/plugins/jira/tasks/issue_collector.go b/backend/plugins/jira/tasks/issue_collector.go index 5b2638dc74b..49d18904c5d 100644 --- a/backend/plugins/jira/tasks/issue_collector.go +++ b/backend/plugins/jira/tasks/issue_collector.go @@ -112,8 +112,8 @@ func CollectIssues(taskCtx plugin.SubTaskContext) errors.Error { // JqlTemplateData holds the variables available inside an ExtraJQL template. // Users reference these with Go template syntax, e.g. `{{.BoardName}}`. type JqlTemplateData struct { - BoardId uint64 // numeric ID of the connected Jira board - BoardName string // display name of the connected Jira board + BoardId uint64 // numeric ID of the connected Jira board + BoardName string // display name of the connected Jira board ProjectName string // Devlake project name associated with the Jira board scope } @@ -134,8 +134,8 @@ func renderExtraJQL(tmplStr string, data *JiraTaskData) (string, errors.Error) { } vars := JqlTemplateData{ - BoardId: data.Options.BoardId, - ProjectName: data.ProjectName, + BoardId: data.Options.BoardId, + ProjectName: data.ProjectName, } if data.Board != nil { vars.BoardName = data.Board.Name diff --git a/backend/plugins/jira/tasks/issue_collector_test.go b/backend/plugins/jira/tasks/issue_collector_test.go index 50c66947c59..501a7246e98 100644 --- a/backend/plugins/jira/tasks/issue_collector_test.go +++ b/backend/plugins/jira/tasks/issue_collector_test.go @@ -143,10 +143,11 @@ func Test_buildFilterJQL(t *testing.T) { } func Test_renderExtraJQL(t *testing.T) { - makeData := func(boardId uint64, boardName string, _ string) *JiraTaskData { + makeData := func(boardId uint64, boardName string, projectName string) *JiraTaskData { return &JiraTaskData{ - Options: &JiraOptions{BoardId: boardId}, - Board: &models.JiraBoard{BoardId: boardId, Name: boardName}, + Options: &JiraOptions{BoardId: boardId}, + Board: &models.JiraBoard{BoardId: boardId, Name: boardName}, + ProjectName: projectName, } } @@ -181,6 +182,18 @@ func Test_renderExtraJQL(t *testing.T) { data: &JiraTaskData{Options: &JiraOptions{BoardId: 1}, Board: nil}, want: `project = ""`, }, + { + name: "ProjectName substitution", + tmpl: `labels = "{{.ProjectName}}"`, + data: makeData(1, "My Board", "My Devlake Project"), + want: `labels = "My Devlake Project"`, + }, + { + name: "board with no mapped Devlake project falls back to empty ProjectName", + tmpl: `labels = "{{.ProjectName}}"`, + data: makeData(1, "My Board", ""), + want: `labels = ""`, + }, { name: "invalid template returns error", tmpl: `project = "{{.Unclosed"`, diff --git a/backend/plugins/jira/tasks/task_data.go b/backend/plugins/jira/tasks/task_data.go index 67044810e66..a49b3a8f3f7 100644 --- a/backend/plugins/jira/tasks/task_data.go +++ b/backend/plugins/jira/tasks/task_data.go @@ -31,6 +31,13 @@ type JiraOptions struct { ScopeConfig *models.JiraScopeConfig `json:"scopeConfig" mapstructure:"scopeConfig"` ScopeConfigId uint64 `json:"scopeConfigId" mapstructure:"scopeConfigId"` PageSize int `json:"pageSize" mapstructure:"pageSize"` + // ProjectName is the Devlake project this pipeline run belongs to. It is + // injected generically into every plugin's task options by + // services.GeneratePlanJsonV200, not set by Jira's own blueprint plan + // builder, since that's the only place in the framework where the + // running project is known unambiguously (a board scope can be attached + // to more than one Devlake project). + ProjectName string `json:"projectName" mapstructure:"projectName,omitempty"` } type JiraTaskData struct { diff --git a/backend/server/services/blueprint_makeplan_v200.go b/backend/server/services/blueprint_makeplan_v200.go index bd89d7de3e2..73ee897ad76 100644 --- a/backend/server/services/blueprint_makeplan_v200.go +++ b/backend/server/services/blueprint_makeplan_v200.go @@ -70,6 +70,23 @@ func GeneratePlanJsonV200( } } + // let every task know which Devlake project it's running for, so plugins + // can use it (e.g. Jira's ExtraJQL {{.ProjectName}}) without needing + // project context threaded through their own blueprint-plan-building + // logic. Unrecognized map keys are ignored by mapstructure decoding, so + // this is safe for plugins that don't care about it. + if projectName != "" { + for _, plan := range sourcePlans { + for _, stage := range plan { + for _, task := range stage { + if task.Options != nil { + task.Options["projectName"] = projectName + } + } + } + } + } + // skip collectors if skipCollectors { for i, plan := range sourcePlans { diff --git a/backend/server/services/blueprint_makeplan_v200_test.go b/backend/server/services/blueprint_makeplan_v200_test.go index 22b53b53257..0b3943ae97f 100644 --- a/backend/server/services/blueprint_makeplan_v200_test.go +++ b/backend/server/services/blueprint_makeplan_v200_test.go @@ -29,6 +29,7 @@ import ( mockplugin "github.com/apache/incubator-devlake/mocks/core/plugin" "github.com/apache/incubator-devlake/plugins/org/tasks" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" ) func TestMakePlanV200(t *testing.T) { @@ -101,3 +102,45 @@ func TestMakePlanV200(t *testing.T) { assert.Equal(t, expectedPlan, plan) } + +// TestMakePlanV200InjectsProjectNameIntoDataSourceTaskOptions verifies that +// GeneratePlanJsonV200 stamps the running Devlake project's name into every +// data-source task's options, without requiring the plugin's own +// MakeDataSourcePipelinePlanV200 implementation to know about it. This is +// what lets a scope shared by multiple projects (e.g. a Jira board added to +// two different Devlake projects) resolve the correct project per pipeline +// run instead of guessing from a many-to-one reverse lookup. +func TestMakePlanV200InjectsProjectNameIntoDataSourceTaskOptions(t *testing.T) { + const projectName = "TestMakePlanV200InjectsProjectName-project" + jiraName := "TestMakePlanV200InjectsProjectName-jira" + connId := uint64(1) + scopes := []*coreModels.BlueprintScope{{ScopeId: "jira:JiraBoard:1:1"}} + outputPlan := coreModels.PipelinePlan{ + { + {Plugin: jiraName, Options: map[string]interface{}{"boardId": float64(1)}}, + }, + } + jira := new(mockplugin.CompositeDataSourcePluginBlueprintV200) + jira.On("MakeDataSourcePipelinePlanV200", connId, scopes).Return(outputPlan, []plugin.Scope(nil), nil) + plugin.RegisterPlugin(jiraName, jira) + + // GeneratePlanJsonV200 also calls the "org" plugin's ProjectMapper + // whenever projectName != "". Re-register it here (overwriting any + // registration from other tests in this package) with permissive + // matchers, since this test doesn't care about project_mapping. + org := new(mockplugin.CompositeProjectMapper) + org.On("MapProject", mock.Anything, mock.Anything).Return(coreModels.PipelinePlan{}, nil) + plugin.RegisterPlugin("org", org) + + connections := []*coreModels.BlueprintConnection{ + {PluginName: jiraName, ConnectionId: connId, Scopes: scopes}, + } + + plan, err := GeneratePlanJsonV200(projectName, connections, nil, false) + assert.Nil(t, err) + assert.Equal(t, 1, len(plan)) + assert.Equal(t, 1, len(plan[0])) + assert.Equal(t, projectName, plan[0][0].Options["projectName"]) + // the plugin's own option is preserved alongside the injected one + assert.Equal(t, float64(1), plan[0][0].Options["boardId"]) +} diff --git a/config-ui/src/plugins/register/jira/transformation.tsx b/config-ui/src/plugins/register/jira/transformation.tsx index bbfe760dba0..3db9116785b 100644 --- a/config-ui/src/plugins/register/jira/transformation.tsx +++ b/config-ui/src/plugins/register/jira/transformation.tsx @@ -269,7 +269,7 @@ const renderCollapseItems = ({ label={ <> Extra JQL - + } extra='Tip: use Go template variables to make this dynamic, e.g. owner = "{{.BoardName}}"'