Skip to content

feat(alipay): refine refund and closed transaction handling logic#137

Open
Creling wants to merge 2 commits into
deb-sig:masterfrom
Creling:master
Open

feat(alipay): refine refund and closed transaction handling logic#137
Creling wants to merge 2 commits into
deb-sig:masterfrom
Creling:master

Conversation

@Creling

@Creling Creling commented Feb 2, 2025

Copy link
Copy Markdown

Description

修复了原先逻辑可能遗漏的两类”退款成功“和”交易关闭“交易记录

  • ”退款成功“状态的交易记录的category未必都是“退款”,“退款成功”的饿了么订单记录的category是“餐饮美食”
2024-*****,餐饮美食,饿了么,e50***@alibaba-inc.com,退款-饺子*****,不计收支,27.00,建设银行储蓄卡(5503),退款成功,2024111422001138361434487255_13130601324111427241160943669	,13120600724111419446389943669	,,
  • “交易关闭”状态的交易记录的type未必都是“不计收支”,“交易关闭”的闲鱼卖出记录的type是“收入”
2024-*****,收入,****n,105***@qq.com,logitech *****,收入,150.00,,交易关闭,2024101923001114791428992964	,T200P2331530004732165953	,,

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

@Creling Creling requested review from a team, Triple-Z and gaocegege as code owners February 2, 2025 07:53
@Triple-Z

Copy link
Copy Markdown
Member

扩大了“退款成功”和“交易关闭“的忽略账单范围。对于支付宝账单而言,有没有可能误伤一些本该记录的交易?

/cc @gaocegege

@gaocegege

Copy link
Copy Markdown
Member

整一个 CLI argument flag 来控制呢?

@Creling

Creling commented Feb 12, 2025

Copy link
Copy Markdown
Author

扩大了“退款成功”和“交易关闭“的忽略账单范围。对于支付宝账单而言,有没有可能误伤一些本该记录的交易?

/cc @gaocegege

我的观点是,状态为“退款成功”和“交易关闭“的交易,本就应该考虑忽略它们,如果可能会误伤一些本该记录的交易。应该对这些交易额外添加豁免。

也就是说,对“退款成功”和“交易关闭“状态的交易的处理应该是一个白名单机制:对一部分不该忽略的交易添加白名单,豁免忽略。而不是黑名单机制:手动提交应该忽略的交易。无论如何,“退款成功”和“交易关闭“状态的交易中,应该忽略的交易占了大多数,而这些应该忽略的交易的特征非常复杂,如果手动添加,非常容易遗漏。

我们目前可能还没有找到,状态为“退款成功”和“交易关闭“,但不应该忽略的交易,如果找到了,把这些交易按照特征添加进白名单并不复杂。

@Creling

Creling commented Apr 30, 2025

Copy link
Copy Markdown
Author

关于这个pr,有什么新的进展或者需要讨论的东西吗?

@gaocegege gaocegege left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不好意思回复这么晚,我错过了通知。LGTM,我觉得 OK

@gaocegege

Copy link
Copy Markdown
Member

cc @Triple-Z

@Triple-Z

Copy link
Copy Markdown
Member

对“退款成功”和“交易关闭“状态的交易的处理应该是一个白名单机制:对一部分不该忽略的交易添加白名单,豁免忽略。而不是黑名单机制:手动提交应该忽略的交易。无论如何,“退款成功”和“交易关闭“状态的交易中,应该忽略的交易占了大多数,而这些应该忽略的交易的特征非常复杂,如果手动添加,非常容易遗漏。

但很不幸,我们当前并不能配置“白名单”的规则,配置文件中只支持主动忽略某些交易。

@Creling

Creling commented Apr 30, 2025

Copy link
Copy Markdown
Author

但很不幸,我们当前并不能配置“白名单”的规则,配置文件中只支持主动忽略某些交易。

这里的黑名单和白名单都是指代码中硬编码的规则。

目前的做法是:

if order.Metadata["status"] == "退款成功" && order.Category == "退款" {
     ... 忽略该笔交易 ...
 }

这种写法没有忽略掉很多该被忽略的交易(比如,状态为 "退款成功" 但Category为"餐饮美食"的饿了么支付宝退款),那么,与其修改成:

