Skip to content

Commit 269819b

Browse files
committed
Fix undefined property access with optional chaining
- Replace && checks with optional chaining (?.) for safer code - Fix: Cannot read properties of undefined (reading 'toLowerCase') - Updated list_tasks.ts: assignee name/email/dart_id checks - Updated csv.ts: dartboard, status, tag lookups - Updated types/index.ts: find* helper functions - Added TDD tests for undefined/null handling (21 new tests) - All 363 tests passing This prevents crashes when API returns malformed data with undefined/null values in name, email, or dart_id fields. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent c12bf4f commit 269819b

5 files changed

Lines changed: 388 additions & 20 deletions

File tree

src/parsers/csv.ts

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -510,8 +510,8 @@ export function resolveReferences(
510510
});
511511
} else {
512512
const dartboard = config.dartboards.find(
513-
d => d.name.toLowerCase() === dartboardInput.toLowerCase() ||
514-
d.dart_id.toLowerCase() === dartboardInput.toLowerCase()
513+
d => d.name?.toLowerCase() === dartboardInput.toLowerCase() ||
514+
d.dart_id?.toLowerCase() === dartboardInput.toLowerCase()
515515
);
516516

517517
if (dartboard) {
@@ -543,8 +543,8 @@ export function resolveReferences(
543543
if (row.status) {
544544
const statusInput = row.status.trim();
545545
const status = config.statuses.find(
546-
s => s.name.toLowerCase() === statusInput.toLowerCase() ||
547-
s.dart_id.toLowerCase() === statusInput.toLowerCase()
546+
s => s.name?.toLowerCase() === statusInput.toLowerCase() ||
547+
s.dart_id?.toLowerCase() === statusInput.toLowerCase()
548548
);
549549

550550
if (status) {
@@ -575,9 +575,9 @@ export function resolveReferences(
575575
if (row.assignee) {
576576
const assigneeInput = row.assignee.trim();
577577
const assignee = config.assignees.find(
578-
a => (a.email && a.email.toLowerCase() === assigneeInput.toLowerCase()) ||
579-
a.name.toLowerCase() === assigneeInput.toLowerCase() ||
580-
(a.dart_id && a.dart_id.toLowerCase() === assigneeInput.toLowerCase())
578+
a => a.email?.toLowerCase() === assigneeInput.toLowerCase() ||
579+
a.name?.toLowerCase() === assigneeInput.toLowerCase() ||
580+
a.dart_id?.toLowerCase() === assigneeInput.toLowerCase()
581581
);
582582

583583
if (assignee) {
@@ -637,8 +637,8 @@ export function resolveReferences(
637637

638638
for (const tagInput of tagInputNames) {
639639
const tag = config.tags.find(
640-
t => t.name.toLowerCase() === tagInput.toLowerCase() ||
641-
t.dart_id.toLowerCase() === tagInput.toLowerCase()
640+
t => t.name?.toLowerCase() === tagInput.toLowerCase() ||
641+
t.dart_id?.toLowerCase() === tagInput.toLowerCase()
642642
);
643643

644644
if (tag) {
@@ -830,8 +830,8 @@ export function validateRow(
830830
if (row.status) {
831831
const statusInput = row.status.trim();
832832
const status = config.statuses.find(
833-
s => s.name.toLowerCase() === statusInput.toLowerCase() ||
834-
s.dart_id.toLowerCase() === statusInput.toLowerCase()
833+
s => s.name?.toLowerCase() === statusInput.toLowerCase() ||
834+
s.dart_id?.toLowerCase() === statusInput.toLowerCase()
835835
);
836836

837837
if (!status) {
@@ -848,9 +848,9 @@ export function validateRow(
848848
if (row.assignee) {
849849
const assigneeInput = row.assignee.trim();
850850
const assignee = config.assignees.find(
851-
a => (a.email && a.email.toLowerCase() === assigneeInput.toLowerCase()) ||
852-
a.name.toLowerCase() === assigneeInput.toLowerCase() ||
853-
(a.dart_id && a.dart_id.toLowerCase() === assigneeInput.toLowerCase())
851+
a => a.email?.toLowerCase() === assigneeInput.toLowerCase() ||
852+
a.name?.toLowerCase() === assigneeInput.toLowerCase() ||
853+
a.dart_id?.toLowerCase() === assigneeInput.toLowerCase()
854854
);
855855

856856
if (!assignee) {

src/tools/list_tasks.test.ts

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
/**
2+
* list_tasks Tool Handler Tests
3+
*
4+
* Tests for handling undefined/null values in config data
5+
* to prevent "Cannot read properties of undefined" errors
6+
*/
7+
8+
import { describe, it, expect, beforeEach, vi } from 'vitest';
9+
import { handleListTasks } from './list_tasks.js';
10+
import { DartClient } from '../api/dartClient.js';
11+
import { configCache } from '../cache/configCache.js';
12+
13+
// Mock DartClient
14+
vi.mock('../api/dartClient.js');
15+
vi.mock('../cache/configCache.js');
16+
17+
describe('list_tasks - optional chaining safety', () => {
18+
beforeEach(() => {
19+
// Clear cache before each test
20+
vi.clearAllMocks();
21+
process.env.DART_TOKEN = 'dsa_test_token';
22+
});
23+
24+
it('should handle assignees with undefined name gracefully', async () => {
25+
const mockConfig = {
26+
assignees: [
27+
{ dart_id: 'user1', name: undefined as any, email: 'test@example.com' },
28+
{ dart_id: 'user2', name: 'John Doe', email: 'john@example.com' },
29+
],
30+
dartboards: [{ dart_id: 'db1', name: 'Engineering' }],
31+
statuses: [{ dart_id: 'st1', name: 'In Progress' }],
32+
tags: [{ dart_id: 'tag1', name: 'urgent' }],
33+
priorities: [],
34+
sizes: [],
35+
folders: [],
36+
};
37+
38+
const mockTasks = {
39+
tasks: [{ dart_id: 'task1', title: 'Test Task', created_at: '2024-01-01' }],
40+
total: 1,
41+
};
42+
43+
vi.mocked(configCache.get).mockReturnValue(null);
44+
vi.mocked(DartClient).mockImplementation(() => ({
45+
getConfig: vi.fn().mockResolvedValue(mockConfig),
46+
listTasks: vi.fn().mockResolvedValue(mockTasks),
47+
} as any));
48+
49+
// This should not throw "Cannot read properties of undefined (reading 'toLowerCase')"
50+
const result = await handleListTasks({ assignee: 'john@example.com' });
51+
52+
expect(result.tasks).toHaveLength(1);
53+
});
54+
55+
it('should handle assignees with undefined email gracefully', async () => {
56+
const mockConfig = {
57+
assignees: [
58+
{ dart_id: 'user1', name: 'Jane Doe', email: undefined },
59+
{ dart_id: 'user2', name: 'John Doe', email: 'john@example.com' },
60+
],
61+
dartboards: [{ dart_id: 'db1', name: 'Engineering' }],
62+
statuses: [{ dart_id: 'st1', name: 'In Progress' }],
63+
tags: [{ dart_id: 'tag1', name: 'urgent' }],
64+
priorities: [],
65+
sizes: [],
66+
folders: [],
67+
};
68+
69+
const mockTasks = {
70+
tasks: [{ dart_id: 'task1', title: 'Test Task', created_at: '2024-01-01' }],
71+
total: 1,
72+
};
73+
74+
vi.mocked(configCache.get).mockReturnValue(null);
75+
vi.mocked(DartClient).mockImplementation(() => ({
76+
getConfig: vi.fn().mockResolvedValue(mockConfig),
77+
listTasks: vi.fn().mockResolvedValue(mockTasks),
78+
} as any));
79+
80+
// This should not throw error when email is undefined
81+
const result = await handleListTasks({ assignee: 'Jane Doe' });
82+
83+
expect(result.tasks).toHaveLength(1);
84+
});
85+
86+
it('should handle assignees with null dart_id gracefully', async () => {
87+
const mockConfig = {
88+
assignees: [
89+
{ dart_id: null as any, name: 'Test User', email: 'test@example.com' },
90+
{ dart_id: 'user2', name: 'John Doe', email: 'john@example.com' },
91+
],
92+
dartboards: [{ dart_id: 'db1', name: 'Engineering' }],
93+
statuses: [{ dart_id: 'st1', name: 'In Progress' }],
94+
tags: [{ dart_id: 'tag1', name: 'urgent' }],
95+
priorities: [],
96+
sizes: [],
97+
folders: [],
98+
};
99+
100+
const mockTasks = {
101+
tasks: [{ dart_id: 'task1', title: 'Test Task', created_at: '2024-01-01' }],
102+
total: 1,
103+
};
104+
105+
vi.mocked(configCache.get).mockReturnValue(null);
106+
vi.mocked(DartClient).mockImplementation(() => ({
107+
getConfig: vi.fn().mockResolvedValue(mockConfig),
108+
listTasks: vi.fn().mockResolvedValue(mockTasks),
109+
} as any));
110+
111+
// Should handle null dart_id without crashing
112+
const result = await handleListTasks({ assignee: 'john@example.com' });
113+
114+
expect(result.tasks).toHaveLength(1);
115+
});
116+
117+
it('should handle tags with undefined name gracefully', async () => {
118+
const mockConfig = {
119+
assignees: [{ dart_id: 'user1', name: 'John Doe', email: 'john@example.com' }],
120+
dartboards: [{ dart_id: 'db1', name: 'Engineering' }],
121+
statuses: [{ dart_id: 'st1', name: 'In Progress' }],
122+
tags: [
123+
{ dart_id: 'tag1', name: undefined as any },
124+
{ dart_id: 'tag2', name: 'urgent' },
125+
],
126+
priorities: [],
127+
sizes: [],
128+
folders: [],
129+
};
130+
131+
const mockTasks = {
132+
tasks: [{ dart_id: 'task1', title: 'Test Task', created_at: '2024-01-01' }],
133+
total: 1,
134+
};
135+
136+
vi.mocked(configCache.get).mockReturnValue(null);
137+
vi.mocked(DartClient).mockImplementation(() => ({
138+
getConfig: vi.fn().mockResolvedValue(mockConfig),
139+
listTasks: vi.fn().mockResolvedValue(mockTasks),
140+
} as any));
141+
142+
// Should handle undefined tag name without crashing
143+
const result = await handleListTasks({ tags: ['urgent'] });
144+
145+
expect(result.tasks).toHaveLength(1);
146+
});
147+
148+
it('should match assignee by name even when email is null', async () => {
149+
const mockConfig = {
150+
assignees: [
151+
{ dart_id: 'user1', name: 'John Doe', email: null as any },
152+
],
153+
dartboards: [{ dart_id: 'db1', name: 'Engineering' }],
154+
statuses: [{ dart_id: 'st1', name: 'In Progress' }],
155+
tags: [],
156+
priorities: [],
157+
sizes: [],
158+
folders: [],
159+
};
160+
161+
const mockTasks = {
162+
tasks: [{ dart_id: 'task1', title: 'Test Task', assignee: 'user1', created_at: '2024-01-01' }],
163+
total: 1,
164+
};
165+
166+
vi.mocked(configCache.get).mockReturnValue(null);
167+
vi.mocked(DartClient).mockImplementation(() => ({
168+
getConfig: vi.fn().mockResolvedValue(mockConfig),
169+
listTasks: vi.fn().mockResolvedValue(mockTasks),
170+
} as any));
171+
172+
// Should successfully match by name even with null email
173+
const result = await handleListTasks({ assignee: 'John Doe' });
174+
175+
expect(result.tasks).toHaveLength(1);
176+
expect(result.filters_applied).toHaveProperty('assignee');
177+
});
178+
});

src/tools/list_tasks.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,8 @@ async function resolveFilters(
226226

227227
const assignee = config.assignees.find(
228228
(a) =>
229-
a.name.toLowerCase() === assigneeInput.toLowerCase() ||
230-
(a.email && a.email.toLowerCase() === assigneeInput.toLowerCase())
229+
a.name?.toLowerCase() === assigneeInput.toLowerCase() ||
230+
a.email?.toLowerCase() === assigneeInput.toLowerCase()
231231
);
232232

233233
if (!assignee) {

0 commit comments

Comments
 (0)