Skip to content

Commit 62bc759

Browse files
Copilotpelikhan
andauthored
Make discussion categories case-insensitive (#14820)
* Initial plan * Implement case-insensitive discussion category normalization - Normalize discussion categories to lowercase in Go compiler - Add case-insensitive matching in JavaScript resolveCategoryId - Update tests to verify normalization behavior - Category IDs (DIC_*) remain unchanged Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent 8908ac8 commit 62bc759

4 files changed

Lines changed: 379 additions & 100 deletions

File tree

actions/setup/js/create_discussion.cjs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,18 +63,22 @@ function resolveCategoryId(categoryConfig, itemCategory, categories) {
6363
const categoryToMatch = itemCategory || categoryConfig;
6464

6565
if (categoryToMatch) {
66-
// Try to match against category IDs first
66+
// Try to match against category IDs first (exact match, case-sensitive)
6767
const categoryById = categories.find(cat => cat.id === categoryToMatch);
6868
if (categoryById) {
6969
return { id: categoryById.id, matchType: "id", name: categoryById.name };
7070
}
71-
// Try to match against category names
72-
const categoryByName = categories.find(cat => cat.name === categoryToMatch);
71+
72+
// Normalize the category to match for case-insensitive comparison
73+
const normalizedCategoryToMatch = categoryToMatch.toLowerCase();
74+
75+
// Try to match against category names (case-insensitive)
76+
const categoryByName = categories.find(cat => cat.name.toLowerCase() === normalizedCategoryToMatch);
7377
if (categoryByName) {
7478
return { id: categoryByName.id, matchType: "name", name: categoryByName.name };
7579
}
76-
// Try to match against category slugs (routes)
77-
const categoryBySlug = categories.find(cat => cat.slug === categoryToMatch);
80+
// Try to match against category slugs (routes, case-insensitive)
81+
const categoryBySlug = categories.find(cat => cat.slug.toLowerCase() === normalizedCategoryToMatch);
7882
if (categoryBySlug) {
7983
return { id: categoryBySlug.id, matchType: "slug", name: categoryBySlug.name };
8084
}
Lines changed: 274 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,274 @@
1+
// @ts-check
2+
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
3+
import { createRequire } from "module";
4+
5+
const require = createRequire(import.meta.url);
6+
const { main: createDiscussionMain } = require("./create_discussion.cjs");
7+
8+
describe("create_discussion category normalization", () => {
9+
let mockGithub;
10+
let mockCore;
11+
let mockContext;
12+
let mockExec;
13+
let originalEnv;
14+
15+
beforeEach(() => {
16+
// Save original environment
17+
originalEnv = { ...process.env };
18+
19+
// Mock GitHub API with discussion categories
20+
mockGithub = {
21+
rest: {},
22+
graphql: vi.fn().mockImplementation((query, variables) => {
23+
// Handle repository query (fetch categories)
24+
if (query.includes("discussionCategories")) {
25+
return Promise.resolve({
26+
repository: {
27+
id: "R_test123",
28+
discussionCategories: {
29+
nodes: [
30+
{
31+
id: "DIC_kwDOGFsHUM4BsUn1",
32+
name: "General",
33+
slug: "general",
34+
description: "General discussions",
35+
},
36+
{
37+
id: "DIC_kwDOGFsHUM4BsUn2",
38+
name: "Audits",
39+
slug: "audits",
40+
description: "Audit reports",
41+
},
42+
{
43+
id: "DIC_kwDOGFsHUM4BsUn3",
44+
name: "Research",
45+
slug: "research",
46+
description: "Research discussions",
47+
},
48+
],
49+
},
50+
},
51+
});
52+
}
53+
// Handle create discussion mutation
54+
if (query.includes("createDiscussion")) {
55+
return Promise.resolve({
56+
createDiscussion: {
57+
discussion: {
58+
id: "D_test456",
59+
number: 42,
60+
title: variables.title,
61+
url: "https://github.com/test-owner/test-repo/discussions/42",
62+
},
63+
},
64+
});
65+
}
66+
return Promise.reject(new Error("Unknown GraphQL query"));
67+
}),
68+
};
69+
70+
// Mock Core
71+
mockCore = {
72+
info: vi.fn(),
73+
warning: vi.fn(),
74+
error: vi.fn(),
75+
setOutput: vi.fn(),
76+
};
77+
78+
// Mock Context
79+
mockContext = {
80+
repo: { owner: "test-owner", repo: "test-repo" },
81+
runId: 12345,
82+
payload: {
83+
repository: {
84+
html_url: "https://github.com/test-owner/test-repo",
85+
},
86+
},
87+
};
88+
89+
// Mock Exec
90+
mockExec = {
91+
exec: vi.fn().mockResolvedValue(0),
92+
};
93+
94+
// Set globals
95+
global.github = mockGithub;
96+
global.core = mockCore;
97+
global.context = mockContext;
98+
global.exec = mockExec;
99+
100+
// Set required environment variables
101+
process.env.GH_AW_WORKFLOW_NAME = "Test Workflow";
102+
process.env.GH_AW_WORKFLOW_ID = "test-workflow";
103+
process.env.GH_AW_WORKFLOW_SOURCE_URL = "https://github.com/owner/repo/blob/main/workflow.md";
104+
process.env.GITHUB_SERVER_URL = "https://github.com";
105+
});
106+
107+
afterEach(() => {
108+
// Restore environment
109+
process.env = originalEnv;
110+
vi.clearAllMocks();
111+
});
112+
113+
it("should match category name case-insensitively (lowercase config, capitalized repo)", async () => {
114+
const handler = await createDiscussionMain({
115+
max: 5,
116+
category: "audits", // lowercase config
117+
});
118+
119+
const result = await handler(
120+
{
121+
title: "Test Discussion",
122+
body: "This is a test discussion.",
123+
},
124+
{}
125+
);
126+
127+
expect(result.success).toBe(true);
128+
expect(result.number).toBe(42);
129+
130+
// Verify the correct category ID was used (Audits with capital A)
131+
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
132+
expect(createMutationCall).toBeDefined();
133+
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn2"); // Audits category
134+
});
135+
136+
it("should match category name case-insensitively (capitalized config, capitalized repo)", async () => {
137+
const handler = await createDiscussionMain({
138+
max: 5,
139+
category: "Audits", // Capitalized config (user error)
140+
});
141+
142+
const result = await handler(
143+
{
144+
title: "Test Discussion",
145+
body: "This is a test discussion.",
146+
},
147+
{}
148+
);
149+
150+
expect(result.success).toBe(true);
151+
expect(result.number).toBe(42);
152+
153+
// Verify the correct category ID was used
154+
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
155+
expect(createMutationCall).toBeDefined();
156+
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn2"); // Audits category
157+
});
158+
159+
it("should match category name case-insensitively (mixed case config)", async () => {
160+
const handler = await createDiscussionMain({
161+
max: 5,
162+
category: "AuDiTs", // Mixed case (should still match)
163+
});
164+
165+
const result = await handler(
166+
{
167+
title: "Test Discussion",
168+
body: "This is a test discussion.",
169+
},
170+
{}
171+
);
172+
173+
expect(result.success).toBe(true);
174+
expect(result.number).toBe(42);
175+
176+
// Verify the correct category ID was used
177+
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
178+
expect(createMutationCall).toBeDefined();
179+
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn2"); // Audits category
180+
});
181+
182+
it("should match category slug case-insensitively", async () => {
183+
const handler = await createDiscussionMain({
184+
max: 5,
185+
category: "RESEARCH", // Uppercase slug
186+
});
187+
188+
const result = await handler(
189+
{
190+
title: "Test Discussion",
191+
body: "This is a test discussion.",
192+
},
193+
{}
194+
);
195+
196+
expect(result.success).toBe(true);
197+
expect(result.number).toBe(42);
198+
199+
// Verify the correct category ID was used (Research)
200+
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
201+
expect(createMutationCall).toBeDefined();
202+
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn3"); // Research category
203+
});
204+
205+
it("should preserve category IDs (exact match, case-sensitive)", async () => {
206+
const handler = await createDiscussionMain({
207+
max: 5,
208+
category: "DIC_kwDOGFsHUM4BsUn3", // Direct category ID
209+
});
210+
211+
const result = await handler(
212+
{
213+
title: "Test Discussion",
214+
body: "This is a test discussion.",
215+
},
216+
{}
217+
);
218+
219+
expect(result.success).toBe(true);
220+
expect(result.number).toBe(42);
221+
222+
// Verify the exact category ID was used
223+
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
224+
expect(createMutationCall).toBeDefined();
225+
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn3");
226+
});
227+
228+
it("should use item category over config category (case-insensitive)", async () => {
229+
const handler = await createDiscussionMain({
230+
max: 5,
231+
category: "general", // Config says general
232+
});
233+
234+
const result = await handler(
235+
{
236+
title: "Test Discussion",
237+
body: "This is a test discussion.",
238+
category: "AUDITS", // Item overrides with uppercase
239+
},
240+
{}
241+
);
242+
243+
expect(result.success).toBe(true);
244+
expect(result.number).toBe(42);
245+
246+
// Verify Audits category was used (from item, not config)
247+
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
248+
expect(createMutationCall).toBeDefined();
249+
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn2"); // Audits category
250+
});
251+
252+
it("should fallback to first category when no match found", async () => {
253+
const handler = await createDiscussionMain({
254+
max: 5,
255+
category: "NonExistentCategory",
256+
});
257+
258+
const result = await handler(
259+
{
260+
title: "Test Discussion",
261+
body: "This is a test discussion.",
262+
},
263+
{}
264+
);
265+
266+
expect(result.success).toBe(true);
267+
expect(result.number).toBe(42);
268+
269+
// Verify fallback to first category (General)
270+
const createMutationCall = mockGithub.graphql.mock.calls.find(call => call[0].includes("createDiscussion"));
271+
expect(createMutationCall).toBeDefined();
272+
expect(createMutationCall[1].categoryId).toBe("DIC_kwDOGFsHUM4BsUn1"); // General (first)
273+
});
274+
});

pkg/workflow/create_discussion.go

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,8 @@ func (c *Compiler) parseDiscussionsConfig(outputMap map[string]any) *CreateDiscu
9292
return nil // Invalid configuration, return nil to cause validation error
9393
}
9494

95-
// Validate category naming convention (lowercase, preferably plural)
96-
if validateDiscussionCategory(config.Category, discussionLog, c.markdownPath) {
97-
return nil // Invalid configuration, return nil to cause validation error
98-
}
95+
// Normalize and validate category naming convention
96+
config.Category = normalizeDiscussionCategory(config.Category, discussionLog, c.markdownPath)
9997

10098
// Log configured values
10199
if config.TitlePrefix != "" {
@@ -201,18 +199,18 @@ func (c *Compiler) buildCreateOutputDiscussionJob(data *WorkflowData, mainJobNam
201199
})
202200
}
203201

204-
// validateDiscussionCategory validates discussion category naming conventions
205-
// Categories should be lowercase and preferably plural
206-
// Returns true if validation fails (invalid), false if valid
207-
func validateDiscussionCategory(category string, log *logger.Logger, markdownPath string) bool {
202+
// normalizeDiscussionCategory normalizes discussion category to lowercase
203+
// and provides warnings about naming conventions
204+
// Returns normalized category (or original if it's a category ID)
205+
func normalizeDiscussionCategory(category string, log *logger.Logger, markdownPath string) string {
208206
// Empty category is allowed (GitHub Discussions will use default)
209207
if category == "" {
210-
return false
208+
return category
211209
}
212210

213-
// GitHub Discussion category IDs start with "DIC_" - these are valid
211+
// GitHub Discussion category IDs start with "DIC_" - don't normalize these
214212
if strings.HasPrefix(category, "DIC_") {
215-
return false
213+
return category
216214
}
217215

218216
// List of known category naming issues and their corrections
@@ -223,26 +221,25 @@ func validateDiscussionCategory(category string, log *logger.Logger, markdownPat
223221
"Research": "research",
224222
}
225223

226-
// Check if category has uppercase letters
227-
if category != strings.ToLower(category) {
224+
// Check if category has uppercase letters and normalize
225+
normalizedCategory := strings.ToLower(category)
226+
if category != normalizedCategory {
228227
var message string
229228
// Check if we have a known correction
230229
if corrected, exists := categoryCorrections[category]; exists {
231-
message = fmt.Sprintf("Discussion category %q should use lowercase: %q", category, corrected)
230+
message = fmt.Sprintf("Discussion category %q normalized to lowercase: %q", category, corrected)
232231
if log != nil {
233-
log.Printf("Invalid discussion category %q: should use lowercase: %q", category, corrected)
232+
log.Printf("Normalized discussion category %q to lowercase: %q", category, corrected)
234233
}
235234
} else {
236-
message = fmt.Sprintf("Discussion category %q should use lowercase", category)
235+
message = fmt.Sprintf("Discussion category %q normalized to lowercase: %q", category, normalizedCategory)
237236
if log != nil {
238-
log.Printf("Invalid discussion category %q: should use lowercase", category)
237+
log.Printf("Normalized discussion category %q to lowercase: %q", category, normalizedCategory)
239238
}
240239
}
241240

242-
// Print formatted warning to stderr
243-
fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", message))
244-
245-
return true // Validation failed
241+
// Print formatted info message to stderr
242+
fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "info", message))
246243
}
247244

248245
// Warn about singular forms of common categories
@@ -251,11 +248,11 @@ func validateDiscussionCategory(category string, log *logger.Logger, markdownPat
251248
"report": "reports",
252249
}
253250

254-
if plural, isSingular := singularToPlural[category]; isSingular {
251+
if plural, isSingular := singularToPlural[normalizedCategory]; isSingular {
255252
if log != nil {
256-
log.Printf("⚠ Discussion category %q is singular; consider using plural form %q for consistency", category, plural)
253+
log.Printf("⚠ Discussion category %q is singular; consider using plural form %q for consistency", normalizedCategory, plural)
257254
}
258255
}
259256

260-
return false // Validation passed
257+
return normalizedCategory
261258
}

0 commit comments

Comments
 (0)