From 903d605fe10127d7b4a4f4735ec42a0cd66952e3 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 24 Oct 2025 10:55:10 -0700 Subject: [PATCH] improvements --- models/project/workflows.go | 8 +- routers/web/projects/workflows.go | 8 +- services/projects/workflow_notifier.go | 114 ++++-------------- templates/repo/projects/workflows.tmpl | 4 +- .../components/projects/ProjectWorkflow.vue | 6 +- .../js/components/projects/WorkflowStore.ts | 42 +++---- 6 files changed, 58 insertions(+), 124 deletions(-) diff --git a/models/project/workflows.go b/models/project/workflows.go index 48e88c6c56..49ce17c1ce 100644 --- a/models/project/workflows.go +++ b/models/project/workflows.go @@ -64,7 +64,7 @@ func (we WorkflowEvent) LangKey() string { } } -func (we WorkflowEvent) UUID() string { +func (we WorkflowEvent) EventID() string { switch we { case WorkflowEventItemOpened: return "item_opened" @@ -90,9 +90,9 @@ func (we WorkflowEvent) UUID() string { type WorkflowFilterType string const ( - WorkflowFilterTypeIssueType WorkflowFilterType = "issue_type" // issue, pull_request, etc. - WorkflowFilterTypeColumn WorkflowFilterType = "column" // target column for item_column_changed event - WorkflowFilterTypeLabels WorkflowFilterType = "labels" // filter by issue/PR labels + WorkflowFilterTypeIssueType WorkflowFilterType = "issue_type" // issue, pull_request, etc. + WorkflowFilterTypeColumn WorkflowFilterType = "target_column" // target column for item_column_changed event + WorkflowFilterTypeLabels WorkflowFilterType = "labels" // filter by issue/PR labels ) type WorkflowFilter struct { diff --git a/routers/web/projects/workflows.go b/routers/web/projects/workflows.go index a8bd0bda20..47e1b7d5e2 100644 --- a/routers/web/projects/workflows.go +++ b/routers/web/projects/workflows.go @@ -52,7 +52,7 @@ func getFilterSummary(ctx stdCtx.Context, filters []project_model.WorkflowFilter log.Error("GetColumn: %v", err) continue } - summary.WriteString(" (Column: " + col.Title + ")") + summary.WriteString(" (Target Column: " + col.Title + ")") case project_model.WorkflowFilterTypeLabels: labelID, _ := strconv.ParseInt(filter.Value, 10, 64) if labelID > 0 { @@ -243,7 +243,7 @@ func WorkflowsEvents(ctx *context.Context) { outputWorkflows = append(outputWorkflows, &WorkflowConfig{ ID: wf.ID, EventID: strconv.FormatInt(wf.ID, 10), - DisplayName: string(ctx.Tr(wf.WorkflowEvent.LangKey())) + filterSummary, + DisplayName: string(ctx.Tr(wf.WorkflowEvent.LangKey())), BaseEventType: string(wf.WorkflowEvent), WorkflowEvent: string(wf.WorkflowEvent), Capabilities: capabilities[event], @@ -258,7 +258,7 @@ func WorkflowsEvents(ctx *context.Context) { // Add placeholder for creating new workflow outputWorkflows = append(outputWorkflows, &WorkflowConfig{ ID: 0, - EventID: event.UUID(), + EventID: event.EventID(), DisplayName: string(ctx.Tr(event.LangKey())), BaseEventType: string(event), WorkflowEvent: string(event), @@ -514,7 +514,7 @@ func WorkflowsPost(ctx *context.Context) { "workflow": map[string]any{ "id": wf.ID, "event_id": strconv.FormatInt(wf.ID, 10), - "display_name": string(ctx.Tr(wf.WorkflowEvent.LangKey())) + filterSummary, + "display_name": string(ctx.Tr(wf.WorkflowEvent.LangKey())), "filters": wf.WorkflowFilters, "actions": wf.WorkflowActions, "filter_summary": filterSummary, diff --git a/services/projects/workflow_notifier.go b/services/projects/workflow_notifier.go index a48bb7e556..65ea72a8f8 100644 --- a/services/projects/workflow_notifier.go +++ b/services/projects/workflow_notifier.go @@ -55,7 +55,7 @@ func (m *workflowNotifier) NewIssue(ctx context.Context, issue *issues_model.Iss // Find workflows for the ItemOpened event for _, workflow := range workflows { if workflow.WorkflowEvent == project_model.WorkflowEventItemOpened { - fireIssueWorkflow(ctx, workflow, issue) + fireIssueWorkflow(ctx, workflow, issue, 0) } } } @@ -92,7 +92,7 @@ func (m *workflowNotifier) IssueChangeStatus(ctx context.Context, doer *user_mod // Find workflows for the specific event for _, workflow := range workflows { if workflow.WorkflowEvent == workflowEvent { - fireIssueWorkflow(ctx, workflow, issue) + fireIssueWorkflow(ctx, workflow, issue, 0) } } } @@ -124,7 +124,7 @@ func (*workflowNotifier) IssueChangeProjects(ctx context.Context, doer *user_mod // Find workflows for the ItemOpened event for _, workflow := range workflows { if workflow.WorkflowEvent == project_model.WorkflowEventItemAddedToProject { - fireIssueWorkflow(ctx, workflow, issue) + fireIssueWorkflow(ctx, workflow, issue, 0) } } } @@ -158,7 +158,7 @@ func (*workflowNotifier) IssueChangeProjectColumn(ctx context.Context, doer *use // Find workflows for the ItemColumnChanged event for _, workflow := range workflows { if workflow.WorkflowEvent == project_model.WorkflowEventItemColumnChanged { - fireIssueWorkflowWithColumn(ctx, workflow, issue, newColumnID) + fireIssueWorkflow(ctx, workflow, issue, newColumnID) } } } @@ -192,7 +192,7 @@ func (*workflowNotifier) MergePullRequest(ctx context.Context, doer *user_model. // Find workflows for the PullRequestMerged event for _, workflow := range workflows { if workflow.WorkflowEvent == project_model.WorkflowEventPullRequestMerged { - fireIssueWorkflow(ctx, workflow, issue) + fireIssueWorkflow(ctx, workflow, issue, 0) } } } @@ -231,14 +231,12 @@ func (*workflowNotifier) PullRequestReview(ctx context.Context, pr *issues_model for _, workflow := range workflows { if (workflow.WorkflowEvent == project_model.WorkflowEventCodeChangesRequested && review.Type == issues_model.ReviewTypeReject) || (workflow.WorkflowEvent == project_model.WorkflowEventCodeReviewApproved && review.Type == issues_model.ReviewTypeApprove) { - fireIssueWorkflow(ctx, workflow, issue) + fireIssueWorkflow(ctx, workflow, issue, 0) } } } -// fireIssueWorkflowWithColumn fires a workflow for an issue with a specific column ID -// This is used for ItemColumnChanged events where we need to check the target column -func fireIssueWorkflowWithColumn(ctx context.Context, workflow *project_model.Workflow, issue *issues_model.Issue, columnID int64) { +func fireIssueWorkflow(ctx context.Context, workflow *project_model.Workflow, issue *issues_model.Issue, columnID int64) { if !workflow.Enabled { return } @@ -249,6 +247,15 @@ func fireIssueWorkflowWithColumn(ctx context.Context, workflow *project_model.Wo return } + if !matchWorkflowsFilters(workflow, issue, columnID) { + return + } + + executeWorkflowActions(ctx, workflow, issue) +} + +// matchWorkflowsFilters checks if the issue matches all filters of the workflow +func matchWorkflowsFilters(workflow *project_model.Workflow, issue *issues_model.Issue, columnID int64) bool { for _, filter := range workflow.WorkflowFilters { switch filter.Type { case project_model.WorkflowFilterTypeIssueType: @@ -258,10 +265,10 @@ func fireIssueWorkflowWithColumn(ctx context.Context, workflow *project_model.Wo } // Filter value can be "issue" or "pull_request" if filter.Value == "issue" && issue.IsPull { - return + return false } if filter.Value == "pull_request" && !issue.IsPull { - return + return false } case project_model.WorkflowFilterTypeColumn: // If filter value is empty, match all columns @@ -271,18 +278,18 @@ func fireIssueWorkflowWithColumn(ctx context.Context, workflow *project_model.Wo filterColumnID, _ := strconv.ParseInt(filter.Value, 10, 64) if filterColumnID == 0 { log.Error("Invalid column ID: %s", filter.Value) - return + return false } // For column changed event, check against the new column ID - if columnID != filterColumnID { - return + if columnID > 0 && columnID != filterColumnID { + return false } case project_model.WorkflowFilterTypeLabels: // Check if issue has the specified label labelID, _ := strconv.ParseInt(filter.Value, 10, 64) if labelID == 0 { log.Error("Invalid label ID: %s", filter.Value) - return + return false } // Check if issue has this label hasLabel := false @@ -293,85 +300,14 @@ func fireIssueWorkflowWithColumn(ctx context.Context, workflow *project_model.Wo } } if !hasLabel { - return + return false } default: log.Error("Unsupported filter type: %s", filter.Type) - return + return false } } - - executeWorkflowActions(ctx, workflow, issue) -} - -func fireIssueWorkflow(ctx context.Context, workflow *project_model.Workflow, issue *issues_model.Issue) { - if !workflow.Enabled { - return - } - - // Load issue labels for labels filter - if err := issue.LoadLabels(ctx); err != nil { - log.Error("LoadLabels: %v", err) - return - } - - for _, filter := range workflow.WorkflowFilters { - switch filter.Type { - case project_model.WorkflowFilterTypeIssueType: - // If filter value is empty, match all types - if filter.Value == "" { - continue - } - // Filter value can be "issue" or "pull_request" - if filter.Value == "issue" && issue.IsPull { - return - } - if filter.Value == "pull_request" && !issue.IsPull { - return - } - case project_model.WorkflowFilterTypeColumn: - // If filter value is empty, match all columns - if filter.Value == "" { - continue - } - columnID, _ := strconv.ParseInt(filter.Value, 10, 64) - if columnID == 0 { - log.Error("Invalid column ID: %s", filter.Value) - return - } - issueProjectColumnID, err := issue.ProjectColumnID(ctx) - if err != nil { - log.Error("Issue.ProjectColumnID: %v", err) - return - } - if issueProjectColumnID != columnID { - return - } - case project_model.WorkflowFilterTypeLabels: - // Check if issue has the specified label - labelID, _ := strconv.ParseInt(filter.Value, 10, 64) - if labelID == 0 { - log.Error("Invalid label ID: %s", filter.Value) - return - } - // Check if issue has this label - hasLabel := false - for _, label := range issue.Labels { - if label.ID == labelID { - hasLabel = true - break - } - } - if !hasLabel { - return - } - default: - log.Error("Unsupported filter type: %s", filter.Type) - return - } - } - - executeWorkflowActions(ctx, workflow, issue) + return true } func executeWorkflowActions(ctx context.Context, workflow *project_model.Workflow, issue *issues_model.Issue) { diff --git a/templates/repo/projects/workflows.tmpl b/templates/repo/projects/workflows.tmpl index 1752cd8c8a..2d5d0325f3 100644 --- a/templates/repo/projects/workflows.tmpl +++ b/templates/repo/projects/workflows.tmpl @@ -3,8 +3,8 @@ {{template "repo/header" .}}
- {{svg "octicon-arrow-left"}} {{ctx.Locale.Tr "projects.workflows"}} {{.Project.Title}} -
+ {{svg "octicon-arrow-left"}} {{ctx.Locale.Tr "projects.workflows"}} {{.Project.Title}} +
{{template "projects/workflows" .}} diff --git a/web_src/js/components/projects/ProjectWorkflow.vue b/web_src/js/components/projects/ProjectWorkflow.vue index 306286e6d3..6668b8de9a 100644 --- a/web_src/js/components/projects/ProjectWorkflow.vue +++ b/web_src/js/components/projects/ProjectWorkflow.vue @@ -829,11 +829,11 @@ onUnmounted(() => { -
+
- {{ store.projectColumns.find(c => String(c.id) === store.workflowFilters.column)?.title || 'Any column' }} + {{ store.projectColumns.find(c => String(c.id) === store.workflowFilters.target_column)?.title || 'Any column' }}
diff --git a/web_src/js/components/projects/WorkflowStore.ts b/web_src/js/components/projects/WorkflowStore.ts index f7009a8cca..5f81389e6d 100644 --- a/web_src/js/components/projects/WorkflowStore.ts +++ b/web_src/js/components/projects/WorkflowStore.ts @@ -4,7 +4,7 @@ import {showInfoToast, showErrorToast} from '../../modules/toast.ts'; type WorkflowFiltersState = { issue_type: string; - column: string; + target_column: string; labels: string[]; }; @@ -14,7 +14,7 @@ type WorkflowActionsState = { column: string; add_labels: string[]; remove_labels: string[]; - issueState: WorkflowIssueStateAction; + issue_state: WorkflowIssueStateAction; }; type WorkflowDraftState = { @@ -22,12 +22,12 @@ type WorkflowDraftState = { actions: WorkflowActionsState; }; -const createDefaultFilters = (): WorkflowFiltersState => ({issue_type: '', column: '', labels: []}); -const createDefaultActions = (): WorkflowActionsState => ({column: '', add_labels: [], remove_labels: [], issueState: ''}); +const createDefaultFilters = (): WorkflowFiltersState => ({issue_type: '', target_column: '', labels: []}); +const createDefaultActions = (): WorkflowActionsState => ({column: '', add_labels: [], remove_labels: [], issue_state: ''}); const cloneFilters = (filters: WorkflowFiltersState): WorkflowFiltersState => ({ issue_type: filters.issue_type, - column: filters.column, + target_column: filters.target_column, labels: Array.from(filters.labels), }); @@ -35,7 +35,7 @@ const cloneActions = (actions: WorkflowActionsState): WorkflowActionsState => ({ column: actions.column, add_labels: Array.from(actions.add_labels), remove_labels: Array.from(actions.remove_labels), - issueState: actions.issueState, + issue_state: actions.issue_state, }); export function createWorkflowStore(props: {projectLink: string, eventID: string}) { @@ -107,21 +107,19 @@ export function createWorkflowStore(props: {projectLink: string, eventID: string // Find the workflow from existing workflowEvents const workflow = store.workflowEvents.find((e) => e.event_id === eventId); - console.log('[WorkflowStore] loadWorkflowData - eventId:', eventId); - console.log('[WorkflowStore] loadWorkflowData - found workflow:', workflow); // Load existing configuration from the workflow data // Convert backend filter format to frontend format - const frontendFilters = {issue_type: '', column: '', labels: []}; + const frontendFilters = {issue_type: '', target_column: '', labels: []}; // Convert backend action format to frontend format - const frontendActions: WorkflowActionsState = {column: '', add_labels: [], remove_labels: [], issueState: ''}; + const frontendActions: WorkflowActionsState = {column: '', add_labels: [], remove_labels: [], issue_state: ''}; if (workflow?.filters && Array.isArray(workflow.filters)) { for (const filter of workflow.filters) { if (filter.type === 'issue_type') { frontendFilters.issue_type = filter.value; - } else if (filter.type === 'column') { - frontendFilters.column = filter.value; + } else if (filter.type === 'target_column') { + frontendFilters.target_column = filter.value; } else if (filter.type === 'labels') { frontendFilters.labels.push(filter.value); } @@ -140,9 +138,9 @@ export function createWorkflowStore(props: {projectLink: string, eventID: string frontendActions.remove_labels.push(action.value); } else if (action.type === 'close') { if (action.value === 'reopen' || action.value === 'false') { - frontendActions.issueState = 'reopen'; + frontendActions.issue_state = 'reopen'; } else if (action.value === 'true' || action.value === 'close') { - frontendActions.issueState = 'close'; + frontendActions.issue_state = 'close'; } } } @@ -160,9 +158,9 @@ export function createWorkflowStore(props: {projectLink: string, eventID: string frontendActions.remove_labels.push(action.value); } else if (action.type === 'close') { if (action.value === 'reopen' || action.value === 'false') { - frontendActions.issueState = 'reopen'; + frontendActions.issue_state = 'reopen'; } else if (action.value === 'true' || action.value === 'close') { - frontendActions.issueState = 'close'; + frontendActions.issue_state = 'close'; } } } @@ -261,15 +259,15 @@ export function createWorkflowStore(props: {projectLink: string, eventID: string // Convert backend data to frontend format and update form // Use the selectedWorkflow which now points to the reloaded workflow with complete data - const frontendFilters = {issue_type: '', column: '', labels: []}; - const frontendActions: WorkflowActionsState = {column: '', add_labels: [], remove_labels: [], issueState: ''}; + const frontendFilters = {issue_type: '', target_column: '', labels: []}; + const frontendActions: WorkflowActionsState = {column: '', add_labels: [], remove_labels: [], issue_state: ''}; if (store.selectedWorkflow.filters && Array.isArray(store.selectedWorkflow.filters)) { for (const filter of store.selectedWorkflow.filters) { if (filter.type === 'issue_type') { frontendFilters.issue_type = filter.value; - } else if (filter.type === 'column') { - frontendFilters.column = filter.value; + } else if (filter.type === 'target_column') { + frontendFilters.target_column = filter.value; } else if (filter.type === 'labels') { frontendFilters.labels.push(filter.value); } @@ -286,9 +284,9 @@ export function createWorkflowStore(props: {projectLink: string, eventID: string frontendActions.remove_labels.push(action.value); } else if (action.type === 'close') { if (action.value === 'reopen' || action.value === 'false') { - frontendActions.issueState = 'reopen'; + frontendActions.issue_state = 'reopen'; } else if (action.value === 'true' || action.value === 'close') { - frontendActions.issueState = 'close'; + frontendActions.issue_state = 'close'; } } }