-
Notifications
You must be signed in to change notification settings - Fork 112
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
Updated the query to return all relevant transactions #9719
base: main
Are you sure you want to change the base?
Updated the query to return all relevant transactions #9719
Conversation
Signed-off-by: Harsh Sawarkar <[email protected]>
…evant-Transactions
Signed-off-by: Harsh Sawarkar <[email protected]>
@@ -344,7 +345,8 @@ const getTransferDistinctTimestampsQuery = ( | |||
|
|||
// the condition to exclude synthetic transactions attached to a user submitted transaction | |||
const transactionByPayerExcludeSyntheticCondition = `${Transaction.getFullName(Transaction.NONCE)} = 0 or | |||
${Transaction.getFullName(Transaction.PARENT_CONSENSUS_TIMESTAMP)} is not null`; | |||
${Transaction.getFullName(Transaction.PARENT_CONSENSUS_TIMESTAMP)} is not null | |||
or ${Transaction.getFullName(Transaction.TYPE)} in (${typesToInclude.map((type) => `'${type}'`).join(', ')})`; |
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.
Once other comments are addressed, .map
can be removed. Also, we should only add condition if array is not empty.
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.
Should update the hedera-mirror-rest/__tests__/specs/transactions/account-id.json
spec test to include these types of transactions.
@@ -41,6 +41,10 @@ hedera: | |||
username: metrics | |||
uriPath: "/swagger" | |||
ipMetrics: false | |||
transactions: |
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 should be the protoId of these types. See TransactionType.java
.
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.
Also, should move this config to be under the query
config and change typesToInclude
to precedingTransactionTypes
and add config prop to configuration.md
Description:
Modified the query to return all relevant transaction
Related issue(s):
Fixes #9606
Notes for reviewer:
Checklist