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 - Wrong Field(s) Become Hidden Due to Tags/Rate Column Indices #6270

Closed
1 task done
rmelillo76 opened this issue Oct 30, 2023 · 8 comments
Closed
1 task done
Assignees
Labels
Milestone

Comments

@rmelillo76
Copy link

rmelillo76 commented Oct 30, 2023

MMEX version:

  • 1.7.0

Description of the bug

The inception of Tags and/or Rates may cause the wrong field(s) to be hidden on existing Transaction Reports that have hidden columns. Note: This is easy to correct on a report-by-report basis upon opening the report, but may cause confusion and frustration for users. If this can't be easily solved via logic or a SQL script, we may want to note this as a known issue when releasing 1.7.

Reproduction

Open a Transaction Report that was created prior to the inception of Rate and/or Tags that contains hidden field(s). In my case, I had hidden the "Rate" field. but now "Amount" erroneously becomes hidden. This appears to be because the fields are identified via index, and the index has changed due to the new Tags field.

Report opened in 1.63. The Notes field is hidden so the last column displayed is Amount:
image

image

I then opened the same database in 1.7 (after being upgraded to DB version 19) and viewed the report. The Amount field is now hidden instead and the Rate and Notes fields are displayed.

image

image

Thoughts?

@whalley whalley added this to the v1.7.0 milestone Oct 30, 2023
@whalley whalley added the bug label Oct 30, 2023
@n-stein
Copy link
Contributor

n-stein commented Oct 31, 2023

Good catch, Rich. I think I added Tags in the middle to make the order of fields in the "Hide Columns" pop-up match the filter dialog and didn't consider the implications of changing the item indices.

The easiest solution is to simply move Tags and Rate to the bottom of the list to preserve the index of all the pre-existing items, but that means those check boxes will now be at the end after the custom fields. Not ideal, but better than breaking saved reports.

Side note, we should consider changing the filter description to use the actual column heading rather than the index. I can't imagine "Hide Columns: 10" means much to anyone...

@n-stein
Copy link
Contributor

n-stein commented Oct 31, 2023

A consideration -- moving "Rate" to the bottom would fix the pre-1.6.4 reports with any hidden columns that have index ≥10, but will throw off any reports saved in 1.6.4 where Rate is now expected to be at index 10.

@rmelillo76
Copy link
Author

A consideration -- moving "Rate" to the bottom would fix the pre-1.6.4 reports with any hidden columns that have index ≥10, but will throw off any reports saved in 1.6.4 where Rate is now expected to be at index 10.

Thanks Nick. I think that's the way to handle this too. It's possible not too many users bother to hide columns, so the impact to existing reports since 1.6.4 may be low anyway. And in a future change we consider a change to show the true name rather than the index (e..g. "10") like you mentioned.

@n-stein
Copy link
Contributor

n-stein commented Nov 1, 2023

On a related note, I noticed that the list of column names is not translated.

image

I assume this is unintentional and will fix.

n-stein added a commit to n-stein/moneymanagerex that referenced this issue Nov 2, 2023
@n-stein
Copy link
Contributor

n-stein commented Nov 2, 2023

Fixed, also changed the report text to show column names instead of indices.

whalley added a commit that referenced this issue Nov 2, 2023
@rmelillo76
Copy link
Author

Fixed, also changed the report text to show column names instead of indices.

Thanks! Code looks good, but for some reason the build failed in Appveyor, looks like ther was a git issue. So, I'll test once we have a successful build: https://ci.appveyor.com/project/whalley/moneymanagerex/builds/48431811/job/3lr5il7uhsatvlf3

@n-stein
Copy link
Contributor

n-stein commented Nov 4, 2023

I'll test once we have a successful build

Should be able to test now using the latest build.

@rmelillo76
Copy link
Author

Looks good, closing - thanks @n-stein !

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

No branches or pull requests

3 participants