Skip to content

fix: 限制共享账本 Editor 创建标签 - #436

Open
tedzhouhk wants to merge 3 commits into
TNT-Likely:mainfrom
tedzhouhk:agent/fix-shared-editor-tags
Open

fix: 限制共享账本 Editor 创建标签#436
tedzhouhk wants to merge 3 commits into
TNT-Likely:mainfrom
tedzhouhk:agent/fix-shared-editor-tags

Conversation

@tedzhouhk

@tedzhouhk tedzhouhk commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

变更内容

  • 共享账本 Editor 的标签选择器不再显示“新建标签”;空状态提示标签由 Owner 管理。
  • Owner 与个人账本仍可从标签选择器创建标签。
  • 权限判断在账本 provider reload 时 fail closed,避免切换账本的短窗口暴露创建入口。
  • Editor 确认选择时清理升级前残留的正数 personal tag ID,并保留 Owner mirror 的负数 synthetic ID。
  • TagEditPage 保存后向调用方返回已保存的 Tag,使新建标签可以立即自动选中;查询不到已保存标签时安全地无结果返回。
  • 将硬编码的“暂无标签”替换为现有 l10n 文案,并新增 Owner 管理提示的中、英、韩、繁中文案。

范围说明

本 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.dart
  • test/widgets/tag_edit_page_result_test.dart
  • 共 7/7 passed
  • flutter gen-l10n 通过,生成文件无额外变化
  • 最终 diff 相对 main 不包含任何 cloud sync、realtime、cache 或同步 E2E 文件

Fixes #435

@TNT-Likely

TNT-Likely commented Aug 12, 2026

Copy link
Copy Markdown
Owner

感谢 PR,也感谢你把 issue 里的方案写得那么清楚。入口屏蔽和 TagEditPage 返回值这两块做得对。

但我想把这个 PR 的范围收敛到只剩 App 侧入口屏蔽 —— 不是因为你其余的判断有错,而是我重新划了一下 App 和服务端的分工。下面说明理由。

请保留的部分

  • lib/pages/tag/widgets/tag_selector.dart —— 屏蔽 Editor 的创建入口、提示文案、确认时过滤掉不可见的历史个人标签 ID
  • lib/providers/tag_providers.dart —— canCreateTagForCurrentLedgerProvider(含你用 unwrapPrevious() 避免切账本短窗口暴露入口这个细节,考虑得很到位)
  • lib/pages/tag/tag_edit_page.dart —— 返回新建的 Tag
  • l10n 一个 key × 4 语言
  • 对应的 widget 测试

顺便确认过:TagSelector.show 全仓只有金额输入面板一处调用,所以屏蔽是全覆盖的;标签管理页的创建入口没动也是对的(Editor 仍应能建自己的个人标签)。

请拆走的部分

  1. lib/cloud/sync/sync_engine_apply.dart —— 收紧 name fallback
  2. lib/cloud/sync/sync_engine_realtime.dart —— 整个 revert(串行队列 + stale override 清理 + 双层分块删除)
  3. lib/cloud/sync/entity_serializer.dart / sync_engine_serialization.dart —— opaque override 三态序列化,退回 isNotEmpty ? x : null
  4. LookupCache 的扩展(账本角色 + shared tag 按名索引)—— 它是给第 1 项做支撑的,一起去掉
  5. 整文件 dart format 冲刷

为什么 apply 侧这次不收

你在 issue 里把「Owner 端不按未知名字自动建标签」列进方案1 是对的。但我把 _syncTransactionTags 的两个分支逐行过了一遍之后,认为这道防线放在 App 上收益不大:

  • tagIds 非空的分支,name fallback 只复用已有本地标签,匹配不到就丢弃关联,从不创建
  • 只有 tagIds 完全缺失(name-only)的分支才会用新 UUID 建标签

而 App 的 push 只要有标签就一定带 tagIds(App 建的标签都有 syncId)。所以重复标签只会由 name-only payload 触发,而这种 payload 不会来自 App,全部来自 Web / 服务端侧:全局编辑弹窗、AI 批量写入、CSV 导入。

我查了一下,主要产生路径是 Web 的全局编辑弹窗 —— 它对 Editor 列的是他自己的个人标签,而且只发标签名不发 tag_ids。已经单独记成 BeeCount-Cloud#70,那个前端小改动(加共享账本分支 + 补发 tag_ids)才是真正堵住入口的地方。

所以分工是: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 项里的一个坑,避免以后再踩

sync_engine_apply.dart 里这行删掉的时候,想请你留意背后的原因,因为它不直观:

if (!payload.containsKey('tagIds') && !payload.containsKey('tags')) return;

注释的前提是「新版会用 tags="" + tagIds=[] 显式表达删除全部标签」。这对 App 自己的 push 成立,但对服务端不成立,而 pull 到的 payload 是服务端产的。服务端三处都是「空集合就不写 key」:

  • src/routers/write/_shared.py::_projection_row_to_tx_dict —— if row.tags_csv: / if tag_ids: 才写
  • src/snapshot_builder.py 的 build —— 同样条件
  • src/snapshot_mutator.py::update_transaction —— 显式清空时直接 item.pop("tags") / item.pop("tagIds")

#68 也没改这个语义(校验只在 "tags" in change.payload or "tagIds" in change.payload 时才跑)。

所以这行的实际效果是:用户在 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 应同步删除');
  });
}
分支 结果
main ✅ 通过,关联被清除
本 PR ❌ 失败,关联仍在:[TransactionTag(id: 1, transactionId: 1, tagId: 1)]

一条通用原则,以后改同步协议时可以套用:BeeCount Cloud 是自建部署,App 和 Cloud 各自独立升级,用户会跑各种老服务端版本,所以 App 侧不能假设服务端行为会跟着一起升级 —— 缺字段的兜底必须对老服务端也安全。这跟 attachments 的处理是不对称的:entity_serializer.dart 里那段注释说 attachments 即使 [] 也必须写出来,正是为了让对端能区分「没发」和「全删」,但 tags 至今没做这个改造,所以不能照抄那边的假设。

关于格式冲刷

我把两侧都按统一格式归一化后重算了真实差异:

文件 原始 diff 真实改动
lib/providers/tag_providers.dart +37/-13 12 行
lib/cloud/sync/sync_engine_realtime.dart +214/-63 158 行(约 120 行纯格式)

tagsWithStatsProvidertagStatsProviderisTagNameDuplicateProvider 等一堆 provider 被重排但零行为变化。仓库目前没有强制 dart format,麻烦只留真实改动,这样 diff 才好 review。

'暂无标签'l10n.tagManageEmpty 这条跟本 issue 无关,但是真的改善,留着我接受。

一个想确认的点

确认时过滤掉正数 ID 之后,Editor 编辑历史交易时,那些升级前残留的个人标签关联会被静默丢掉。我认为这是方案1 的应然结果,所以接受 —— 只是想确认这是你的本意而不是副作用。

一个小 nit

tag_edit_page.dartsavedTag = (await repo.getTagById(id))! 强解包了一个 Future<Tag?>。返回 null 时异常被外层 catch 吞掉,用户会看到「错误: Null check operator used on a null value」,但标签其实已经创建成功了,而且页面不会关闭。建议兜底成无结果的 pop()

质量部分(你容器里没跑成的)

你 PR 里提到完整 flutter analyze 被 OOM kill 了,我用 Flutter 3.27.3 在本地补跑:

  • flutter analyze:0 error,21 个改动文件中没有任何一条分析问题
  • 全量 flutter test:510 通过(main 基线 493),失败项与 main 完全一致,无新增失败
  • l10n 生成文件格式正确,是对的 Flutter 版本生成的,只新增了那一个 key

工程质量本身是好的,这次收范围纯粹是分工问题。缩到只剩入口屏蔽那几个文件之后我就合。

@tedzhouhk

Copy link
Copy Markdown
Contributor Author

感谢这么详细的 review 和本地补跑。我已经按建议把 PR 收敛到 App 侧入口屏蔽,最新提交是 9b199d6

  • 只保留 TagSelector、创建权限 provider、TagEditPage 返回值、l10n 和对应 widget tests;
  • 已完整移除 sync apply / serialization / realtime / LookupCache 及相关同步测试改动,服务端缺少 tags/tagIds 时清空标签的原有语义也随之恢复;
  • 去掉了 tag_providers.dart 等文件的格式冲刷;
  • TagEditPage 已移除 null 强解包:查询不到已保存的 Tag 时会正常无结果 pop()
  • 受限内存容器中两个 widget test 文件共 7/7 通过,flutter gen-l10n 通过。

你问到的确认点:Editor 编辑历史交易时,确认选择会过滤掉升级前残留、在当前共享账本 picker 中不可见的正数 personal tag ID;这是方案 1 的预期行为,不是意外副作用。Owner mirror 的负数 synthetic ID 在资源暂时不可见时仍会保留。

PR 描述也已按收敛后的范围更新。再次感谢你把 App/Cloud 的协议边界和 #70 的真正入口梳理得这么清楚。

@TNT-Likely

Copy link
Copy Markdown
Owner

收敛得很到位,谢谢。生产代码只剩 3 个文件,tag_providers.dart 正好 12 行、格式冲刷清干净了,sync_engine_* 相对 main 完全没有 diff。

