hyperledger / fabric-samples

Samples for Hyperledger Fabric
https://wiki.hyperledger.org/display/fabric
Apache License 2.0
2.78k stars 3.37k forks source link

GetHistoryForKey added to the chaincode-typescript #1226

Closed ajiths10 closed 3 months ago

ajiths10 commented 3 months ago

Added a chaincode transaction to retrieve the complete history of an asset using its asset ID.

denyeart commented 3 months ago

@ajiths10 Thank you for submitting the PR.

Since asset-transfer-basic is the most fundamental sample, it should provide only the minimal necessary functions. Adding your proposed GetHistoryForKey to this sample might be too advanced.

If you believe that a sample TypeScript implementation of GetHistoryForKey is necessary, it would be more appropriate to implement the TypeScript version of the asset-transfer-ledger-queries sample, as it already includes the GetHistoryForKey functionality. What do you think about this approach?

Makes sense to me. Note that there is a chaincode-javascript reference with GetHistoryForKey() that can be used as reference. Ideally the various languages would have the same support in the asset-transfer-ledger-queries.

ajiths10 commented 3 months ago

Hi @satota2 ,

I believe GetHistoryForKey is a fundamental function available in many other languages, including JavaScript. Adding this functionality to TypeScript would be highly beneficial and valuable to the TypeScript community. Since I found it challenging to locate this code in community spaces, I decided to include it in a TypeScript sample myself.

What are your thoughts?

satota2 commented 3 months ago

@ajiths10

Thank you for your response.

While GetHistoryForKey is indeed a fundamental function as you mentioned, compared to the basic asset CRUD and transfer operations handled in asset-transfer-basic, its general usage frequency are relatively lower. Therefore, as I previously mentioned, I believe adding it to this simple scenario sample might be too much.

On the other hand, I agree that having a TypeScript sample for GetHistoryForKey would be very beneficial. So, as I previously suggested, I propose implementing a TypeScript version of the asset-transfer-ledger-queries sample. This sample includes other ledger query functionalities in addition to GetHistoryForKey, making it more valuable for TypeScript users.

Regarding the difficulty in finding the GetHistoryForKey functionality, I agree that it needs improvement. As a potential solution, including a note in the asset-transfer-ledger-queries description that mentions it contains GetHistoryForKey might help navigate users better.

What do you think?

ajiths10 commented 3 months ago

@ajiths10

Thank you for your response.

While GetHistoryForKey is indeed a fundamental function as you mentioned, compared to the basic asset CRUD and transfer operations handled in asset-transfer-basic, its general usage frequency are relatively lower. Therefore, as I previously mentioned, I believe adding it to this simple scenario sample might be too much.

On the other hand, I agree that having a TypeScript sample for GetHistoryForKey would be very beneficial. So, as I previously suggested, I propose implementing a TypeScript version of the asset-transfer-ledger-queries sample. This sample includes other ledger query functionalities in addition to GetHistoryForKey, making it more valuable for TypeScript users.

Regarding the difficulty in finding the GetHistoryForKey functionality, I agree that it needs improvement. As a potential solution, including a note in the asset-transfer-ledger-queries description that mentions it contains GetHistoryForKey might help navigate users better.

What do you think?

@satota2 , Yes, that sounds good!

satota2 commented 3 months ago

@ajiths10 Thank you for agreeing. I have created two issues based on the above discussion, so I would like to close this PR.

If you are interested in helping resolve these issues, I would greatly appreciate your collaboration. Would you be willing to help? Of course, even if you can assist with just one of the issues, it would be very welcome.

ajiths10 commented 3 months ago

@ajiths10 Thank you for agreeing. I have created two issues based on the above discussion, so I would like to close this PR.

If you are interested in helping resolve these issues, I would greatly appreciate your collaboration. Would you be willing to help? Of course, even if you can assist with just one of the issues, it would be very welcome.

@satota2 Thanks for addressing this. I look forward to it.

satota2 commented 3 months ago

@ajiths10 OK. I close this PR. If you are interested in taking on any of these issues, please leave a comment on the respective issue. I will then assign the issue to you. This will help avoid duplicate work by other comunity members (including possibly myself) on the same issue.