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

TNO-3016 Fix exclude content filters #2236

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

fbarreta
Copy link
Collaborator

@fbarreta fbarreta commented Aug 29, 2024

There are two fixes:
On line 830 there's a check on the section setting OverrideExcludeHistorical, when there's nothing to do with the exclude report content. So the feature is controlled only by the content setting ExcludeReports.
This fixes the reports using folders as data source.

On the line 854, there was a check that I believe was include by mistake on sectionSettings.RemoveDuplicates, that should be sectionSettings.OverrideExcludeHistorical (since the array is called excludeAboveAndHistorical), and also another problem that the on false was returning an empty array, and was overwriting the content that should be excluded by the excludeOnlyTheseContentIds.
This fixes the reports using filters as data source.

image
Report used to test content to be excluded.

image
Option to exclude report empty
image
Content

image
Option to exclude report filled
image
Content

@fbarreta fbarreta added the subscriber PR contains changes towards the subscriber application, label Aug 29, 2024
@fbarreta fbarreta requested a review from Fosol August 29, 2024 21:59
@fbarreta fbarreta self-assigned this Aug 29, 2024
@@ -851,9 +851,9 @@ public IEnumerable<ReportInstance> GetLatestInstances(int id, int? ownerId = nul

// Modify the query to exclude content.
var excludeOnlyTheseContentIds = excludeContentIds.Any() && !sectionSettings.OverrideExcludeHistorical ? excludeContentIds : Array.Empty<long>();
var excludeAboveAndHistorical = sectionSettings.RemoveDuplicates
var excludeAboveAndHistorical = sectionSettings.OverrideExcludeHistorical
Copy link
Collaborator

Choose a reason for hiding this comment

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

If a user selects the "Include all content from linked folder even if in prior report" in a section does this work? This is where the overrideExcludeHistorical setting comes from. It's a per-section configuration option. It ensures that that specific section will still include content that was also in a linked report. The reason for this is because Scott wants the folder content to show up no matter what.

image

Copy link
Collaborator Author

@fbarreta fbarreta Aug 29, 2024

Choose a reason for hiding this comment

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

It would, because the variable excludeOnlyTheseContentIds on line 853 is assigned like that:
var excludeOnlyTheseContentIds = excludeContentIds.Any() && !sectionSettings.OverrideExcludeHistorical ? excludeContentIds : Array.Empty();
It would only be populated if the Override is NOT checked.
In this case its checked, excludeOnlyTheseContentIds is empty and would not exclude those contents.

@Fosol Fosol merged commit b4869c2 into dev Aug 29, 2024
5 checks passed
@Fosol Fosol deleted the TNO-3016-exclude-content-from-reports branch August 29, 2024 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
subscriber PR contains changes towards the subscriber application,
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants