fix: 限制共享账本 Editor 创建标签 - #436
Conversation
|
感谢 PR,也感谢你把 issue 里的方案写得那么清楚。入口屏蔽和 但我想把这个 PR 的范围收敛到只剩 App 侧入口屏蔽 —— 不是因为你其余的判断有错,而是我重新划了一下 App 和服务端的分工。下面说明理由。 请保留的部分
顺便确认过: 请拆走的部分
为什么 apply 侧这次不收你在 issue 里把「Owner 端不按未知名字自动建标签」列进方案1 是对的。但我把
而 App 的 push 只要有标签就一定带 我查了一下,主要产生路径是 Web 的全局编辑弹窗 —— 它对 Editor 列的是他自己的个人标签,而且只发标签名不发 所以分工是:App 侧只负责不再产生新的隐形标签(你这个 PR 的层1),Web 侧负责不再产生 name-only payload(#70)。 apply 路径保持现状。 存量脏数据我不打算靠代码兜 —— 目前实际受影响的估计就一个用户,我手工清掉那几条重复标签,比在客户端长期养一套 stale override 清理机制便宜。 顺带说明:配套的 BeeCount-Cloud#68 我也不打算合,理由我在那边单独回复了。它 1160 行里大部分是 LWW 顺序、附件 GC 时机、advisory lock 缓存这些跟本 issue 无关的加固;而按上面的分析,服务端 write 边界校验并不是必需的那块。那些加固本身有价值,但我暂时不需要,你不必为此额外做事。 第 2、3 项想解决的「Owner mirror 晚到导致标签关联丢失」是个真问题,我不否认,但它跟本 issue 是两件事。这个我暂时不需要,就这么放着也没问题 —— 如果你觉得值得做、也有兴趣继续贡献,那麻烦另开 issue 先把方案过一遍再动手;不做也完全 OK。 顺便说一下第 3 项里的一个坑,避免以后再踩
if (!payload.containsKey('tagIds') && !payload.containsKey('tags')) return;注释的前提是「新版会用
#68 也没改这个语义(校验只在 所以这行的实际效果是:用户在 Web 上把一笔交易的标签全部删掉,App 永远还显示着那些标签。 我写了回归测试确认,用的是个人账本 —— 不是共享账本边界情况,是全量用户的核心行为: 复现测试(main 通过 / 本 PR 失败)// test/cloud/sync/tag_clear_regression_test.dart
import 'package:drift/drift.dart' show Value;
import 'package:drift/native.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:shared_preferences/shared_preferences.dart';
import 'package:beecount/cloud/sync/change_tracker.dart';
import 'package:beecount/cloud/sync/sync_engine.dart';
import 'package:beecount/data/db.dart';
import 'package:beecount/data/repositories/local/local_repository.dart';
import '_fakes/fake_beecount_cloud_provider.dart';
void main() {
TestWidgetsFlutterBinding.ensureInitialized();
late BeeDatabase db;
late ChangeTracker changeTracker;
late LocalRepository repo;
late FakeBeeCountCloudProvider provider;
late SyncEngine engine;
setUp(() async {
SharedPreferences.setMockInitialValues({});
db = BeeDatabase.forTesting(NativeDatabase.memory());
changeTracker = ChangeTracker(db);
repo = LocalRepository(db, changeTracker: changeTracker);
provider = FakeBeeCountCloudProvider();
engine = SyncEngine(
db: db,
provider: provider,
changeTracker: changeTracker,
repo: repo,
);
});
tearDown(() async {
await db.close();
});
test('个人账本:server 清空标签(payload 无 tags/tagIds key)→ 本地关联应被清除',
() async {
final ledgerId = await db.into(db.ledgers).insert(
LedgersCompanion.insert(name: 'L', syncId: const Value('ledger-1')),
);
final catId = await db.into(db.categories).insert(
CategoriesCompanion.insert(
name: 'Food',
kind: 'expense',
syncId: const Value('cat-1'),
),
);
final tagId = await db.into(db.tags).insert(
TagsCompanion.insert(name: 'lunch', syncId: const Value('tag-1')),
);
final txId = await db.into(db.transactions).insert(
TransactionsCompanion.insert(
ledgerId: ledgerId,
type: 'expense',
amount: 12.5,
categoryId: Value(catId),
happenedAt: Value(DateTime.utc(2026, 5, 1, 10)),
syncId: const Value('tx-A'),
),
);
await db.into(db.transactionTags).insert(
TransactionTagsCompanion.insert(transactionId: txId, tagId: tagId),
);
expect(await db.select(db.transactionTags).get(), hasLength(1));
// 用户在 Web 上把标签全部删掉 —— 真实 payload 里 tags / tagIds 两个 key 都不存在
provider.pushFakeChange(
entityType: 'transaction',
entitySyncId: 'tx-A',
ledgerId: 'ledger-1',
payload: {
'syncId': 'tx-A',
'type': 'expense',
'amount': 12.5,
'happenedAt': '2026-05-01T10:00:00Z',
'note': 'lunch',
'categoryName': 'Food',
'categoryKind': 'expense',
'categoryId': 'cat-1',
// 故意不带 'tags' / 'tagIds'
},
);
expect(await engine.pull('1'), 1);
expect(await db.select(db.transactionTags).get(), isEmpty,
reason: '标签已在 Web 端被清空,本地 transaction_tags 应同步删除');
});
}
一条通用原则,以后改同步协议时可以套用:BeeCount Cloud 是自建部署,App 和 Cloud 各自独立升级,用户会跑各种老服务端版本,所以 App 侧不能假设服务端行为会跟着一起升级 —— 缺字段的兜底必须对老服务端也安全。这跟 attachments 的处理是不对称的: 关于格式冲刷我把两侧都按统一格式归一化后重算了真实差异:
一个想确认的点确认时过滤掉正数 ID 之后,Editor 编辑历史交易时,那些升级前残留的个人标签关联会被静默丢掉。我认为这是方案1 的应然结果,所以接受 —— 只是想确认这是你的本意而不是副作用。 一个小 nit
质量部分(你容器里没跑成的)你 PR 里提到完整
工程质量本身是好的,这次收范围纯粹是分工问题。缩到只剩入口屏蔽那几个文件之后我就合。 |
|
感谢这么详细的 review 和本地补跑。我已经按建议把 PR 收敛到 App 侧入口屏蔽,最新提交是
你问到的确认点:Editor 编辑历史交易时,确认选择会过滤掉升级前残留、在当前共享账本 picker 中不可见的正数 personal tag ID;这是方案 1 的预期行为,不是意外副作用。Owner mirror 的负数 synthetic ID 在资源暂时不可见时仍会保留。 PR 描述也已按收敛后的范围更新。再次感谢你把 App/Cloud 的协议边界和 #70 的真正入口梳理得这么清楚。 |
|
收敛得很到位,谢谢。生产代码只剩 3 个文件, 我本地验证了一遍:
真机走了一遍,有一个必须修,另外还有一处想请你拆掉。 必须修:
|
变更内容
TagEditPage保存后向调用方返回已保存的Tag,使新建标签可以立即自动选中;查询不到已保存标签时安全地无结果返回。范围说明
本 PR 按维护者反馈收敛为 App 侧入口屏蔽及对应 UI 行为,不修改同步 apply、序列化、realtime、LookupCache 或服务端协议。
Web 端产生 name-only 标签 payload 的路径由 BeeCount-Cloud #70 单独处理。
Editor 编辑历史交易时,升级前残留且在当前 picker 中不可见的个人标签关联会被移除;这是方案 1 的预期行为,不是副作用。
验证
Flutter 3.27.3;以下测试在 Docker 中以
--memory=1536m --memory-swap=1536m --cpus=1和--concurrency=1运行:test/widgets/tag_selector_shared_editor_test.darttest/widgets/tag_edit_page_result_test.dartflutter gen-l10n通过,生成文件无额外变化main不包含任何 cloud sync、realtime、cache 或同步 E2E 文件Fixes #435