-
Notifications
You must be signed in to change notification settings - Fork 9
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
Isset purchase update fix #416
Conversation
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.
Few minor change requests, otherwise looks good. Thanks for the tests!!
Object.keys(item).includes(searchQuery.field) | ||
); | ||
const filteredSearchQueries = []; | ||
for (let i = 0; i < searchQueries.length; i++) { |
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.
I know we need to add some lint rules to prevent some of these back and forths we have on PRs, but please use array methods over for loops when possible.
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.
Done
@@ -439,6 +470,16 @@ class CriteriaCompletionChecker { | |||
} | |||
} | |||
|
|||
private issetCheck(matchObj: any): boolean { |
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.
If this is a json obj,
private issetCheck(matchObj: any): boolean { | |
private issetCheck(matchObj: Record<any,any>): boolean { |
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.
not a json object.
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.
then what is it? would prefer to be explicit here
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.
please properly type this. after that is done, can merge.
matchObj can be anything (string, boolean, double, integer)
…On Wed, Jul 10, 2024 at 7:41 PM Mitch Prewitt ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/anonymousUserTracking/criteriaCompletionChecker.ts
<#416 (comment)>
:
> @@ -439,6 +470,16 @@ class CriteriaCompletionChecker {
}
}
+ private issetCheck(matchObj: any): boolean {
then what is it? would prefer to be explicit here
—
Reply to this email directly, view it on GitHub
<#416 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BDZHOGNJ6IUOSMDKZNYPNGLZLU6JBAVCNFSM6AAAAABKMTS6PSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNRZGI2TSMJQGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
JIRA Ticket(s) if any
Description