Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Improve output when transfers are approved/cancelled and event data in sync #399

Merged
merged 2 commits into from Oct 4, 2022

Conversation

YoshihitoAso
Copy link
Member

close #390

The following modifications have been made to resolve #390 issues.
Please provide feedback.

  1. Add transfer approval/cancellation operation history TBL (transfer_approval_history). The record is added only when the approval or cancellation is successful. Note that this TBL was converted from one that already existed and was not being used.

移転承諾/申請キャンセルのオペレーション履歴TBL(transfer_approval_history)を追加しました。承諾、キャンセルが成功した場合にのみ、レコードを追記するようにしています。なお、このTBLは既に存在していたもので、利用されていなかったものを転用しています。

  1. In the reference API related to transfer approval, modified the logic to determine the transfer approved status and transfer canceled status to include the above TBL status as a condition.

移転承諾に関連する参照APIにおいて、移転承認済みステータス、移転キャンセル済みステータスを判定するロジックを修正し、上記のTBLの状態も条件に含めて判定するように変更しました。

@purplesmoke05
Copy link
Member

purplesmoke05 commented Oct 4, 2022

IDXTransferApproval Possible State

transfer_approved escrow_finished cancelled Summary
A True True - => Transferred
B None True - => EscrowFinished
C None - True => Cancelled
D None None None => Applied

IDXTransferApproval outer joined with TransferApprovalHistory Matrix

# IDXTransferApproval TransferApprovalHistory Summary
a A Approve => Transferred
b A None => Transferred (TransferApprovalHistory is missing due to old application)
c B Approve => Transferred (IDXTransferApproval is on syncing)
d B None => EscrowFinished
e C Canceled => Canceled
f C None => Canceled (TransferApprovalHistory is missing due to old application)
g D Approve => Transferred (IDXTransferApproval is on syncing)
h D Canceled => Canceled (IDXTransferApproval is on syncing)
i D None => Applied

Comment on lines +1981 to +2004
(
and_(IDXTransferApproval.escrow_finished == True,
IDXTransferApproval.transfer_approved == None,
TransferApprovalHistory.operation_type == None),
1
), # EscrowFinish(escrow_finished)
(
and_(IDXTransferApproval.transfer_approved == None,
TransferApprovalHistory.operation_type == TransferApprovalOperationType.APPROVE.value),
2
), # Approve(operation completed, event synchronizing)
(
IDXTransferApproval.transfer_approved == True,
2
), # Approve(transferred)
(
and_(IDXTransferApproval.cancelled == None,
TransferApprovalHistory.operation_type == TransferApprovalOperationType.CANCEL.value),
3
), # Cancel(operation completed, event synchronizing)
(
IDXTransferApproval.cancelled == True,
3
), # Cancel(canceled)
Copy link
Member

Choose a reason for hiding this comment

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

From this perspective found no problems. LGTM.
(It is more intuitive to match "transferred" event first.)

Comment on lines 32 to 33
import config

Copy link
Member

Choose a reason for hiding this comment

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

This line should be written after root path import.

@YoshihitoAso YoshihitoAso merged commit 5e443f5 into dev-22.12 Oct 4, 2022
@YoshihitoAso YoshihitoAso deleted the feature/#390 branch October 4, 2022 11:05
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.

[FEATURE] Add status of transfer approval in progress
2 participants