portfolio icon indicating copy to clipboard operation
portfolio copied to clipboard

Changing filters resets widgets

Open funnym0nk3y opened this issue 3 years ago • 8 comments

Describe the bug Changing filters to include or exclude accounts resets the data source to include all. More specific: I made a selection of some accounts and created a new data source/filter named "selection" . Changing the accounts in this data source leads to the following behaviour: The widget in the performance overview still displays "selection" as data source but displays values from all accounts.

Desktop (please complete the following information):

  • OS: Windows
  • Version: 0.56.0

funnym0nk3y avatar Nov 22 '21 21:11 funnym0nk3y

Hello @funnym0nk3y, do you mean this update issue?

https://user-images.githubusercontent.com/90478568/142997042-d7c6e736-63e8-404f-93b8-99e678847281.mov

In the video security 1 is assigned to ref1 and dep1 and security 2 to ref2 and dep2. In current version after changing filters (add/remove elements to/from filter) the current view is not updated, if the previously selected filter was changed. I think I found a solution for this and I will create a PR as soon as possible.

Or do you mean another upate issue? Can you describe it then with some screenshots, please? Thanks.

PS: sorry for German PP in video.

OnkelDok avatar Nov 23 '21 09:11 OnkelDok

Unfortunately this is not what I mean. I attached a gif where it shows the issue. The widget still shows data source as filter, but does show the data of the whole dataset.

PP_bug

funnym0nk3y avatar Nov 23 '21 11:11 funnym0nk3y

Thank you for the gif. In your gif you also see the issue I captured in my video. Ok I understand this and I will try to find the reason for this update issue.

OnkelDok avatar Nov 23 '21 11:11 OnkelDok

I think I have a rough idea what the cause is for this issue with resetting filter of the dashboard widgets after changing filters. But because I dont have this much background I cannot say if this issue can be fixed by changing only some lines of code. Maybe some datahandling in the background must be changed. Maybe another contributer (or @buchen himself) have to look at this.

This is my guess as to the cause of the error:

  • Filters are stored in .settings-file (if configured) or in central .settings-file like:
ClientFilterDropDown@json=[{"label"\:"filter1","uuids"\:"ad9b3775-d64c-4616-b275-0e0ba2b04e90,28e90642-ec4d-4eb2-907d-5b4fe99bd8d6"}]
  • Dashboard settings are stored in the .xml-file
<widget type="CALCULATION">
  <label>Performance-Berechnung, filter1</label>
  <configuration>
    <entry>
      <string>DATA_SERIES</string>
      <string>ClientFilterad9b3775-d64c-4616-b275-0e0ba2b04e9028e90642-ec4d-4eb2-907d-5b4fe99bd8d6</string>
    </entry>
  </configuration>
</widget>

So as you can see the dashboard-widget setting contains concatenated UUIDS (the UUIDs of the accounts in my file). When user switches to the performance dashboard view, then the following block of code is executed to choose the data series (applying the filter): https://github.com/buchen/portfolio/blob/c1b1dfd87ee66c1f18364fbe461bae935b673891/name.abuchen.portfolio.ui/src/name/abuchen/portfolio/ui/views/dashboard/DataSeriesConfig.java#L40-L46 The created string in first line (line 40) is the string from the xml above:

ClientFilterad9b3775-d64c-4616-b275-0e0ba2b04e9028e90642-ec4d-4eb2-907d-5b4fe99bd8d6

With this string the corresponding filter is searched (line 42). But when the user changed the filter before (e.g. removing account from filter), then this filter will not be found. As result the fallback will be searched (line 43 - 64) = main portfolio. The corresponding string (for the updated filter) would be for example:

ClientFilterad9b3775-d64c-4616-b275-0e0ba2b04e90

So the problem here is that the configuration of the dashboard-widget-settings contains filter details and not only the information "use filter filter1". Currently its hard to describe. Maybe @buchen understand what I try to describe. ;)

I attached my test files. Please not that I had to add ".txt" at the end of filename because github doesn't supports uploading .xml or .setting. filter_update_bug_2546.settings.txt filter_update_bug_2546.xml.txt

OnkelDok avatar Nov 23 '21 13:11 OnkelDok

Again I looked about the code. And I think my former assumption is correct. I try to explain it as short as possible:

In current version the custom filters doesn't have an unique identifier/key. They only have a name (but two filters can have the same name) and they have the filter data (two filters can have the same data). The dashboards (and I think also some other reports) are linking to a filter by using the filter data (link value = concatenation of the filterdata UUIDs). That's the reason for the dashboards (and some other reports) losing their link to the filter when the filter was changed (e.g. account removed from filter).

This link would not break when the custom filters would get an unique identifier/key with which they can identified. Then the dashboards (and other reports) have to use this identifier/key to link to a filter. With this the filter can be changed at will without cutting the link.

Locally I made some changes for the custom filters to have an unique identifier/key. This is a small change, I think. They main part would be to fix the existing links at program start:

  • Iterate all custom filters and create a unique identifier/key for each
  • Then change ALL existing links between dashboards (and also other reports) and the filter from old filter-data-link to the new identfier/key link

Maybe I make a PR to share my current progress. Because at the moment I struggle with the fix-existing-links-part and hopefully @buchen can give some hints to move forward.

Notes for me: Reports/Places where the custom filters where used and which setting/config should be migrated to new link: StatementOfAssets, StatementOfAssetsHistoryView (single charts), HoldingsPieChartView, DataSeriesConfig, PerformanceView, PerformanceChartView (single charts), ReturnsVolatilityChartView (single chart), SecuritiesPerformanceView, PaymentsView, TaxonomyView

OnkelDok avatar Jul 21 '22 18:07 OnkelDok

I would appreciate your PR. I hab a lot of trouble with lost filters in complex reporting-pages with a lot of widgets after changing the filters. I would take your change even if I lost all filters one last time. It is easier to create the filters again than searching for lost filters every time I have to change one.

oduudo avatar Jul 22 '22 11:07 oduudo

@oduudo & @funnym0nk3y: I think I'm close to finish the PR. If you have time, then you can help a lot with following: To test if the logic to migrate the old link to the new link of all reports, widgets and charts at startup is working correctly, I created a sample file (xml + settings file). I uploaded them here: https://github.com/buchen/portfolio/pull/2931#issuecomment-1211242642 If you download the files, open them with PP and check if I select the custom filter on all possible reports, views, widgets and charts that would be helpful. For widgets I used only a few because I think the migration logic should migrate all widgets the same way. If you want you can add all widgets with the custom filter and upload the file, then I can check with the current mingration logic if all widgets were migrated correctly.

Thank you.

OnkelDok avatar Aug 13 '22 19:08 OnkelDok

Hi onkeldok, had some troubles this week. Think i can test it next week. Sorry for that. Thanks a lot for your work! Udo

oduudo avatar Aug 18 '22 15:08 oduudo

@funnym0nk3y: the bug should be fixed now (with commit 53e54695d5318fb35f5e01102105cc0cbd2cf32f). Can you please verify and close this issue?

OnkelDok avatar Jul 17 '23 18:07 OnkelDok

Due to the lack of response, I will close the ticket. The issue is fixed with 53e54695d5318fb35f5e01102105cc0cbd2cf32f (Version 0.64.2).

OnkelDok avatar Aug 09 '23 20:08 OnkelDok