-
Notifications
You must be signed in to change notification settings - Fork 15
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
All activities are ignored when excludeByCategory and limitByCategory are empty. #4
Conversation
…tegory are empty.
@@ -113,7 +113,7 @@ export default { | |||
} | |||
|
|||
const filteredValue = activityGroup.value.filter(item => { | |||
let r = false | |||
let r = true | |||
// Items must pass both tests, so we intentionally do not ELSE these. | |||
// If there are excludes, the item must NOT be excluded. | |||
if (this.excludeByCategory.length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code now has no sense.
We are setting a variable to true at the beginning.
We are not changing it to false during the road and only returning that variable.
Let's double-check again if this PR changes anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the logic behind this code is the following:
With let r = true
By default we allow any item from activityGroup
to be passed by setting
let r = true
If the value is present in the excludeByCategory
, then the
r = !this.excludeByCategory.includes(item.value.toString())
will result in false
, thus not adding the item into the resulting filteredValue
array.
Then the value is checked again using limitByCategory
array.
If the value is missing in this array, then the
r = this.limitByCategory.includes(item.value.toString())
will return false
once again, ensuring it is not in the resulting filteredValue
.
With let r = false
Now let's consider the case where:
let r = false
And where excludeByCategory
& limitByCategory
are both empty, i.e. the condition
if (this.excludeByCategory.length)
and
if (this.limitByCategory.length)
are never passed.
The code never gets to checking
r = !this.excludeByCategory.includes(item.value.toString())
and r = this.limitByCategory.includes(item.value.toString())
And it never gets a chance to change the r
to be true
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, you were right.
I've missed this line of code:
r = !this.excludeByCategory.includes(item.value.toString())
@@ -113,7 +113,7 @@ export default { | |||
} | |||
|
|||
const filteredValue = activityGroup.value.filter(item => { | |||
let r = false | |||
let r = true | |||
// Items must pass both tests, so we intentionally do not ELSE these. | |||
// If there are excludes, the item must NOT be excluded. | |||
if (this.excludeByCategory.length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, you were right.
I've missed this line of code:
r = !this.excludeByCategory.includes(item.value.toString())
Fix added to March release https://yusa.atlassian.net/browse/DS-414 |
#157 introduced a code that hides all activities when both
Limit by category and Exclude by category fields of the Activity Finder 4 block are not set.
Steps to reproduce: