Skip to content

Commit fee0a1b

Browse files
committed
fix: ensure create_column_transaction always returns a list for value to maintain API consistency
1 parent 5a51939 commit fee0a1b

3 files changed

Lines changed: 253 additions & 10 deletions

File tree

conduit/client/maniphest.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -338,15 +338,15 @@ def create_column_transaction(
338338
after_phids: Optional[List[PHID]] = None,
339339
) -> ManiphestTaskTransaction:
340340
"""Create a transaction to move task to a workboard column."""
341-
if before_phids or after_phids:
342-
column_position = {"columnPHID": column_phid}
343-
if before_phids:
344-
column_position["beforePHIDs"] = before_phids
345-
if after_phids:
346-
column_position["afterPHIDs"] = after_phids
347-
return {"type": "column", "value": [column_position]}
348-
else:
349-
return {"type": "column", "value": column_phid}
341+
column_position = {"columnPHID": column_phid}
342+
343+
if before_phids:
344+
column_position["beforePHIDs"] = before_phids
345+
if after_phids:
346+
column_position["afterPHIDs"] = after_phids
347+
348+
# Always return value as a list to maintain API consistency
349+
return {"type": "column", "value": [column_position]}
350350

351351
@staticmethod
352352
def create_space_transaction(space_phid: PHID) -> ManiphestTaskTransaction:

conduit/client/tests/test_maniphest.py

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,3 +339,235 @@ def test_search_helper_methods(self):
339339
# Verify all returned tasks are authored by current user
340340
for task in results["data"]:
341341
self.assertEqual(task["fields"]["authorPHID"], self.user["phid"])
342+
343+
def test_create_column_transaction_type_consistency(self):
344+
"""
345+
测试 create_column_transaction 方法始终返回正确的列表格式。
346+
347+
这个测试专门针对修复的问题:
348+
API Error: Exception when processing transaction of type "column":
349+
Error while reading "value": Expected a list, but value is not a list.
350+
"""
351+
test_column_phid = "PHID-PCOL-TESTCOLUMN"
352+
test_before_phid = "PHID-TASK-BEFORE"
353+
test_after_phid = "PHID-TASK-AFTER"
354+
355+
with self.subTest("Simple column transaction (no positioning)"):
356+
"""测试简单列事务始终返回列表格式"""
357+
transaction = self.cli.create_column_transaction(
358+
column_phid=test_column_phid
359+
)
360+
361+
# 验证基本结构
362+
self.assertEqual(transaction["type"], "column")
363+
self.assertIsInstance(
364+
transaction["value"],
365+
list,
366+
"简单列事务的 value 必须是列表格式,不能是字符串",
367+
)
368+
self.assertEqual(len(transaction["value"]), 1, "列表应该只包含一个元素")
369+
370+
# 验证列位置对象
371+
column_position = transaction["value"][0]
372+
self.assertIsInstance(column_position, dict, "列表元素应该是字典格式")
373+
self.assertEqual(
374+
column_position["columnPHID"],
375+
test_column_phid,
376+
"columnPHID 应该正确设置",
377+
)
378+
379+
# 验证没有定位字段
380+
self.assertNotIn(
381+
"beforePHIDs", column_position, "简单列事务不应该包含 beforePHIDs"
382+
)
383+
self.assertNotIn(
384+
"afterPHIDs", column_position, "简单列事务不应该包含 afterPHIDs"
385+
)
386+
387+
with self.subTest("Column transaction with before_phids only"):
388+
"""测试只包含 before_phids 的列事务"""
389+
transaction = self.cli.create_column_transaction(
390+
column_phid=test_column_phid, before_phids=[test_before_phid]
391+
)
392+
393+
# 验证基本结构
394+
self.assertEqual(transaction["type"], "column")
395+
self.assertIsInstance(
396+
transaction["value"], list, "带定位的列事务也必须是列表格式"
397+
)
398+
self.assertEqual(len(transaction["value"]), 1, "列表应该只包含一个元素")
399+
400+
# 验证列位置对象
401+
column_position = transaction["value"][0]
402+
self.assertEqual(column_position["columnPHID"], test_column_phid)
403+
self.assertIn("beforePHIDs", column_position, "应该包含 beforePHIDs")
404+
self.assertEqual(
405+
column_position["beforePHIDs"],
406+
[test_before_phid],
407+
"beforePHIDs 应该正确设置",
408+
)
409+
self.assertNotIn("afterPHIDs", column_position, "不应该包含 afterPHIDs")
410+
411+
with self.subTest("Column transaction with after_phids only"):
412+
"""测试只包含 after_phids 的列事务"""
413+
transaction = self.cli.create_column_transaction(
414+
column_phid=test_column_phid, after_phids=[test_after_phid]
415+
)
416+
417+
# 验证基本结构
418+
self.assertEqual(transaction["type"], "column")
419+
self.assertIsInstance(
420+
transaction["value"], list, "带定位的列事务也必须是列表格式"
421+
)
422+
self.assertEqual(len(transaction["value"]), 1, "列表应该只包含一个元素")
423+
424+
# 验证列位置对象
425+
column_position = transaction["value"][0]
426+
self.assertEqual(column_position["columnPHID"], test_column_phid)
427+
self.assertNotIn("beforePHIDs", column_position, "不应该包含 beforePHIDs")
428+
self.assertIn("afterPHIDs", column_position, "应该包含 afterPHIDs")
429+
self.assertEqual(
430+
column_position["afterPHIDs"],
431+
[test_after_phid],
432+
"afterPHIDs 应该正确设置",
433+
)
434+
435+
with self.subTest("Column transaction with both before_phids and after_phids"):
436+
"""测试同时包含 before_phids 和 after_phids 的列事务"""
437+
transaction = self.cli.create_column_transaction(
438+
column_phid=test_column_phid,
439+
before_phids=[test_before_phid],
440+
after_phids=[test_after_phid],
441+
)
442+
443+
# 验证基本结构
444+
self.assertEqual(transaction["type"], "column")
445+
self.assertIsInstance(
446+
transaction["value"], list, "完整定位的列事务也必须是列表格式"
447+
)
448+
self.assertEqual(len(transaction["value"]), 1, "列表应该只包含一个元素")
449+
450+
# 验证列位置对象
451+
column_position = transaction["value"][0]
452+
self.assertEqual(column_position["columnPHID"], test_column_phid)
453+
self.assertIn("beforePHIDs", column_position, "应该包含 beforePHIDs")
454+
self.assertIn("afterPHIDs", column_position, "应该包含 afterPHIDs")
455+
self.assertEqual(
456+
column_position["beforePHIDs"],
457+
[test_before_phid],
458+
"beforePHIDs 应该正确设置",
459+
)
460+
self.assertEqual(
461+
column_position["afterPHIDs"],
462+
[test_after_phid],
463+
"afterPHIDs 应该正确设置",
464+
)
465+
466+
with self.subTest("Column transaction with multiple positioning PHIDs"):
467+
"""测试多个定位 PHID 的列事务"""
468+
multiple_before = ["PHID-TASK-BEFORE1", "PHID-TASK-BEFORE2"]
469+
multiple_after = ["PHID-TASK-AFTER1", "PHID-TASK-AFTER2"]
470+
471+
transaction = self.cli.create_column_transaction(
472+
column_phid=test_column_phid,
473+
before_phids=multiple_before,
474+
after_phids=multiple_after,
475+
)
476+
477+
# 验证基本结构
478+
self.assertEqual(transaction["type"], "column")
479+
self.assertIsInstance(transaction["value"], list)
480+
self.assertEqual(len(transaction["value"]), 1)
481+
482+
# 验证列位置对象
483+
column_position = transaction["value"][0]
484+
self.assertEqual(column_position["columnPHID"], test_column_phid)
485+
self.assertEqual(
486+
column_position["beforePHIDs"],
487+
multiple_before,
488+
"应该支持多个 beforePHIDs",
489+
)
490+
self.assertEqual(
491+
column_position["afterPHIDs"], multiple_after, "应该支持多个 afterPHIDs"
492+
)
493+
494+
with self.subTest("Type consistency across all variants"):
495+
"""验证所有变体都返回相同的基本类型结构"""
496+
variants = [
497+
# 简单列事务
498+
self.cli.create_column_transaction(column_phid=test_column_phid),
499+
# 只包含 before
500+
self.cli.create_column_transaction(
501+
column_phid=test_column_phid, before_phids=[test_before_phid]
502+
),
503+
# 只包含 after
504+
self.cli.create_column_transaction(
505+
column_phid=test_column_phid, after_phids=[test_after_phid]
506+
),
507+
# 包含两者
508+
self.cli.create_column_transaction(
509+
column_phid=test_column_phid,
510+
before_phids=[test_before_phid],
511+
after_phids=[test_after_phid],
512+
),
513+
]
514+
515+
for i, transaction in enumerate(variants):
516+
with self.subTest(variant=i):
517+
# 所有变体都应该有相同的基本类型结构
518+
self.assertEqual(
519+
transaction["type"], "column", f"变体 {i} 的类型应该是 'column'"
520+
)
521+
self.assertIsInstance(
522+
transaction["value"], list, f"变体 {i} 的 value 必须是列表"
523+
)
524+
self.assertEqual(
525+
len(transaction["value"]),
526+
1,
527+
f"变体 {i} 的列表应该只包含一个元素",
528+
)
529+
self.assertIsInstance(
530+
transaction["value"][0], dict, f"变体 {i} 的列表元素应该是字典"
531+
)
532+
self.assertEqual(
533+
transaction["value"][0]["columnPHID"],
534+
test_column_phid,
535+
f"变体 {i} 的 columnPHID 应该正确",
536+
)
537+
538+
def test_column_transaction_prevents_api_error(self):
539+
"""
540+
测试修复后的列事务不会导致 "Expected a list" API 错误。
541+
542+
这是一个回归测试,确保之前修复的问题不会重新出现。
543+
"""
544+
test_column_phid = "PHID-PCOL-TESTCOLUMN"
545+
546+
# 模拟用户原始调用场景
547+
transaction = self.cli.create_column_transaction(column_phid=test_column_phid)
548+
549+
# 验证事务格式符合 Phabricator API 要求
550+
self.assertEqual(transaction["type"], "column")
551+
552+
# 关键验证:value 必须是列表,这是导致原始错误的原因
553+
self.assertIsInstance(
554+
transaction["value"],
555+
list,
556+
"修复后的列事务必须返回列表格式以避免 'Expected a list' 错误",
557+
)
558+
559+
# 验证列表内容
560+
self.assertGreater(len(transaction["value"]), 0, "列表不能为空")
561+
self.assertIsInstance(
562+
transaction["value"][0], dict, "列表元素必须是包含列位置信息的字典"
563+
)
564+
565+
# 验证列位置信息结构
566+
column_position = transaction["value"][0]
567+
self.assertIn("columnPHID", column_position, "必须包含 columnPHID 字段")
568+
self.assertEqual(
569+
column_position["columnPHID"], test_column_phid, "columnPHID 值必须正确"
570+
)
571+
572+
# 这个测试确保了修复的有效性,防止回归
573+
# 如果这里失败,说明 "Expected a list, but value is not a list" 错误可能会重新出现

