Labels are metadata attached to patches, extracted from subject prefixes at parse time. When a prefix matches a known label name (project-scoped or global), it is stripped from the patch name and recorded as a label association instead.
The parser, REST API (with filtering since v1.4), web UI (with colored badges) and CLI management commands (list, create, update, delete, relabel) are all included. A default global "RFC" label is seeded on fresh databases. Suggested-by: Franciszek Stachura <[email protected]> Signed-off-by: Robin Jarry <[email protected]> --- cmd/pw/admin/labels.go | 317 +++++++++++++++++++++++++++++++ cmd/pw/admin/main.go | 1 + docs/deployment/management.rst | 89 +++++++++ docs/usage/overview.rst | 22 +++ pkg/api/label_test.go | 154 +++++++++++++++ pkg/api/patches.go | 26 +++ pkg/api/types.go | 1 + pkg/api/util.go | 24 +-- pkg/db/label.go | 116 +++++++++++ pkg/db/migrations/0004_labels.go | 60 ++++++ pkg/db/models.go | 39 ++++ pkg/db/schema.go | 2 + pkg/db/seed.go | 13 ++ pkg/mail/cover.go | 2 +- pkg/mail/headers.go | 18 +- pkg/mail/parser.go | 47 +++++ pkg/mail/parser_test.go | 111 +++++++++++ pkg/mail/patch.go | 1 + pkg/mail/series.go | 4 +- pkg/mail/similarity.go | 4 +- pkg/web/filters.go | 22 +++ pkg/web/patch.templ | 10 + pkg/web/patches.go | 10 + pkg/web/patches.templ | 23 +++ pkg/web/static/style.css | 16 ++ 25 files changed, 1112 insertions(+), 20 deletions(-) create mode 100644 cmd/pw/admin/labels.go create mode 100644 pkg/api/label_test.go create mode 100644 pkg/db/label.go create mode 100644 pkg/db/migrations/0004_labels.go diff --git a/cmd/pw/admin/labels.go b/cmd/pw/admin/labels.go new file mode 100644 index 000000000000..859540f2408c --- /dev/null +++ b/cmd/pw/admin/labels.go @@ -0,0 +1,317 @@ +// Patchwork - automated patch tracking system +// Copyright (C) The Patchwork Contributors (see CONTRIBUTORS) +// +// SPDX-License-Identifier: GPL-2.0-or-later + +package admin + +import ( + "bufio" + "fmt" + "os" + "strings" + "text/tabwriter" + + "github.com/uptrace/bun" + + "github.com/getpatchwork/patchwork/cmd/pw/pw" + "github.com/getpatchwork/patchwork/pkg/db" + "github.com/getpatchwork/patchwork/pkg/log" + "github.com/getpatchwork/patchwork/pkg/mail" +) + +type LabelCmd struct { + List LabelListCmd `cmd:"" help:"List labels."` + Create LabelCreateCmd `cmd:"" help:"Create a label."` + Update LabelUpdateCmd `cmd:"" help:"Update a label."` + Delete LabelDeleteCmd `cmd:"" help:"Delete a label."` + Relabel RelabelCmd `cmd:"" help:"Relabel existing patches from subject prefixes."` +} + +type LabelListCmd struct{} + +func (c *LabelListCmd) Run(ctx *pw.Context) error { + var labels []db.Label + err := ctx.DB.NewSelect().Model(&labels). + OrderExpr("id ASC"). + Scan(ctx) + if err != nil { + return err + } + + q := db.New(ctx, ctx.DB) + projectNames := make(map[int]string) + + w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) + fmt.Fprintf(w, "ID\tNAME\tPROJECT\tCOLOR\tDESCRIPTION\n") + for _, l := range labels { + proj := "(global)" + if l.ProjectID != nil { + name, ok := projectNames[*l.ProjectID] + if !ok { + p, err := q.GetProjectByID(*l.ProjectID) + if err == nil { + name = p.Linkname + } else { + name = fmt.Sprintf("?%d", *l.ProjectID) + } + projectNames[*l.ProjectID] = name + } + proj = name + } + fmt.Fprintf(w, "%d\t%s\t%s\t#%06x\t%s\n", + l.ID, l.Name, proj, l.Color, l.Description) + } + return w.Flush() +} + +type LabelCreateCmd struct { + Name string `required:"" short:"n" help:"Label name."` + Project string `short:"p" help:"Project linkname (omit for global)."` + Description string `short:"d" help:"Label description."` + Color int `short:"c" default:"0" help:"Color as integer (e.g. 0xff0000 for red)."` +} + +func (c *LabelCreateCmd) Run(ctx *pw.Context) error { + label := db.Label{ + Name: c.Name, + Description: c.Description, + Color: c.Color, + } + if c.Project != "" { + q := db.New(ctx, ctx.DB) + project, err := q.GetProjectByLinkname(c.Project) + if err != nil { + return fmt.Errorf("project %q not found", c.Project) + } + label.ProjectID = &project.ID + } + + err := db.New(ctx, ctx.DB).Insert(&label) + if err != nil { + return err + } + + fmt.Printf("Created label %q (id=%d)\n", label.Name, label.ID) + return nil +} + +type LabelUpdateCmd struct { + Name string `arg:"" help:"Label name to update."` + Project string `short:"p" help:"Move label to this project linkname (use 'global' to make global)."` + Color *int `short:"c" help:"New color as integer."` + Description string `short:"d" help:"New description."` + Rename string `short:"r" help:"Rename the label."` +} + +func (c *LabelUpdateCmd) Run(ctx *pw.Context) error { + var label db.Label + err := ctx.DB.NewSelect().Model(&label). + Where("name = ?", c.Name). + Scan(ctx) + if err != nil { + return fmt.Errorf("label %q not found", c.Name) + } + + uq := ctx.DB.NewUpdate().Model(&label).Where("id = ?", label.ID) + changed := false + + if c.Project != "" { + if strings.EqualFold(c.Project, "global") { + uq = uq.Set("project_id = NULL") + } else { + q := db.New(ctx, ctx.DB) + project, err := q.GetProjectByLinkname(c.Project) + if err != nil { + return fmt.Errorf("project %q not found", c.Project) + } + uq = uq.Set("project_id = ?", project.ID) + } + changed = true + } + if c.Color != nil { + uq = uq.Set("color = ?", *c.Color) + changed = true + } + if c.Description != "" { + uq = uq.Set("description = ?", c.Description) + changed = true + } + if c.Rename != "" { + uq = uq.Set("name = ?", c.Rename) + changed = true + } + + if !changed { + fmt.Println("Nothing to update.") + return nil + } + + if _, err := uq.Exec(ctx); err != nil { + return err + } + + fmt.Printf("Updated label %q\n", c.Name) + return nil +} + +type LabelDeleteCmd struct { + Force bool `short:"f" help:"Skip confirmation."` + Name string `arg:"" help:"Label name to delete."` +} + +func (c *LabelDeleteCmd) Run(ctx *pw.Context) error { + var label db.Label + err := ctx.DB.NewSelect().Model(&label). + Where("name = ?", c.Name). + Scan(ctx) + if err != nil { + return fmt.Errorf("label %q not found", c.Name) + } + + if !c.Force { + fmt.Printf("Delete label %q (id=%d)? [y/N] ", label.Name, label.ID) + reader := bufio.NewReader(os.Stdin) + answer, _ := reader.ReadString('\n') + answer = strings.TrimSpace(strings.ToLower(answer)) + if answer != "y" && answer != "yes" { + fmt.Println("Aborted.") + return nil + } + } + + _, err = ctx.DB.NewDelete().Model((*db.Label)(nil)). + Where("id = ?", label.ID). + Exec(ctx) + if err != nil { + return err + } + + fmt.Printf("Deleted label %q\n", c.Name) + return nil +} + +type RelabelCmd struct { + Projects []string `arg:"" optional:"" help:"Project listIDs to relabel (all if omitted)."` +} + +func (c *RelabelCmd) Run(ctx *pw.Context) error { + var labels []db.Label + if err := ctx.DB.NewSelect().Model(&labels).Scan(ctx); err != nil { + return err + } + if len(labels) == 0 { + fmt.Println("No labels defined.") + return nil + } + + type labelKey struct { + name string + projectID int + } + labelMap := make(map[labelKey]*db.Label) + for i := range labels { + pid := 0 + if labels[i].ProjectID != nil { + pid = *labels[i].ProjectID + } + labelMap[labelKey{strings.ToLower(labels[i].Name), pid}] = &labels[i] + } + + findLabel := func(name string, projectID int) *db.Label { + if l, ok := labelMap[labelKey{strings.ToLower(name), projectID}]; ok { + return l + } + return labelMap[labelKey{strings.ToLower(name), 0}] + } + + insertLabel, err := ctx.DB.PrepareContext(ctx, + "INSERT INTO patch_label (patch_id, label_id) VALUES (?, ?) ON CONFLICT (patch_id, label_id) DO NOTHING") + if err != nil { + return err + } + defer insertLabel.Close() + + updateName, err := ctx.DB.PrepareContext(ctx, + "UPDATE patch SET name = ? WHERE id = ?") + if err != nil { + return err + } + defer updateName.Close() + + const batchSize = 1000 + lastID := 0 + total := 0 + updated := 0 + + for { + sq := ctx.DB.NewSelect().Model((*db.Patch)(nil)). + Column("id", "name", "headers", "project_id"). + Where("id > ?", lastID). + OrderExpr("id ASC"). + Limit(batchSize) + if len(c.Projects) > 0 { + sq = sq.Where( + "project_id IN (SELECT id FROM project WHERE listid IN ?)", + bun.Tuple(c.Projects), + ) + } + + var patches []db.Patch + if err := sq.Scan(ctx, &patches); err != nil { + return err + } + if len(patches) == 0 { + break + } + + for _, patch := range patches { + lastID = patch.ID + total++ + + subject := mail.ParseSubjectFromHeaders(patch.Headers) + if subject == "" { + continue + } + + _, prefixes := mail.CleanSubject(subject, nil) + if len(prefixes) == 0 { + continue + } + + var matched []db.Label + var remaining []string + for _, pfx := range prefixes { + if l := findLabel(pfx, patch.ProjectID); l != nil { + matched = append(matched, *l) + } else { + remaining = append(remaining, pfx) + } + } + if len(matched) == 0 { + continue + } + + newName := mail.RebuildSubject( + mail.StripPrefixes(patch.Name), remaining, + ) + if newName != patch.Name { + if _, err := updateName.ExecContext(ctx, newName, patch.ID); err != nil { + log.Warnf("update patch %d name: %v", patch.ID, err) + } + } + + for _, l := range matched { + if _, err := insertLabel.ExecContext(ctx, patch.ID, l.ID); err != nil { + log.Warnf("set label for patch %d: %v", patch.ID, err) + } + } + updated++ + } + + fmt.Printf("\rprocessed %d patches, %d updated", total, updated) + } + + fmt.Printf("\rprocessed %d patches, %d updated\n", total, updated) + return nil +} diff --git a/cmd/pw/admin/main.go b/cmd/pw/admin/main.go index d3dfa5367f82..a0ceb87c9a84 100644 --- a/cmd/pw/admin/main.go +++ b/cmd/pw/admin/main.go @@ -9,6 +9,7 @@ type CLI struct { Project ProjectCmd `cmd:"" help:"Manage projects."` User UserCmd `cmd:"" help:"Manage users."` Tag TagCmd `cmd:"" help:"Manage tags."` + Label LabelCmd `cmd:"" help:"Manage labels."` State StateCmd `cmd:"" help:"Manage states."` Maintainer MaintainerCmd `cmd:"" help:"Manage project maintainers."` DelegateRule DelegateRuleCmd `cmd:"" help:"Manage delegation rules."` diff --git a/docs/deployment/management.rst b/docs/deployment/management.rst index b357658b3c47..d87ce04462c3 100644 --- a/docs/deployment/management.rst +++ b/docs/deployment/management.rst @@ -344,6 +344,95 @@ Delete a tag. Skip confirmation. +Labels +------ + +``pw admin label list`` +~~~~~~~~~~~~~~~~~~~~~~~ + +.. program:: pw admin label list + +List all labels. + +``pw admin label create -n <name>`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. program:: pw admin label create + +Create a new label. + +.. option:: -n, --name <name> + + Label name (required). + +.. option:: -p, --project <linkname> + + Project linkname. When omitted, the label is global. + +.. option:: -d, --description <text> + + Label description. + +.. option:: -c, --color <number> + + Color as integer (e.g. ``0x00bcd4`` for cyan). Default: ``0``. + +``pw admin label update <name>`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. program:: pw admin label update + +Update an existing label. + +.. option:: <name> + + Label name to update. + +.. option:: -p, --project <linkname> + + Move the label to this project. Use ``global`` to make it global. + +.. option:: -c, --color <number> + + New color as integer. + +.. option:: -d, --description <text> + + New description. + +.. option:: -r, --rename <name> + + Rename the label. + +``pw admin label delete <name>`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. program:: pw admin label delete + +Delete a label. + +.. option:: <name> + + Label name to delete. + +.. option:: -f, --force + + Skip confirmation. + +``pw admin label relabel [<project>...]`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +.. program:: pw admin label relabel + +Re-parse all patch subjects and assign labels based on subject prefixes. +Matched prefixes are stripped from patch names. Labels must already exist +before running this command. + +.. option:: <project> + + Project listIDs to process. When omitted, all patches are processed. + + Maintainers ----------- diff --git a/docs/usage/overview.rst b/docs/usage/overview.rst index 65a454abcc3b..7939412dbd29 100644 --- a/docs/usage/overview.rst +++ b/docs/usage/overview.rst @@ -123,6 +123,28 @@ one delegate can be assigned to a patch. :doc:`delegation` for more information. +Labels +~~~~~~ + +Labels are arbitrary bits of metadata attached to a patch. They can be used to +signify priority, category, or other similar information. Labels can be +associated with a project or be global (shared across all projects). Global +labels are useful for things common to many projects, such as "RFC". + +When a patch is received, Patchwork automatically matches subject prefixes +(the text inside ``[brackets]``) against known label names. Matched prefixes +are stripped from the patch name and recorded as labels instead. For example, +a patch with subject ``[PATCH RFC v2 1/3] fix something`` with an "RFC" label +defined would be stored with name ``[v2,1/3] fix something`` and the label +"RFC" attached. + +Labels are managed through the ``pw admin label`` commands. The ``relabel`` +subcommand can retroactively apply labels to existing patches. + +Labels are displayed with colored badges in the web interface and included in +REST API responses for patches (since API version 1.4). + + Tags ~~~~ diff --git a/pkg/api/label_test.go b/pkg/api/label_test.go new file mode 100644 index 000000000000..75970e6df81e --- /dev/null +++ b/pkg/api/label_test.go @@ -0,0 +1,154 @@ +// Patchwork - automated patch tracking system +// Copyright (C) The Patchwork Contributors (see CONTRIBUTORS) +// +// SPDX-License-Identifier: GPL-2.0-or-later + +package api + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func (s *testServer) insertLabel(t *testing.T, name string, projectID *int, color int) int { + t.Helper() + var id int + if projectID != nil { + s.db.NewRaw(` + INSERT INTO label (name, description, color, project_id) + VALUES (?, '', ?, ?) + RETURNING id + `, name, color, *projectID).Scan(t.Context(), &id) + } else { + s.db.NewRaw(` + INSERT INTO label (name, description, color) + VALUES (?, '', ?) + RETURNING id + `, name, color).Scan(t.Context(), &id) + } + return id +} + +func (s *testServer) addPatchLabel(t *testing.T, patchID, labelID int) { + t.Helper() + s.exec(t, ` + INSERT INTO patch_label (patch_id, label_id) + VALUES (?, ?) + `, patchID, labelID) +} + +func TestPatchLabelsInList(t *testing.T) { + s := newTestServer(t) + projID := s.insertProject(t) + patchID := s.insertPatch(t, projID, "<lab-list@test>", "labeled patch") + labelID := s.insertLabel(t, "RFC", nil, 0x0097a7) + s.addPatchLabel(t, patchID, labelID) + + items := getList(t, s, "/api/1.4/patches") + require.Len(t, items, 1) + labels, ok := items[0]["labels"].([]any) + require.True(t, ok, "labels field should be an array") + require.Len(t, labels, 1) + assert.Equal(t, "RFC", labels[0]) +} + +func TestPatchLabelsInDetail(t *testing.T) { + s := newTestServer(t) + projID := s.insertProject(t) + patchID := s.insertPatch(t, projID, "<lab-det@test>", "labeled detail") + labelID := s.insertLabel(t, "RFC", nil, 0x0097a7) + s.addPatchLabel(t, patchID, labelID) + + p := getOne(t, s, fmt.Sprintf("/api/1.4/patches/%d", patchID)) + labels, ok := p["labels"].([]any) + require.True(t, ok, "labels field should be an array") + require.Len(t, labels, 1) + assert.Equal(t, "RFC", labels[0]) +} + +func TestPatchLabelsEmpty(t *testing.T) { + s := newTestServer(t) + projID := s.insertProject(t) + s.insertPatch(t, projID, "<no-lab@test>", "no labels") + + items := getList(t, s, "/api/1.4/patches") + require.Len(t, items, 1) + labels, ok := items[0]["labels"].([]any) + require.True(t, ok, "labels field should be an array") + assert.Empty(t, labels) +} + +func TestPatchLabelsMultiple(t *testing.T) { + s := newTestServer(t) + projID := s.insertProject(t) + patchID := s.insertPatch(t, projID, "<multi-lab@test>", "multi labels") + l1 := s.insertLabel(t, "RFC", nil, 0x0097a7) + l2 := s.insertLabel(t, "WIP", &projID, 0xff9800) + s.addPatchLabel(t, patchID, l1) + s.addPatchLabel(t, patchID, l2) + + p := getOne(t, s, fmt.Sprintf("/api/1.4/patches/%d", patchID)) + labels, ok := p["labels"].([]any) + require.True(t, ok) + require.Len(t, labels, 2) + assert.Equal(t, "RFC", labels[0]) + assert.Equal(t, "WIP", labels[1]) +} + +func TestPatchFilterByLabel(t *testing.T) { + s := newTestServer(t) + projID := s.insertProject(t) + p1 := s.insertPatch(t, projID, "<fl1@test>", "rfc patch") + p2 := s.insertPatch(t, projID, "<fl2@test>", "normal patch") + labelID := s.insertLabel(t, "RFC", nil, 0x0097a7) + s.addPatchLabel(t, p1, labelID) + _ = p2 + + items := getList(t, s, "/api/1.4/patches?labels=RFC") + require.Len(t, items, 1) + assert.Equal(t, "rfc patch", items[0]["name"]) +} + +func TestPatchFilterByLabelExclude(t *testing.T) { + s := newTestServer(t) + projID := s.insertProject(t) + p1 := s.insertPatch(t, projID, "<fle1@test>", "rfc patch") + s.insertPatch(t, projID, "<fle2@test>", "normal patch") + labelID := s.insertLabel(t, "RFC", nil, 0x0097a7) + s.addPatchLabel(t, p1, labelID) + + items := getList(t, s, "/api/1.4/patches?labels=-RFC") + require.Len(t, items, 1) + assert.Equal(t, "normal patch", items[0]["name"]) +} + +func TestPatchFilterByMultipleLabels(t *testing.T) { + s := newTestServer(t) + projID := s.insertProject(t) + p1 := s.insertPatch(t, projID, "<fml1@test>", "both labels") + p2 := s.insertPatch(t, projID, "<fml2@test>", "one label") + l1 := s.insertLabel(t, "RFC", nil, 0x0097a7) + l2 := s.insertLabel(t, "WIP", nil, 0xff9800) + s.addPatchLabel(t, p1, l1) + s.addPatchLabel(t, p1, l2) + s.addPatchLabel(t, p2, l1) + + items := getList(t, s, "/api/1.4/patches?labels=RFC,WIP") + require.Len(t, items, 1) + assert.Equal(t, "both labels", items[0]["name"]) +} + +func TestPatchLabelsNotInOlderAPI(t *testing.T) { + s := newTestServer(t) + projID := s.insertProject(t) + patchID := s.insertPatch(t, projID, "<old-api@test>", "old api") + labelID := s.insertLabel(t, "RFC", nil, 0x0097a7) + s.addPatchLabel(t, patchID, labelID) + + p := getOne(t, s, fmt.Sprintf("/api/1.3/patches/%d", patchID)) + _, hasLabels := p["labels"] + assert.False(t, hasLabels, "labels should not appear in API 1.3") +} diff --git a/pkg/api/patches.go b/pkg/api/patches.go index 79cac6b091d9..3493ce16de22 100644 --- a/pkg/api/patches.go +++ b/pkg/api/patches.go @@ -10,6 +10,7 @@ import ( "fmt" "net/http" "strconv" + "strings" "time" "github.com/danielgtaylor/huma/v2" @@ -58,6 +59,7 @@ type ListPatchesInput struct { Msgid string `query:"msgid" doc:"Message ID"` Since string `query:"since" doc:"Earliest date"` Before string `query:"before" doc:"Latest date"` + Labels string `query:"labels" doc:"Comma-separated label names (prefix with - to exclude)"` } type ListPatchesOutput struct { @@ -307,6 +309,9 @@ func loadPatchDetails(q *db.Queries, patches []db.Patch) error { if err := q.LoadPatchRelated(patches); err != nil { return err } + if err := q.LoadPatchLabels(patches); err != nil { + return err + } return nil } @@ -360,9 +365,28 @@ func applyPatchFilters(q *bun.SelectQuery, input *ListPatchesInput) *bun.SelectQ if input.Q != "" { q = q.Where("patch.name LIKE ?", "%"+input.Q+"%") } + if input.Labels != "" { + q = applyPatchLabelsFilter(q, input.Labels) + } return q } +func applyPatchLabelsFilter(q *bun.SelectQuery, labels string) *bun.SelectQuery { + var include, exclude []string + for _, name := range strings.Split(labels, ",") { + name = strings.TrimSpace(name) + if name == "" { + continue + } + if strings.HasPrefix(name, "-") { + exclude = append(exclude, name[1:]) + } else { + include = append(include, name) + } + } + return db.FilterPatchLabels(q, include, exclude) +} + func patchToListResponse(p *db.Patch, base string) PatchListResponse { r := PatchListResponse{ ID: p.ID, @@ -425,6 +449,8 @@ func patchToListResponse(p *db.Patch, base string) PatchListResponse { if r.Series == nil { r.Series = []SeriesEmbedded{} } + names := labelNames(p.Labels) + r.Labels = &names return r } diff --git a/pkg/api/types.go b/pkg/api/types.go index c017d45c2ce2..fc80672c8f82 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -129,6 +129,7 @@ type PatchListResponse struct { Checks string `json:"checks" format:"uri"` Tags map[string]int `json:"tags"` Related []PatchEmbedded `json:"related" since:"1.2"` + Labels *[]string `json:"labels,omitempty" since:"1.4"` } type PatchDetailResponse struct { diff --git a/pkg/api/util.go b/pkg/api/util.go index 2f99be16e9d7..a37ca63a57fc 100644 --- a/pkg/api/util.go +++ b/pkg/api/util.go @@ -11,11 +11,11 @@ import ( "net/url" "strings" - "github.com/emersion/go-message/mail" "github.com/uptrace/bun" "github.com/getpatchwork/patchwork/pkg/db" "github.com/getpatchwork/patchwork/pkg/log" + "github.com/getpatchwork/patchwork/pkg/mail" ) func strp(s string) *string { return &s } @@ -45,6 +45,14 @@ func personToEmbedded(p *db.Person, base string) PersonEmbedded { } } +func labelNames(labels []db.Label) []string { + names := make([]string, len(labels)) + for i, l := range labels { + names[i] = l.Name + } + return names +} + func loadSeriesDetail(ctx context.Context, database bun.IDB, base string, series []db.Series) { for i := range series { s := &series[i] @@ -177,19 +185,7 @@ func parseHeadersMap(raw string) map[string]string { } func parseSubjectFromHeaders(headers string) string { - if headers == "" { - return "" - } - raw := strings.ReplaceAll(headers, "\n", "\r\n") - if !strings.HasSuffix(raw, "\r\n\r\n") { - raw += "\r\n" - } - m, err := mail.CreateReader(strings.NewReader(raw)) - if err != nil { - return "" - } - subject, _ := m.Header.Subject() - return subject + return mail.ParseSubjectFromHeaders(headers) } func listArchiveURL(project *db.Project, msgid string) string { diff --git a/pkg/db/label.go b/pkg/db/label.go new file mode 100644 index 000000000000..4a5e26a850a5 --- /dev/null +++ b/pkg/db/label.go @@ -0,0 +1,116 @@ +// Patchwork - automated patch tracking system +// Copyright (C) The Patchwork Contributors (see CONTRIBUTORS) +// +// SPDX-License-Identifier: GPL-2.0-or-later + +package db + +import ( + "strings" + + "github.com/uptrace/bun" +) + +func (q *Queries) ListProjectLabels(projectID int) ([]Label, error) { + var labels []Label + err := q.DB.NewSelect().Model(&labels). + WhereGroup(" AND ", func(sq *bun.SelectQuery) *bun.SelectQuery { + return sq.Where("project_id = ?", projectID). + WhereOr("project_id IS NULL") + }). + OrderExpr("name ASC"). + Scan(q.Ctx) + return labels, err +} + +func (q *Queries) FindLabelsByName(projectID int, names []string) ([]Label, error) { + var labels []Label + lower := make([]string, len(names)) + for i, n := range names { + lower[i] = strings.ToLower(n) + } + err := q.DB.NewSelect().Model(&labels). + Where("LOWER(name) IN ?", bun.Tuple(lower)). + WhereGroup(" AND ", func(sq *bun.SelectQuery) *bun.SelectQuery { + return sq.Where("project_id = ?", projectID). + WhereOr("project_id IS NULL") + }). + Scan(q.Ctx) + return labels, err +} + +func (q *Queries) SetPatchLabels(patchID int, labelIDs []int) error { + for _, labelID := range labelIDs { + pl := PatchLabel{PatchID: patchID, LabelID: labelID} + _, err := q.DB.NewInsert().Model(&pl). + On("CONFLICT (patch_id, label_id) DO NOTHING"). + Exec(q.Ctx) + if err != nil { + return err + } + } + return nil +} + +func FilterPatchLabels( + q *bun.SelectQuery, include, exclude []string, +) *bun.SelectQuery { + if len(exclude) > 0 { + sub := q.NewSelect(). + Model((*PatchLabel)(nil)). + Column("patch_id"). + Join("JOIN label ON label.id = patch_label.label_id"). + Where("label.name IN ?", bun.Tuple(exclude)) + q = q.Where("patch.id NOT IN (?)", sub) + } + if len(include) > 0 { + sub := q.NewSelect(). + Model((*PatchLabel)(nil)). + Column("patch_id"). + Join("JOIN label ON label.id = patch_label.label_id"). + Where("label.name IN ?", bun.Tuple(include)). + GroupExpr("patch_id"). + Having("COUNT(DISTINCT label.name) >= ?", len(include)) + q = q.Where("patch.id IN (?)", sub) + } + return q +} + +func (q *Queries) LoadPatchLabels(patches []Patch) error { + if len(patches) == 0 { + return nil + } + + ids := make([]int, len(patches)) + byId := make(map[int]*Patch, len(patches)) + for i := range patches { + p := &patches[i] + ids[i] = p.ID + byId[p.ID] = p + } + + type labelRow struct { + PatchID int `bun:"patch_id"` + Name string `bun:"name"` + Color int `bun:"color"` + } + var labelRows []labelRow + if err := q.DB.NewSelect().Model((*PatchLabel)(nil)). + ColumnExpr("patch_id, label.name, label.color"). + Join("JOIN label ON label_id = label.id"). + Where("patch_id IN ?", bun.Tuple(ids)). + OrderExpr("label.name ASC"). + Scan(q.Ctx, &labelRows); err != nil { + return err + } + + for _, r := range labelRows { + if p, ok := byId[r.PatchID]; ok { + p.Labels = append(p.Labels, Label{ + Name: r.Name, + Color: r.Color, + }) + } + } + return nil +} diff --git a/pkg/db/migrations/0004_labels.go b/pkg/db/migrations/0004_labels.go new file mode 100644 index 000000000000..5790b8074025 --- /dev/null +++ b/pkg/db/migrations/0004_labels.go @@ -0,0 +1,60 @@ +// Patchwork - automated patch tracking system +// Copyright (C) The Patchwork Contributors (see CONTRIBUTORS) +// +// SPDX-License-Identifier: GPL-2.0-or-later + +package migrations + +import ( + "context" + + "github.com/uptrace/bun" + + "github.com/getpatchwork/patchwork/pkg/db" +) + +type label0004 struct { + bun.BaseModel `bun:"table:label" unique:"project_id,name"` + + ID int `bun:"id,pk,autoincrement"` + ProjectID *int `bun:"project_id" fk:"project.id,cascade"` + Name string `bun:"name,notnull"` + Description string `bun:"description,notnull"` + Color int `bun:"color,notnull"` +} + +type patchLabel0004 struct { + bun.BaseModel `bun:"table:patch_label" unique:"patch_id,label_id"` + + ID int `bun:"id,pk,autoincrement"` + PatchID int `bun:"patch_id,notnull" fk:"patch.id,cascade"` + LabelID int `bun:"label_id,notnull" fk:"label.id,cascade"` +} + +func init() { + Register(up0004, down0004) +} + +func up0004(ctx context.Context, tx bun.Tx) error { + if err := db.CreateSchemaFrom(ctx, tx, []any{ + (*label0004)(nil), + (*patchLabel0004)(nil), + }); err != nil { + return err + } + _, err := tx.NewInsert().Model(&label0004{ + Name: "RFC", + Description: "Request for comments", + Color: 0x0097a7, + }).On("CONFLICT DO NOTHING").Exec(ctx) + return err +} + +func down0004(ctx context.Context, tx bun.Tx) error { + for _, table := range []string{"patch_label", "label"} { + if _, err := tx.NewDropTable().Table(table).IfExists().Exec(ctx); err != nil { + return err + } + } + return nil +} diff --git a/pkg/db/models.go b/pkg/db/models.go index bb1bc0c53713..f8e1ed8c0a9e 100644 --- a/pkg/db/models.go +++ b/pkg/db/models.go @@ -7,6 +7,7 @@ package db import ( "encoding/json" + "fmt" "time" "github.com/uptrace/bun" @@ -128,6 +129,43 @@ type State struct { ActionRequired bool `bun:"action_required,notnull" json:"action_required"` } +type Label struct { + bun.BaseModel `bun:"table:label" unique:"project_id,name" json:"-"` + + ID int `bun:"id,pk,autoincrement" json:"id"` + ProjectID *int `bun:"project_id" json:"-" fk:"project.id,cascade"` + Name string `bun:"name,notnull" json:"name"` + Description string `bun:"description,notnull" json:"description,omitempty"` + Color int `bun:"color,notnull" json:"-"` +} + +func (l Label) MarshalJSON() ([]byte, error) { + return json.Marshal(l.Name) +} + +func (l *Label) ColorHex() string { + return fmt.Sprintf("#%06x", l.Color) +} + +func (l *Label) TextColor() string { + r := (l.Color >> 16) & 0xff + g := (l.Color >> 8) & 0xff + b := l.Color & 0xff + luminance := 0.299*float64(r) + 0.587*float64(g) + 0.114*float64(b) + if luminance > 128 { + return "#000" + } + return "#fff" +} + +type PatchLabel struct { + bun.BaseModel `bun:"table:patch_label" unique:"patch_id,label_id" json:"-"` + + ID int `bun:"id,pk,autoincrement"` + PatchID int `bun:"patch_id,notnull" fk:"patch.id,cascade"` + LabelID int `bun:"label_id,notnull" fk:"label.id,cascade"` +} + type Tag struct { bun.BaseModel `bun:"table:tag" json:"-"` @@ -244,6 +282,7 @@ type Patch struct { CheckCounts [4]int `bun:"-" json:"-"` Tags map[string]int `bun:"-" json:"tags"` SeriesList []SeriesRef `bun:"-" json:"series"` + Labels []Label `bun:"-" json:"labels"` } type PatchTag struct { diff --git a/pkg/db/schema.go b/pkg/db/schema.go index 52e709302763..b2edf4d214b6 100644 --- a/pkg/db/schema.go +++ b/pkg/db/schema.go @@ -170,6 +170,7 @@ func CreateSchema(ctx context.Context, database bun.IDB) error { (*Project)(nil), (*ProjectMaintainer)(nil), (*DelegationRule)(nil), + (*Label)(nil), (*Person)(nil), (*PatchRelation)(nil), (*Cover)(nil), @@ -180,6 +181,7 @@ func CreateSchema(ctx context.Context, database bun.IDB) error { (*Patch)(nil), (*PatchTag)(nil), (*PatchComment)(nil), + (*PatchLabel)(nil), (*CoverComment)(nil), (*Check)(nil), (*Bundle)(nil), diff --git a/pkg/db/seed.go b/pkg/db/seed.go index 40f9fc894e22..0cfafd627f20 100644 --- a/pkg/db/seed.go +++ b/pkg/db/seed.go @@ -49,5 +49,18 @@ func SeedDefaults(ctx context.Context, database bun.IDB) error { } } + labels := []Label{ + {Name: "RFC", Color: 0x0097a7, Description: "Request for comments"}, + } + for i := range labels { + _, err := database.NewInsert(). + Model(&labels[i]). + On("CONFLICT DO NOTHING"). + Exec(ctx) + if err != nil { + return err + } + } + return nil } diff --git a/pkg/mail/cover.go b/pkg/mail/cover.go index 876586e62a37..a799f6b6241b 100644 --- a/pkg/mail/cover.go +++ b/pkg/mail/cover.go @@ -76,7 +76,7 @@ func (p *parser) handleCoverLetter() error { s.ID, db.Ptr[int](cover.ID), ) - coverName := stripPrefixes(p.subject) + coverName := StripPrefixes(p.subject) if s.Name == nil { _ = p.db.UpdateSeriesName(s.ID, db.Ptr(coverName)) } else { diff --git a/pkg/mail/headers.go b/pkg/mail/headers.go index 3da553a4beb0..c6d3d82dfadb 100644 --- a/pkg/mail/headers.go +++ b/pkg/mail/headers.go @@ -206,7 +206,23 @@ func ParsePullRequest(content string) string { return strings.TrimSpace(spaceRe.ReplaceAllString(m[1], " ")) } -func stripPrefixes(name string) string { +func ParseSubjectFromHeaders(headers string) string { + if headers == "" { + return "" + } + raw := strings.ReplaceAll(headers, "\n", "\r\n") + if !strings.HasSuffix(raw, "\r\n\r\n") { + raw += "\r\n" + } + m, err := mail.CreateReader(strings.NewReader(raw)) + if err != nil { + return "" + } + subject, _ := m.Header.Subject() + return subject +} + +func StripPrefixes(name string) string { for { m := prefixRe.FindStringSubmatch(name) if m == nil { diff --git a/pkg/mail/parser.go b/pkg/mail/parser.go index 6f405cde8d6e..2b259a52fbb8 100644 --- a/pkg/mail/parser.go +++ b/pkg/mail/parser.go @@ -37,6 +37,7 @@ type parser struct { from *mail.Address prefixes []string + labels []db.Label subject string listid string date time.Time @@ -117,6 +118,8 @@ func ParseMail(ctx context.Context, database *bun.DB, r io.Reader, listid ...str p.version = ParseVersion(p.subject, p.prefixes) p.refs = FindReferences(&m.Header) + p.matchLabels() + log.Debugf("series marker: n=%d total=%d version=%d comment=%v refs=%v", p.number, p.total, p.version, isComment, p.refs) @@ -156,6 +159,50 @@ func ParseMail(ctx context.Context, database *bun.DB, r io.Reader, listid ...str return nil } +func (p *parser) matchLabels() { + if len(p.prefixes) == 0 { + return + } + labels, err := p.db.FindLabelsByName(p.project.ID, p.prefixes) + if err != nil || len(labels) == 0 { + return + } + p.labels = labels + + matched := make(map[string]bool) + for _, l := range labels { + matched[strings.ToLower(l.Name)] = true + } + var remaining []string + for _, pfx := range p.prefixes { + if !matched[strings.ToLower(pfx)] { + remaining = append(remaining, pfx) + } + } + p.prefixes = remaining + p.subject = RebuildSubject(StripPrefixes(p.subject), remaining) +} + +func RebuildSubject(name string, prefixes []string) string { + if len(prefixes) > 0 { + return fmt.Sprintf("[%s] %s", strings.Join(prefixes, ","), name) + } + return name +} + +func (p *parser) assignLabels() { + if len(p.labels) == 0 || p.patch == nil { + return + } + ids := make([]int, len(p.labels)) + for i, l := range p.labels { + ids[i] = l.ID + } + if err := p.db.SetPatchLabels(p.patch.ID, ids); err != nil { + log.Warnf("set patch labels: %v", err) + } +} + func (p *parser) parseSeriesMarker(isComment bool) { p.number, p.total = ParseSeriesMarker(p.prefixes) if p.number == 0 && p.total == 0 && !isComment { diff --git a/pkg/mail/parser_test.go b/pkg/mail/parser_test.go index 3d3128d74011..09d660198a6c 100644 --- a/pkg/mail/parser_test.go +++ b/pkg/mail/parser_test.go @@ -438,6 +438,117 @@ func TestFindMessageIDInvalidFallback(t *testing.T) { strings.NewReader(data), "test.example.com") } +func TestLabelExtraction(t *testing.T) { + database, ctx, _, _ := testDB(t, "test.example.com") + + t.Run("label stripped from patch name", func(t *testing.T) { + err := parseEmail(t, ctx, database, sampleDiff, + withSubject("[PATCH RFC] fix something"), + withMsgID("<label-strip@test>"), + withListID("test.example.com")) + require.NoError(t, err) + + var name string + database.NewSelect().TableExpr("patch"). + Column("name").Where("msgid = ?", "<label-strip@test>"). + Scan(context.Background(), &name) + assert.Equal(t, "fix something", name) + }) + + t.Run("label association created", func(t *testing.T) { + var count int + database.NewRaw(` + SELECT count(*) FROM patch_label pl + JOIN patch p ON p.id = pl.patch_id + WHERE p.msgid = ? + `, "<label-strip@test>").Scan(context.Background(), &count) + assert.Equal(t, 1, count) + }) + + t.Run("non-label prefixes preserved", func(t *testing.T) { + err := parseEmail(t, ctx, database, sampleDiff, + withSubject("[PATCH RFC v2 1/3] another fix"), + withMsgID("<label-keep@test>"), + withListID("test.example.com")) + require.NoError(t, err) + + var name string + database.NewSelect().TableExpr("patch"). + Column("name").Where("msgid = ?", "<label-keep@test>"). + Scan(context.Background(), &name) + assert.Equal(t, "[v2,1/3] another fix", name) + }) + + t.Run("no label match keeps all prefixes", func(t *testing.T) { + err := parseEmail(t, ctx, database, sampleDiff, + withSubject("[PATCH WIP] some change"), + withMsgID("<no-label@test>"), + withListID("test.example.com")) + require.NoError(t, err) + + var name string + database.NewSelect().TableExpr("patch"). + Column("name").Where("msgid = ?", "<no-label@test>"). + Scan(context.Background(), &name) + assert.Equal(t, "[WIP] some change", name) + + var count int + database.NewRaw(` + SELECT count(*) FROM patch_label pl + JOIN patch p ON p.id = pl.patch_id + WHERE p.msgid = ? + `, "<no-label@test>").Scan(context.Background(), &count) + assert.Equal(t, 0, count) + }) +} + +func TestLabelProjectScoped(t *testing.T) { + database, ctx, _, proj := testDB(t, "test.example.com") + + database.NewRaw(` + INSERT INTO label (name, description, color, project_id) + VALUES ('WIP', '', 0xff9800, ?) + `, proj.ID).Exec(context.Background()) + + t.Run("project label matched", func(t *testing.T) { + err := parseEmail(t, ctx, database, sampleDiff, + withSubject("[PATCH WIP] project label"), + withMsgID("<proj-label@test>"), + withListID("test.example.com")) + require.NoError(t, err) + + var name string + database.NewSelect().TableExpr("patch"). + Column("name").Where("msgid = ?", "<proj-label@test>"). + Scan(context.Background(), &name) + assert.Equal(t, "project label", name) + }) +} + +func TestLabelCaseInsensitive(t *testing.T) { + database, ctx, _, _ := testDB(t, "test.example.com") + + err := parseEmail(t, ctx, database, sampleDiff, + withSubject("[PATCH rfc] lowercase prefix"), + withMsgID("<label-case@test>"), + withListID("test.example.com")) + require.NoError(t, err) + + var name string + database.NewSelect().TableExpr("patch"). + Column("name").Where("msgid = ?", "<label-case@test>"). + Scan(context.Background(), &name) + assert.Equal(t, "lowercase prefix", name) + + var count int + database.NewRaw(` + SELECT count(*) FROM patch_label pl + JOIN patch p ON p.id = pl.patch_id + WHERE p.msgid = ? + `, "<label-case@test>").Scan(context.Background(), &count) + assert.Equal(t, 1, count) +} + func TestFindReferencesInvalidFallback(t *testing.T) { h := makeHeader(t, map[string]string{ "From": "[email protected]", diff --git a/pkg/mail/patch.go b/pkg/mail/patch.go index 7523954af9b0..4559f1a42e2b 100644 --- a/pkg/mail/patch.go +++ b/pkg/mail/patch.go @@ -51,6 +51,7 @@ func (p *parser) handlePatch() error { } _ = p.db.RefreshTagCounts(p.patch) + p.assignLabels() log.Infof("patch saved id=%d msgid=%s", p.patch.ID, p.msgid) p.createPatchCreatedEvent() diff --git a/pkg/mail/series.go b/pkg/mail/series.go index 4873091c1bbc..f271d0abf8d9 100644 --- a/pkg/mail/series.go +++ b/pkg/mail/series.go @@ -232,7 +232,7 @@ func (p *parser) linkPreviousSeries() { if p.series.Name == nil { return } - seriesName := stripPrefixes(*p.series.Name) + seriesName := StripPrefixes(*p.series.Name) if seriesName == "" { return } @@ -243,7 +243,7 @@ func (p *parser) linkPreviousSeries() { if candidates[i].Name == nil { continue } - candName := stripPrefixes(*candidates[i].Name) + candName := StripPrefixes(*candidates[i].Name) if candName == "" { continue } diff --git a/pkg/mail/similarity.go b/pkg/mail/similarity.go index 8b30f84c0ce1..f1527800b73a 100644 --- a/pkg/mail/similarity.go +++ b/pkg/mail/similarity.go @@ -8,8 +8,8 @@ package mail import "strings" func nameSimilarity(a, b string) float64 { - a = strings.ToLower(stripPrefixes(a)) - b = strings.ToLower(stripPrefixes(b)) + a = strings.ToLower(StripPrefixes(a)) + b = strings.ToLower(StripPrefixes(b)) if a == "" || b == "" { return 0.0 } diff --git a/pkg/web/filters.go b/pkg/web/filters.go index 6d156e35539a..2df92ac2ad43 100644 --- a/pkg/web/filters.go +++ b/pkg/web/filters.go @@ -10,6 +10,7 @@ import ( "fmt" "net/url" "strconv" + "strings" "github.com/uptrace/bun" @@ -111,6 +112,27 @@ func applyWebFilters(ctx context.Context, database bun.IDB, q *bun.SelectQuery, } } + if v := params.Get("labels"); v != "" { + var include, exclude []string + for _, name := range strings.Split(v, " ") { + name = strings.TrimSpace(name) + if name == "" { + continue + } + if strings.HasPrefix(name, "-") { + exclude = append(exclude, name[1:]) + } else { + include = append(include, name) + } + } + q = db.FilterPatchLabels(q, include, exclude) + filters = append(filters, appliedFilter{ + Label: "Labels", + Value: v, + RemoveURL: removeParam(basePath, params, "labels"), + }) + } + return q, filters } diff --git a/pkg/web/patch.templ b/pkg/web/patch.templ index 1c4cb12fd26c..73075b9ae785 100644 --- a/pkg/web/patch.templ +++ b/pkg/web/patch.templ @@ -129,6 +129,16 @@ templ patchDetailPage(d patchDetailData) { } </td> </tr> + if len(d.Patch.Labels) > 0 { + <tr> + <th>Labels</th> + <td> + for _, l := range d.Patch.Labels { + @labelBadge(d.Project.Linkname, l) + } + </td> + </tr> + } <tr> <th>Headers</th> <td> diff --git a/pkg/web/patches.go b/pkg/web/patches.go index 09f3111fd55c..28118b692a97 100644 --- a/pkg/web/patches.go +++ b/pkg/web/patches.go @@ -152,6 +152,12 @@ func (h *webHandler) PatchList(w http.ResponseWriter, r *http.Request) { return } + labels, err := q.ListProjectLabels(project.ID) + if err != nil { + serverErrorPage(w, "list labels", err) + return + } + data := patchListData{ PC: h.projectPageCtx(r, project), Project: *project, @@ -168,6 +174,7 @@ func (h *webHandler) PatchList(w http.ResponseWriter, r *http.Request) { Bundles: bundles, States: states, Delegates: delegates, + Labels: labels, } patchListPage(data).Render(ctx, w) } @@ -712,5 +719,8 @@ func loadWebPatchDetails(q *db.Queries, patches []db.Patch) error { if err := q.LoadPatchCheckCounts(patches); err != nil { return err } + if err := q.LoadPatchLabels(patches); err != nil { + return err + } return nil } diff --git a/pkg/web/patches.templ b/pkg/web/patches.templ index 0646d4070cbc..b03dcedab689 100644 --- a/pkg/web/patches.templ +++ b/pkg/web/patches.templ @@ -7,6 +7,7 @@ package web import ( "fmt" + "net/url" "strings" "github.com/getpatchwork/patchwork/pkg/db" @@ -28,6 +29,7 @@ type patchListData struct { Bundles []db.Bundle States []db.State Delegates []db.User + Labels []db.Label } type appliedFilter struct { @@ -81,6 +83,14 @@ templ patchListPage(d patchListData) { } </select> <input type="text" name="submitter" placeholder="Submitter" title="Submitter"/> + if len(d.Labels) > 0 { + <select name="labels" title="Label"> + <option value="">Label...</option> + for _, l := range d.Labels { + <option value={ l.Name }>{ l.Name }</option> + } + </select> + } <input type="text" name="q" placeholder="Search" title="Search"/> <button type="submit">Filter</button> </form> @@ -206,6 +216,9 @@ templ patchRow(d *patchListData, p *db.Patch) { <a href={ templ.SafeURL(patchURL(d.Project.Linkname, p.Msgid)) }> { truncate(p.Name, 100) } </a> + for _, l := range p.Labels { + @labelBadge(d.Project.Linkname, l) + } </td> <td class="col-series"> if p.SeriesID != nil { @@ -246,6 +259,16 @@ templ patchRow(d *patchListData, p *db.Patch) { </td> } +templ labelBadge(linkname string, l db.Label) { + <a + href={ templ.SafeURL(fmt.Sprintf("/project/%s/list/?labels=%s", linkname, url.QueryEscape(l.Name))) } + class="label-badge" + style={ fmt.Sprintf("background-color:%s;color:%s", l.ColorHex(), l.TextColor()) } + > + { l.Name } + </a> +} + templ tagCounts(abbrevs []string, tags map[string]int) { for _, a := range abbrevs { <pill> diff --git a/pkg/web/static/style.css b/pkg/web/static/style.css index 3e3fdd90561d..e2a81255faa8 100644 --- a/pkg/web/static/style.css +++ b/pkg/web/static/style.css @@ -364,6 +364,22 @@ pill.fail { background: #d9534f; } .check-warn { color: #f0ad4e; font-weight: bold; } .check-fail { color: #d9534f; font-weight: bold; } +/* --- label badges --- */ +.label-badge { + display: inline-block; + padding: 0 6px; + margin: 0 2px; + border-radius: 3px; + font-size: 0.85em; + font-weight: bold; + vertical-align: middle; + white-space: nowrap; + text-decoration: none; +} +a.label-badge:hover { + opacity: 0.8; +} + /* --- download links --- */ title-bar { display: flex; -- 2.55.0 _______________________________________________ Patchwork mailing list [email protected] https://lists.ozlabs.org/listinfo/patchwork
