Skip to content

new: add sequence number in transactions panel #7080

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 3, 2025
Merged

new: add sequence number in transactions panel #7080

merged 3 commits into from
Jan 3, 2025

Conversation

georgeef
Copy link
Contributor

@georgeef georgeef commented Jan 2, 2025 •
edited
Loading

This PR adds a sequence number in transaction panels.

Motivation

The transaction ID used to be assigned by SQLite and it was essentially a sequence number, indicating the order of creation. With the introduction of SUID (see #6964), the transaction ID is a long number and its appearance in transaction panels becomes less convenient.

The ID column in transaction panels is useful for sorting (by order of creation), although the exact value of a transaction ID is essentially opaque (i.e., of little interest to the user).

This PR adds one more column in transaction panels, containing a sequence number (SN) for each transaction, which is assigned sequentially (starting with 1) when a transactions list is generated. The order of SN is equivalent to the order of Date/ID, i.e., transactions are sorted by Date and ID, and then an SN is assigned to them sequentially.

From user's perspective, either the SN or the ID can be used for sorting. Users may hide one of the two columns, according to their preference. The SN has the advantage that it is a shorter number, but it may be confusing since it is assigned dynamically. On the other hand, the ID is static, but it is a longer number and may look random and confusing.

Design

The displayed SN for executions of scheduled transactions in a transactions panel, follows the same rules as the displayed ID. The SN of scheduled transactions is empty, while the SN of scheduled splits appears as ".", in order to distinguish the scheduled transactions from the rest of transactions, when either the SN or the ID column is used.

Implementation

  • In Model_Checking::Full_Data: add long SN, wxString displaySN.
  • In Fused_Transaction: add struct SorterByFUSEDTRANSSN.
  • Refine mmCheckingPanel::filterTable(). Fix bug with wrong filtering when "Ignore Future Transactions" is set.
  • In TransactionListCtrl::EColumn: add COL_SN at the end of the list (in order to preserve already assigned column numbers).
  • Refine TransactionListCtrl.
  • In TransactionListCtrl::OnColClick(), break the association between Date and ID.

This change is Reviewable

@georgeef
Copy link
Contributor Author

georgeef commented Jan 2, 2025 •
edited
Loading

@n-stein
I added a new column code COL_SN at the end of TransactionListCtrl::EColumn. I understand that the column codes are important for saving the column settings, so the new code should not interfere with previously assigned codes.

@georgeef
Copy link
Contributor Author

georgeef commented Jan 2, 2025 •
edited
Loading

@whalley
See last bullet in Implementation above. I had to break the association between Date and ID, since users may use SN instead of ID. This means Date can be sorted together with any other secondary column (which gives more flexibility). Sorting by SN is equivalent to sorting by Date/ID, because the SN is assigned after sorting by Date/ID.

There was a bug (introduced with #6860), which wrongly filters out transactions in multi-account panels (like All Transactions), when "Ignore Future Transactions" is set (if a transaction has a future date, all transactions with larger ID are ignored). This bug has been fixed in this PR.

@giongy
Copy link
Contributor

giongy commented Jan 2, 2025

Hello,
I'm just reading your updates, I'm not a developer but I'm very interested about this software.
Your update will be affect this kind of link?
image
I mean, which number will be showed here? the "fake" SN or the real one?

@georgeef
Copy link
Contributor Author

georgeef commented Jan 2, 2025

The ID will be longer (about 16 decimal digits) for new entries in 1.9.0. Only the transaction panels that can open directly from navigation tree at left will show an SN in addition to ID. The ID will still be available and it will be the real ID, as before.

@rmelillo76
Copy link

I know this still a work in progress but wanted to point out that Transaction Report hyperlinks seem to be broken and are not opening the Edit Transaction dialog, (nothing appears to happen when clicking the link). It still does work for the older, shorter ID's:

image

@georgeef
Copy link
Contributor Author

georgeef commented Jan 3, 2025

@rmelillo76
Thank you for reporting. Could you file a separate issue, including the version and how to reproduce? Please check first if there is another recent issue for the same problem.

@guanlisheng as FYI

@georgeef georgeef mentioned this pull request Jan 3, 2025
Merged
@rmelillo76 rmelillo76 mentioned this pull request Jan 3, 2025
Closed
@rmelillo76
Copy link

@rmelillo76 Thank you for reporting. Could you file a separate issue, including the version and how to reproduce? Please check first if there is another recent issue for the same problem.

@guanlisheng as FYI

Thanks, happy to help. Issue #7082 has been opened.

@whalley whalley merged commit 0bd02a3 into moneymanagerex:master Jan 3, 2025
4 checks passed
@georgeef georgeef deleted the new_sn branch January 3, 2025 14:37
@georgeef georgeef mentioned this pull request Jan 6, 2025
Merged
@n-stein
Copy link
Contributor

n-stein commented Jan 6, 2025

I'm having two issues:

  1. SN is displayed in the wrong column:

image

  1. The All Transactions page crashes because it is attempting to sort on the "Deleted Time" column which doesn't exist in that panel.

@n-stein
Copy link
Contributor

n-stein commented Jan 6, 2025

For issue 2 we should verify that g_sortcol and prev_g_sortcol exist in m_real_columns at the beginning of TransactionListCtrl::sortTable. If not, they should be reset to COL_def_sort and COL_def_sort2 respectively.

@n-stein
Copy link
Contributor

n-stein commented Jan 6, 2025

I found issue 1 occurs when the stored column order contains UDFC entries which don't exist in CUSTOMFIELD_V1. This may occur since SETTING_V1 is cross-database -- you can open db1 and the UDFC columns will be stored in the order, but when you open db2 which doesn't have any UDFC fields it still loads them into m_real_columns which throws off the order.

@n-stein
Copy link
Contributor

n-stein commented Jan 6, 2025

I'll create a new issue for these and fix them.

@georgeef
Copy link
Contributor Author

georgeef commented Jan 6, 2025

@n-stein
Thank you for the observations and for the debugging. I didn't notice in my tests.

@n-stein n-stein mentioned this pull request Jan 6, 2025
Closed
@n-stein
Copy link
Contributor

n-stein commented Jan 6, 2025

#7099

I didn't notice in my tests.

To be fair, these bugs are unrelated to your SN change. They are an artifact of the column order save/restore changes made for #6847, so mostly my fault... I simply noticed them when I glanced at the SN column. In hindsight I should have debugged further before commenting in this thread, so my apologies.

@georgeef
Copy link
Contributor Author

georgeef commented Jan 6, 2025

No problem, good that you noticed and you know how to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@georgeef @giongy @rmelillo76 @n-stein @whalley