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

Transaction Report Filter does not work with Split Transaction Notes #5752

Closed
1 task done
stardust7951 opened this issue Mar 13, 2023 · 14 comments · Fixed by #5756
Closed
1 task done

Transaction Report Filter does not work with Split Transaction Notes #5752

stardust7951 opened this issue Mar 13, 2023 · 14 comments · Fixed by #5756
Labels
Milestone

Comments

@stardust7951
Copy link

stardust7951 commented Mar 13, 2023

MMEX version:
1.6.3 Windows

Description of the bug

Everything is in the title, when I do a search on an existing Split Tansaction Note there is nothing in the Transaction Report.

Reproduction

Is the bug reproducible?

  • Always
@renato-mmex
Copy link

Please use the current version 1.6.3
https://github.com/moneymanagerex/moneymanagerex/releases

@stardust7951
Copy link
Author

I'm sorry but I made a mistake in writing my first message.

I'm using version 1.6.3 :

Money Manager Ex
• Version: 1.6.3 64 bit
• Based on: Jan 31 2023 12:09:35
• Database version: 17 (aes128cbc)
• Git commit: 48d8562 (2023-01-31)
MMEX uses the following products:
• wxWidgets 3.2.1 (wxMSW 10.0)
• wxSQLite3 4.9.1 (SQLite 3.40.0)
• RapidJSON 1.1.0
• Lua 5.3.5
• libcurl/7.82.0-DEV Schannel
• gettext 0.19.8.1
• apexcharts.js
Developed with:
• CMake 3.16.2
• MSBuild 15.9.21.664
• Microsoft Visual Studio 15.0
• Microsoft Visual C++ 19.16.27035.0
• Windows SDK 10.0.14393.0
Started on:
• Windows 11 (build 22621), 64-bit edition
• French (France) (windows-1252)
• 2560x1440 32bit 96x96ppi

@vomikan
Copy link
Member

vomikan commented Mar 13, 2023

I can't reproduce this issue.

@n-stein
Copy link
Contributor

n-stein commented Mar 13, 2023

I am able to reproduce on the current 1.6.4 Beta build. Here is a transaction with a split note "TX29 Split 1 note":

image

Transaction filter for Notes TX29*:

image

Transaction report is empty:

image

@vomikan vomikan added the bug label Mar 13, 2023
@vomikan vomikan added this to the v1.6.4 milestone Mar 13, 2023
n-stein added a commit to n-stein/moneymanagerex that referenced this issue Mar 13, 2023
n-stein added a commit to n-stein/moneymanagerex that referenced this issue Mar 13, 2023
@n-stein
Copy link
Contributor

n-stein commented Mar 13, 2023

If the "Notes" box is checked but the text field is left blank, should a transaction be returned only if the main transaction and ALL of its splits have no note, or if at least ONE split has no note?

@tactilis
Copy link

If the "Notes" box is checked but the text field is left blank

On WIndows, at least, you cannot have an empty Transaction Filter Notes text field if the 'Notes' box is checked. The text field always contains an asterisk, so that should be matching zero or more characters in the main or split notes and should therefore return all transactions.

@vomikan
Copy link
Member

vomikan commented Mar 13, 2023

The text field always contains an asterisk

That's not so. If the field is left empty, transactions with empty notes will be searched.

With the advent of split, it has become somewhat more difficult to make the correct search logic.

@vomikan
Copy link
Member

vomikan commented Mar 13, 2023

@n-stein I think this fix is incorrect https://github.com/moneymanagerex/moneymanagerex/pull/5754/files

it would be correct to make a separate transaction for each split transaction, and only then to filter it.

    for (const auto& tran : Model_Checking::instance().all())
    {
    
      here should be 
            for (const auto& split : full_tran.m_splits)
            {
                 tran = split
            }
      
      
        if (!dlg.get()->mmIsRecordMatches(tran, ~splits~)) continue;

@tactilis
Copy link

tactilis commented Mar 13, 2023

The text field always contains an asterisk

That's not so. If the field is left empty

I find it impossible to create an empty field empty.

In this video I do:

  1. Check the Notes box.
  2. Attempt to delete the asterisk - you cannot, it is always shown.
  3. Type a character, then delete it. The asterisk reappears.
mmex_V7X8KsP7Xb.mp4

The asterisk means 'match zero or more characters' but it seems that is not what is happening. Perhaps the asterisk should not be shown.

@n-stein
Copy link
Contributor

n-stein commented Mar 13, 2023

The asterisk is a "Hint" for the notes field, meant as a suggestion of what types of data can be entered there. The field itself is blank. I agree that it is confusing and should be changed.

@n-stein
Copy link
Contributor

n-stein commented Mar 13, 2023

it would be correct to make a separate transaction for each split transaction, and only then to filter it.

    for (const auto& tran : Model_Checking::instance().all())
    {
    
      here should be 
            for (const auto& split : full_tran.m_splits)
            {
                 tran = split
           
      
      
        if (!dlg.get()->mmIsRecordMatches(tran, ~splits~)) continue;

@vomikan It seems you are suggesting we implement #5465. Is that right?

@n-stein
Copy link
Contributor

n-stein commented Mar 13, 2023

Let me rework it, I'll implement #5465 as well.

@n-stein
Copy link
Contributor

n-stein commented Mar 14, 2023

Done. Transaction report will only display matching splits, not the full transaction:

image
image

image
image

Note this is just a result of the changes for #5465, please see that issue for more info.

@n-stein
Copy link
Contributor

n-stein commented Mar 14, 2023

Also, no more asterisk
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants