From 3d081b300fb47e0ac8277532a72b3e5b04d7acda Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 24 Oct 2025 21:35:11 -0700 Subject: [PATCH] Fix bug --- models/project/issue.go | 8 - models/project/workflows.go | 9 + routers/web/projects/workflows.go | 40 +++-- routers/web/web.go | 17 +- .../components/projects/ProjectWorkflow.vue | 70 ++++---- .../js/components/projects/WorkflowStore.ts | 159 ++++++------------ 6 files changed, 138 insertions(+), 165 deletions(-) diff --git a/models/project/issue.go b/models/project/issue.go index de395eb828..47d1537ec7 100644 --- a/models/project/issue.go +++ b/models/project/issue.go @@ -33,14 +33,6 @@ func deleteProjectIssuesByProjectID(ctx context.Context, projectID int64) error return err } -func AddIssueToColumn(ctx context.Context, issueID int64, newColumn *Column) error { - return db.Insert(ctx, &ProjectIssue{ - IssueID: issueID, - ProjectID: newColumn.ProjectID, - ProjectColumnID: newColumn.ID, - }) -} - func (c *Column) moveIssuesToAnotherColumn(ctx context.Context, newColumn *Column) error { if c.ProjectID != newColumn.ProjectID { return errors.New("columns have to be in the same project") diff --git a/models/project/workflows.go b/models/project/workflows.go index 5a03316bec..4c726e5b6c 100644 --- a/models/project/workflows.go +++ b/models/project/workflows.go @@ -41,6 +41,15 @@ func GetWorkflowEvents() []WorkflowEvent { return workflowEvents } +func IsValidWorkflowEvent(event string) bool { + for _, we := range workflowEvents { + if we.EventID() == event { + return true + } + } + return false +} + func (we WorkflowEvent) LangKey() string { switch we { case WorkflowEventItemOpened: diff --git a/routers/web/projects/workflows.go b/routers/web/projects/workflows.go index 79543bd8c3..faa6080994 100644 --- a/routers/web/projects/workflows.go +++ b/routers/web/projects/workflows.go @@ -5,7 +5,6 @@ package projects import ( stdCtx "context" - "errors" "io" "net/http" "strconv" @@ -38,9 +37,15 @@ func getFilterSummary(ctx stdCtx.Context, filters []project_model.WorkflowFilter case project_model.WorkflowFilterTypeIssueType: switch filter.Value { case "issue": - summary.WriteString(" (Issues only)") + if summary.Len() > 0 { + summary.WriteString(" ") + } + summary.WriteString("(Issues only)") case "pull_request": - summary.WriteString(" (Pull requests only)") + if summary.Len() > 0 { + summary.WriteString(" ") + } + summary.WriteString("(Pull requests only)") } case project_model.WorkflowFilterTypeSourceColumn: columnID, _ := strconv.ParseInt(filter.Value, 10, 64) @@ -52,7 +57,10 @@ func getFilterSummary(ctx stdCtx.Context, filters []project_model.WorkflowFilter log.Error("GetColumn: %v", err) continue } - summary.WriteString(" (Source Column: " + col.Title + ")") + if summary.Len() > 0 { + summary.WriteString(" ") + } + summary.WriteString("(Source: " + col.Title + ")") case project_model.WorkflowFilterTypeTargetColumn: columnID, _ := strconv.ParseInt(filter.Value, 10, 64) if columnID <= 0 { @@ -63,7 +71,10 @@ func getFilterSummary(ctx stdCtx.Context, filters []project_model.WorkflowFilter log.Error("GetColumn: %v", err) continue } - summary.WriteString(" (Target Column: " + col.Title + ")") + if summary.Len() > 0 { + summary.WriteString(" ") + } + summary.WriteString("(Target: " + col.Title + ")") case project_model.WorkflowFilterTypeLabels: labelID, _ := strconv.ParseInt(filter.Value, 10, 64) if labelID > 0 { @@ -76,7 +87,10 @@ func getFilterSummary(ctx stdCtx.Context, filters []project_model.WorkflowFilter if err != nil { log.Error("GetLabelsByIDs: %v", err) } else { - summary.WriteString(" (Labels: ") + if summary.Len() > 0 { + summary.WriteString(" ") + } + summary.WriteString("(Labels: ") for i, label := range labels { summary.WriteString(label.Name) if i < len(labels)-1 { @@ -225,8 +239,7 @@ func WorkflowsEvents(ctx *context.Context) { ID int64 `json:"id"` EventID string `json:"event_id"` DisplayName string `json:"display_name"` - BaseEventType string `json:"base_event_type"` // Base event type for grouping - WorkflowEvent string `json:"workflow_event"` // The actual workflow event + WorkflowEvent string `json:"workflow_event"` // The workflow event Capabilities project_model.WorkflowEventCapabilities `json:"capabilities"` Filters []project_model.WorkflowFilter `json:"filters"` Actions []project_model.WorkflowAction `json:"actions"` @@ -255,7 +268,6 @@ func WorkflowsEvents(ctx *context.Context) { ID: wf.ID, EventID: strconv.FormatInt(wf.ID, 10), DisplayName: string(ctx.Tr(wf.WorkflowEvent.LangKey())), - BaseEventType: string(wf.WorkflowEvent), WorkflowEvent: string(wf.WorkflowEvent), Capabilities: capabilities[event], Filters: wf.WorkflowFilters, @@ -271,7 +283,6 @@ func WorkflowsEvents(ctx *context.Context) { ID: 0, EventID: event.EventID(), DisplayName: string(ctx.Tr(event.LangKey())), - BaseEventType: string(event), WorkflowEvent: string(event), Capabilities: capabilities[event], FilterSummary: "", @@ -460,6 +471,7 @@ type WorkflowsPostForm struct { Actions map[string]any `json:"actions"` } +// WorkflowsPost handles creating or updating a workflow func WorkflowsPost(ctx *context.Context) { projectID := ctx.PathParamInt64("id") p, err := project_model.GetProjectByID(ctx, projectID) @@ -495,7 +507,7 @@ func WorkflowsPost(ctx *context.Context) { return } if form.EventID == "" { - ctx.ServerError("InvalidEventID", errors.New("EventID is required")) + ctx.JSON(http.StatusBadRequest, map[string]any{"error": "InvalidEventID", "message": "EventID is required"}) return } @@ -505,6 +517,12 @@ func WorkflowsPost(ctx *context.Context) { eventID, _ := strconv.ParseInt(form.EventID, 10, 64) if eventID == 0 { + // check if workflow event is valid + if !project_model.IsValidWorkflowEvent(form.EventID) { + ctx.JSON(http.StatusBadRequest, map[string]any{"error": "EventID is invalid"}) + return + } + // Create a new workflow for the given event wf := &project_model.Workflow{ ProjectID: projectID, diff --git a/routers/web/web.go b/routers/web/web.go index 13aab1fd0a..0c4b145722 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -1048,7 +1048,7 @@ func registerWebRoutes(m *web.Router) { m.Post("/{workflow_id}", projects.WorkflowsPost) m.Post("/{workflow_id}/status", projects.WorkflowsStatus) m.Post("/{workflow_id}/delete", projects.WorkflowsDelete) - }, reqUnitAccess(unit.TypeProjects, perm.AccessModeRead, true)) + }, reqUnitAccess(unit.TypeProjects, perm.AccessModeWrite, true)) m.Group("", func() { //nolint:dupl // duplicates lines 1421-1441 m.Get("/new", org.RenderNewProject) m.Post("/new", web.Bind(forms.CreateProjectForm{}), org.NewProjectPost) @@ -1437,13 +1437,6 @@ func registerWebRoutes(m *web.Router) { m.Group("/{username}/{reponame}/projects", func() { m.Get("", repo.Projects) m.Get("/{id}", repo.ViewProject) - m.Group("/{id}/workflows", func() { - m.Get("", projects.Workflows) - m.Get("/{workflow_id}", projects.Workflows) - m.Post("/{workflow_id}", projects.WorkflowsPost) - m.Post("/{workflow_id}/status", projects.WorkflowsStatus) - m.Post("/{workflow_id}/delete", projects.WorkflowsDelete) - }) m.Group("", func() { //nolint:dupl // duplicates lines 1034-1054 m.Get("/new", repo.RenderNewProject) m.Post("/new", web.Bind(forms.CreateProjectForm{}), repo.NewProjectPost) @@ -1463,6 +1456,14 @@ func registerWebRoutes(m *web.Router) { m.Post("/default", repo.SetDefaultProjectColumn) m.Post("/move", repo.MoveIssues) }) + + m.Group("/workflows", func() { + m.Get("", projects.Workflows) + m.Get("/{workflow_id}", projects.Workflows) + m.Post("/{workflow_id}", projects.WorkflowsPost) + m.Post("/{workflow_id}/status", projects.WorkflowsStatus) + m.Post("/{workflow_id}/delete", projects.WorkflowsDelete) + }) }) }, reqRepoProjectsWriter, context.RepoMustNotBeArchived()) }, optSignIn, context.RepoAssignment, reqRepoProjectsReader, repo.MustEnableRepoProjects) diff --git a/web_src/js/components/projects/ProjectWorkflow.vue b/web_src/js/components/projects/ProjectWorkflow.vue index 6668b8de9a..a95914ce55 100644 --- a/web_src/js/components/projects/ProjectWorkflow.vue +++ b/web_src/js/components/projects/ProjectWorkflow.vue @@ -94,8 +94,8 @@ const toggleEditMode = () => { // If we removed a temporary item but have no previous selection, fall back to first workflow const fallback = store.workflowEvents.find((w) => { if (!canceledWorkflow) return false; - const baseType = canceledWorkflow.base_event_type || canceledWorkflow.workflow_event; - return baseType && (w.base_event_type === baseType || w.workflow_event === baseType || w.event_id === baseType); + const baseType = canceledWorkflow.workflow_event; + return baseType && (w.workflow_event === baseType || w.event_id === baseType); }) || store.workflowEvents[0]; if (fallback) { store.selectedItem = fallback.event_id; @@ -132,12 +132,6 @@ const deleteWorkflow = async () => { return; } - const currentBaseEventType = store.selectedWorkflow.base_event_type || store.selectedWorkflow.workflow_event || store.selectedWorkflow.event_id; - const currentCapabilities = store.selectedWorkflow.capabilities; - // Extract base name without any parenthetical descriptions - const currentDisplayName = (store.selectedWorkflow.display_name || store.selectedWorkflow.workflow_event || store.selectedWorkflow.event_id) - .replace(/\s*\([^)]*\)\s*/g, ''); - // If deleting a temporary workflow (new or cloned, unsaved), just remove from list if (store.selectedWorkflow.id === 0) { const tempIndex = store.workflowEvents.findIndex((w) => @@ -155,7 +149,7 @@ const deleteWorkflow = async () => { // Find workflows for the same base event type const sameEventWorkflows = store.workflowEvents.filter((w) => - (w.base_event_type === currentBaseEventType || w.workflow_event === currentBaseEventType) + (w.workflow_event === store.selectedWorkflow.workflow_event) ); let workflowToSelect = null; @@ -198,7 +192,7 @@ const cloneWorkflow = (sourceWorkflow) => { if (!sourceWorkflow) return; // Generate a unique temporary ID for the cloned workflow - const tempId = `clone-${sourceWorkflow.base_event_type || sourceWorkflow.workflow_event}-${Date.now()}`; + const tempId = `clone-${sourceWorkflow.workflow_event}-${Date.now()}`; // Extract base name without any parenthetical descriptions const baseName = (sourceWorkflow.display_name || sourceWorkflow.workflow_event || sourceWorkflow.event_id) @@ -209,8 +203,7 @@ const cloneWorkflow = (sourceWorkflow) => { id: 0, // New workflow event_id: tempId, display_name: `${baseName} (Copy)`, - base_event_type: sourceWorkflow.base_event_type || sourceWorkflow.workflow_event || sourceWorkflow.event_id, - workflow_event: sourceWorkflow.workflow_event || sourceWorkflow.base_event_type, + workflow_event: sourceWorkflow.workflow_event, capabilities: sourceWorkflow.capabilities, filters: JSON.parse(JSON.stringify(sourceWorkflow.filters || [])), // Deep clone actions: JSON.parse(JSON.stringify(sourceWorkflow.actions || [])), // Deep clone @@ -325,12 +318,11 @@ const workflowList = computed(() => { return workflows.map((workflow) => ({ ...workflow, isConfigured: isWorkflowConfigured(workflow), - base_event_type: workflow.base_event_type || workflow.workflow_event || workflow.event_id, display_name: workflow.display_name || workflow.workflow_event || workflow.event_id, })); }); -const createNewWorkflow = (baseEventType, capabilities, displayName) => { +const createNewWorkflow = (eventType, capabilities, displayName) => { // Store current selection before creating new workflow if (!isInEditMode.value) { previousSelection.value = { @@ -339,7 +331,7 @@ const createNewWorkflow = (baseEventType, capabilities, displayName) => { }; } - const tempId = `new-${baseEventType}-${Date.now()}`; + const tempId = `new-${eventType}-${Date.now()}`; const newWorkflow = { id: 0, event_id: tempId, @@ -348,14 +340,13 @@ const createNewWorkflow = (baseEventType, capabilities, displayName) => { filters: [], actions: [], filter_summary: '', - base_event_type: baseEventType, - workflow_event: baseEventType, + workflow_event: eventType, enabled: true, // Ensure new workflows are enabled by default }; store.selectedWorkflow = newWorkflow; // For unconfigured events, use the base event type as selected item for UI consistency - store.selectedItem = baseEventType; + store.selectedItem = eventType; store.resetWorkflowData(); // Unconfigured workflows are always in edit mode by default }; @@ -383,8 +374,7 @@ const selectWorkflowItem = async (item) => { } else { // This is an unconfigured event - check if we already have a workflow object for it const existingWorkflow = store.workflowEvents.find((w) => - w.id === 0 && - (w.base_event_type === item.base_event_type || w.workflow_event === item.base_event_type), + w.id === 0 && w.workflow_event === item.workflow_event, ); if (existingWorkflow) { @@ -392,12 +382,12 @@ const selectWorkflowItem = async (item) => { await selectWorkflowEvent(existingWorkflow); } else { // This is truly a new unconfigured event, create new workflow - createNewWorkflow(item.base_event_type, item.capabilities, item.display_name); + createNewWorkflow(item.workflow_event, item.capabilities, item.display_name); } // Update URL for workflow - const newUrl = `${props.projectLink}/workflows/${item.base_event_type}`; - window.history.pushState({eventId: item.base_event_type}, '', newUrl); + const newUrl = `${props.projectLink}/workflows/${item.workflow_event}`; + window.history.pushState({eventId: item.workflow_event}, '', newUrl); } }; @@ -433,18 +423,17 @@ const isItemSelected = (item) => { // For configured workflows or temporary workflows (new), match by event_id return store.selectedItem === item.event_id; } - // For unconfigured events, match by base_event_type - return store.selectedItem === item.base_event_type; + // For unconfigured events, match by workflow_event + return store.selectedItem === item.workflow_event; }; // Get display name for workflow with numbering for same types const getWorkflowDisplayName = (item, index) => { const list = workflowList.value; - const baseEventType = item.base_event_type || item.workflow_event; // Find all workflows of the same type const sameTypeWorkflows = list.filter(w => - (w.base_event_type || w.workflow_event) === baseEventType && + w.workflow_event === item.workflow_event && (w.isConfigured || w.id === 0) // Only count configured workflows ); @@ -517,7 +506,7 @@ watch(isInEditMode, async (newVal) => { const getCurrentDraftKey = () => { if (!store.selectedWorkflow) return null; - return store.selectedWorkflow.event_id || store.selectedWorkflow.base_event_type; + return store.selectedWorkflow.event_id || store.selectedWorkflow.workflow_event; }; const persistDraftState = () => { @@ -576,11 +565,11 @@ onMounted(async () => { // Check if eventID matches a base event type (unconfigured workflow) const items = workflowList.value; const matchingUnconfigured = items.find((item) => - !item.isConfigured && (item.base_event_type === props.eventID || item.event_id === props.eventID), + !item.isConfigured && (item.workflow_event === props.eventID || item.event_id === props.eventID), ); if (matchingUnconfigured) { // Create new workflow for this base event type - createNewWorkflow(matchingUnconfigured.base_event_type, matchingUnconfigured.capabilities, matchingUnconfigured.display_name); + createNewWorkflow(matchingUnconfigured.workflow_event, matchingUnconfigured.capabilities, matchingUnconfigured.display_name); } else { // Fallback: select first available item if (items.length > 0) { @@ -626,10 +615,10 @@ const popstateHandler = (e) => { // Check if it's a base event type const items = workflowList.value; const matchingUnconfigured = items.find((item) => - !item.isConfigured && (item.base_event_type === e.state.eventId || item.event_id === e.state.eventId), + !item.isConfigured && (item.workflow_event === e.state.eventId || item.event_id === e.state.eventId), ); if (matchingUnconfigured) { - createNewWorkflow(matchingUnconfigured.base_event_type, matchingUnconfigured.capabilities, matchingUnconfigured.display_name); + createNewWorkflow(matchingUnconfigured.workflow_event, matchingUnconfigured.capabilities, matchingUnconfigured.display_name); } } } @@ -829,6 +818,23 @@ onUnmounted(() => { +
+ + +
+ {{ store.projectColumns.find(c => String(c.id) === store.workflowFilters.source_column)?.title || 'Any column' }} +
+
+