conduit/client/tests/test_workboard.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,10 @@ def test_column_transaction_creation(self):
9191
column_phid="PHID-PCOL-EXAMPLE"
9292
)
9393
self.assertEqual(simple_tx["type"], "column")
94-
self.assertEqual(simple_tx["value"], "PHID-PCOL-EXAMPLE")
94+
# After fix: value should always be a list containing column position dict
95+
self.assertIsInstance(simple_tx["value"], list)
96+
self.assertEqual(len(simple_tx["value"]), 1)
97+
self.assertEqual(simple_tx["value"][0]["columnPHID"], "PHID-PCOL-EXAMPLE")
9598

9699
# Test 2: Column transaction with positioning
97100
positioned_tx = self.maniphest_client.create_column_transaction(
@@ -117,13 +120,21 @@ def test_column_transaction_creation(self):
117120
)
118121
self.assertEqual(after_only_tx["type"], "column")
119122
self.assertIsInstance(after_only_tx["value"], list)
123+
self.assertEqual(len(after_only_tx["value"]), 1)
124+
self.assertEqual(after_only_tx["value"][0]["columnPHID"], "PHID-PCOL-EXAMPLE")
125+
self.assertEqual(after_only_tx["value"][0]["afterPHIDs"], ["PHID-TASK-AFTER1"])
120126

121127
# Test with just before_phids
122128
before_only_tx = self.maniphest_client.create_column_transaction(
123129
column_phid="PHID-PCOL-EXAMPLE", before_phids=["PHID-TASK-BEFORE1"]
124130
)
125131
self.assertEqual(before_only_tx["type"], "column")
126132
self.assertIsInstance(before_only_tx["value"], list)
133+
self.assertEqual(len(before_only_tx["value"]), 1)
134+
self.assertEqual(before_only_tx["value"][0]["columnPHID"], "PHID-PCOL-EXAMPLE")
135+
self.assertEqual(
136+
before_only_tx["value"][0]["beforePHIDs"], ["PHID-TASK-BEFORE1"]
137+
)
127138

128139
def test_workboard_data_structures(self):
129140
"""Test Workboard-related data structures and API responses."""

0 commit comments

Comments
 (0)