我本地验证了一遍:

  • 之前那个「Web 清空标签」的回归测试已转为通过containsKey 早退删掉之后语义恢复了)
  • 你的 8 个 widget test 全过
  • flutter analyze 0 error(tag_providers.dart 那两条 unused_import / undefined_shown_name warning 在 main 上一模一样,是既有的失效 import,不用管)
  • rebase 到最新 main 干净

真机走了一遍,有一个必须修,另外还有一处想请你拆掉。

必须修:tagSelectOwnerManaged 实际上看不见

这个新增文案在整个文件里只出现在一个位置 —— _buildEmptyState() 内部。而它的调用条件是:

if (filteredTags.isEmpty && allTags.isEmpty) {
  return _buildEmptyState(...);
}

也就是只有「一个标签都没有」时才会显示。只要 Owner 有标签(这是绝大多数情况),Editor 看到的是:有标签列表、没有「新建标签」按钮、也没有任何解释。恰好在最需要解释的时候不出现,我实测确认了。

issue 方案1 里这一条的意图是解释「为什么你不能创建」,所以提示应该跟创建入口互斥渲染 —— 原来放「新建标签」的那个位置,Editor 就放提示:

if (canCreateTag) ...[
  const SizedBox(height: 8),
  _buildCreateNew(l10n),
] else ...[
  const SizedBox(height: 8),
  // tagSelectOwnerManaged 提示放这里
],

_buildEmptyState 里现有的那份可以保留(空列表时也该有解释),两处并存没问题。

顺带一个相邻缺口,一起看看:搜索无结果时(filteredTags.isEmptyallTags 非空)会落到 ListView 且没有任何 section —— Editor 看到的是纯空白,既没结果也没提示。改动前至少还有个「新建标签」按钮兜着,现在对 Editor 是全空。

请拆出去:tag_edit_page.dart 及其测试

这一处修的是另一个 bug:main 上 Navigator.of(context).pop() 不带结果,而 tag_selector._createNewTag 判断 if (result != null) 才自动选中,所以自动选中逻辑在 main 上从来没生效过。改成 pop(savedTag) 是对的修复。

但它跟「屏蔽 Editor 创建入口」是两件事。issue 方案1 里确实单独列了这一条,所以有出处 —— 只是按一个 PR 一件事的原则,麻烦连 test/widgets/tag_edit_page_result_test.dart 一起挪到单独 PR,我很快就能合。

请保留:tag_providers.dart

顺便说清楚,免得你顺手一起删了:那 12 行是必要的

它是纯新增的 canCreateTagForCurrentLedgerProvider,没有改动任何既有 provider。TagSelector 必须知道当前账本角色才能决定是否显示创建入口,这个判断总得有地方放;写成 provider 而不是塞在 widget 里,好处是能单测「切账本短窗口不沿用上一个账本权限」那条 —— 你那个测试写得对,那是个真实的时序坑。

不用你管:「最近使用」和「全部标签」内容相同

我测的时候发现共享账本 Editor 视角下这两个 section 内容完全一样。查了一下是既有 bug,跟你无关

lib/utils/shared_ledger_picker_filter.dart::filterTagsForLedger 在 Editor 分支里会无视传入的列表,直接返回全量 mirror 标签:

if (ctx == null || !ctx.isEditorInShared || ctx.ledgerSyncId == null) {
  return all;
}
final shared = await (select(sharedLedgerTags)..where(...)).get();
return shared.map(_sharedTagAsMain).toList();   // 入参 all 被丢掉

recentTagsForCurrentLedgerProvider 先查了真正的最近 10 条、再交给它过滤,于是 Editor 下被整体替换成全量。这个文件你一行没动,recentTagsForCurrentLedgerProvider 也是原封不动,所以不是本 PR 引入的。

请不要在这个 PR 里修它 —— 我已经单独开了 #443 跟进,里面写了根因、影响范围(只有「最近使用」这一处传子集,accounts / categories 没有等价问题)和建议修法。

那个 issue 里还记了两个相邻问题:Editor 的标签关联其实存在 transaction_tag_overrides 而不是 transaction_tags,所以 getRecentlyUsedTags 根本看不到;以及它没有账本维度过滤。这些都不该塞进本 PR。

如果你有兴趣接 #443 也欢迎,但那是另一个 PR 的事,跟本 PR 的合并不挂钩。


总结一下,这个 PR 剩下两件事:把提示改成跟创建入口互斥渲染tag_edit_page 及其测试拆走。做完我就合。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] 共享账本 Editor 新建标签后不可见,并在关联交易后生成重复标签

2 participants