if order.Metadata["status"] == "退款成功" && (order.Category == "退款" || order.Category == "餐饮美食") {
     ... 忽略该笔交易 ...
 }

不如修改成:

if order.Metadata["status"] == "退款成功" {
   if order.Category == "XXX" { // 白名单
     ... 不忽略该笔交易 ...
   }
   ... 忽略该笔交易 ...
}

因为,状态为 "退款成功" 的交易更有可能是该被忽略的交易。因此,应该首先忽略所有状态为 "退款成功" 的交易,再把其中不该被忽略的交易列一个白名单(PR做法),而不是把状态为 "退款成功" 的交易中该被忽略的交易列一个名单(现有做法,容易导致遗漏)

@Triple-Z

Copy link
Copy Markdown
Member

因为,状态为 "退款成功" 的交易更有可能是该被忽略的交易。因此,应该首先忽略所有状态为 "退款成功" 的交易,再把其中不该被忽略的交易列一个白名单(PR做法),而不是把状态为 "退款成功" 的交易中该被忽略的交易列一个名单(现有做法,容易导致遗漏)

理解。其实我的意思是“硬编码”在代码中的规则,应该限制越多越好(scope 越小越好)。如果用户下载了 double-entry-generator ,发现了这个行为不是他所期望的,还可以通过配置文件来 work-around 。

如果修改为代码的白名单机制,用户发现某些交易不该被忽略,那他将难以处理这个问题,只能上报 issue 来等待开发者更新 double-entry-generator 的小版本。

如果代码修改为白名单,我认为连带的规则必须支持 ignore: false 的能力。

@Creling

Creling commented Apr 30, 2025

Copy link
Copy Markdown
Author

那么如果按照下面的方式修改现有代码,是否可以被接受?

if order.Metadata["status"] == "退款成功" && (order.Category == "退款" || order.Category == "餐饮美食") {
     ... 忽略该笔交易 ...
 }

本质上,我认为,对于"退款成功"状态的交易,更有可能是应该忽略的,而不是不应该忽略的。因此,“scope太小导致未忽略该忽略的“比”“scope太大导致忽略了不该忽略的“发生的情况更多(我已经在饿了么和闲鱼上遇到了两个未忽略应该忽略的case)。不过,我尊重开发者的意见 :)

为所有规则添加一个ignore: false确实是一个好主意,但每个provider是单独解析规则的,目前还没有时间为每一个provider都添加一个ignore: false,如果只为alipay添加,相当于alipay有了独特的功能语法,似乎不太优雅。


题外话,不同provider支持的不同“方言语法”确实有点儿影响体验。比如WeChat支持用Tag来指定item的标签,而对于Alipay,虽然double-entry-generator的文档表示同样使用Tag来指定标签,但实际上需要使用Tags(P.S., 在Beancount v3中,取消了一个item支持多个tag的功能,现在一个item只支持一个tag),确实也在考虑,在保持后向兼容的情况下统一一下语法。

@Triple-Z

Triple-Z commented Apr 30, 2025

Copy link
Copy Markdown
Member

为所有规则添加一个ignore: false确实是一个好主意,但每个provider是单独解析规则的,目前还没有时间为每一个provider都添加一个ignore: false,如果只为alipay添加,相当于alipay有了独特的功能语法,似乎不太优雅。

可以先给 alipay 添加。某个 provider 能用什么指定的过滤条件,在 README 中配置的部分指示清楚即可。后面可能考虑统一重构或是补齐过滤条件。

ignore 这个条件应该是已有的,事实上我们默认就认为当前的规则都是 ignore: false 。只是在这里,需要遵循 ignore: false 的规则,当前如下的代码都是没有遵循这个规则,确实也不是很好设计。

if order.Metadata["status"] == "退款成功" && (order.Category == "退款" || order.Category == "餐饮美食") {
     ... 忽略该笔交易 ...
 }

/cc @gaocegege

@Triple-Z Triple-Z added the status/need-discussion Need more discussion for this issue/PR. label May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/need-discussion Need more discussion for this issue/PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants