fix: Rectified the incorrect tuple value #1166

shamim-io · 2023-06-12T21:23:36Z

closes #1156

Closes Incorrect display of optional in tuple result #1156

Fixes issue: Incorrect display of optional in tuple result

Feature details
Rectified the incorrect tuple value

Feature image

vercel · 2023-06-12T21:23:40Z

@shamim-io is attempting to deploy a commit to the Hiro Systems Team on Vercel.

A member of the Team first needs to authorize it.

He1DAr · 2023-06-13T17:16:09Z

src/components/function-summary/value.tsx

-    const value = arg.repr.replace('u', '');
-    if (arg.name.includes('ustx')) {
+    const value = arg?.repr?.replace('u', '');
+    if (arg?.name?.includes('ustx')) {


I suggest we remove the optional operator ? whenever possible. It looks like it used to expect arg to always exist.

He1DAr · 2023-06-13T17:16:36Z

src/components/function-summary/result.tsx

+
+  const getReprValue = (item: any) => {
+    let reprValue = item?.value ?? 'none';
+    if (item?.type?.includes('list')) {


Same for this one

He1DAr · 2023-06-13T17:17:30Z

Thank you for the PR @shamim-io! Left a few comments, please let me know if you have any questions.

shamim-io · 2023-06-15T18:11:19Z

@He1DAr Can you please provide me a new tx link. The link that was provide to me earlier seems doesn't exist anymore. This was the link provided previously: https://explorer.hiro.so/txid/0x5644ab5008b78ada724946693bae8b64b221f84ed3234152e1553a1f978e8d3a?chain=testnet

He1DAr · 2023-06-16T13:18:30Z

Sure! https://explorer.hiro.so/txid/0x87844d48c19a0664fd8f270f247e6e4c29b7f72e895d7ab9572c9bc246f5867f?chain=testnet

shamim-io · 2023-06-18T09:28:00Z

@He1DAr I have updated the PR. Please check!

He1DAr

Awesome work @shamim-io! Left a few comments.

He1DAr · 2023-06-21T21:02:32Z

src/components/function-summary/result.tsx

+    if (item.type.includes('list')) {
+      reprValue = item.value.map((listEntry: any) => listEntry.value).join(', ');
+    }
+    return typeof reprValue === 'object' ? JSON.stringify(reprValue) : reprValue.toString();


Please try to avoid calling toString for the list case as it's already a string. A similar implementation like existing one should do the trick.

He1DAr · 2023-06-21T21:02:51Z

src/components/function-summary/result.tsx

@@ -15,6 +15,15 @@ export const FunctionSummaryResult = ({ result, txStatus }: FunctionSummaryResul
  if (!result) return null;
  const { success, type, value } = cvToJSON(hexToCV(result.hex));
  const hasType = !type?.includes('UnknownType');
+
+  const getReprValue = (item: any) => {


Please add typing to function argument and unit tests.

He1DAr · 2023-06-21T21:03:47Z

src/components/function-summary/value.tsx

@@ -77,7 +77,16 @@ const TupleResult = ({ tuple, isPoxAddr, btc }: any) => {
  );
 };

-const getValue = (arg: { name: string; type: any; repr: any; value: any }, btc: null | string) => {
+const isJSONString = (str: string) => {


I suggest we move this to utility file and add a unit test.

He1DAr · 2023-06-21T21:04:21Z

src/components/function-summary/value.tsx

+  }
+};
+
+const getValue = (arg: any, btc: null | string) => {


Please add typing to function argument.

He1DAr · 2023-06-21T21:06:33Z

src/components/function-summary/value.tsx

+  }
+};
+
+const getValue = (arg: any, btc: null | string) => {


Please add unit tests to getValue function that cover both cases (arg.repr is/isn't json).

vercel · 2023-06-26T13:33:01Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
hiro-explorer	🔄 Building (Inspect)			Jun 26, 2023 1:37pm

shamim-io · 2023-06-26T13:42:33Z

@He1DAr I have updated the PR by adding typings and test cases wherever possible. However, there are components that still lack test cases. In those cases, I have refrained from writing a test case for a small function to avoid duplicating efforts. Instead, we need to create a test file for the entire component.

He1DAr · 2023-06-28T19:51:18Z

src/components/function-summary/result.tsx

  if (!result) return null;
  const { success, type, value } = cvToJSON(hexToCV(result.hex));
  const hasType = !type?.includes('UnknownType');
+
+  const getReprValue = ({ type, value }: ReprValueProps) => {
+    let reprValue = value ?? 'none';


I would add .toString(); to the value here to match the old code. This part is pretty sensitive to changes.

He1DAr · 2023-06-28T19:51:50Z

src/components/function-summary/result.tsx

  if (!result) return null;
  const { success, type, value } = cvToJSON(hexToCV(result.hex));
  const hasType = !type?.includes('UnknownType');
+
+  const getReprValue = ({ type, value }: ReprValueProps) => {


I believe this function can be defined outside the component.

He1DAr · 2023-06-28T19:52:10Z

src/components/function-summary/value.spec.ts

@@ -0,0 +1,45 @@
+import { getValue } from './value';


Thank you for adding tests!

He1DAr · 2023-06-28T19:52:38Z

src/components/function-summary/result.tsx

+  const getReprValue = ({ type, value }: ReprValueProps) => {
+    let reprValue = value ?? 'none';
+    if (type.includes('list') && Array.isArray(value)) {
+      reprValue = value.map((listEntry: any) => listEntry.value).join(', ');


would be great if we eliminate this remaining any

He1DAr · 2023-06-28T19:53:58Z

@He1DAr I have updated the PR by adding typings and test cases wherever possible. However, there are components that still lack test cases. In those cases, I have refrained from writing a test case for a small function to avoid duplicating efforts. Instead, we need to create a test file for the entire component.

Sounds good! Thank you for adding tests. I left a few tiny comments. Great work @shamim-io!

blockstack-devops · 2023-09-20T14:36:29Z

🎉 This PR is included in version 1.123.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

shamim-io mentioned this pull request Jun 12, 2023

Incorrect display of optional in tuple result #1156

Closed

He1DAr reviewed Jun 13, 2023

View reviewed changes

He1DAr requested changes Jun 21, 2023

View reviewed changes

He1DAr reviewed Jun 28, 2023

View reviewed changes

src/components/function-summary/value.spec.ts

@@ -0,0 +1,45 @@

import { getValue } from './value';

Copy link

Collaborator

He1DAr Jun 28, 2023

Choose a reason for hiding this comment

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

Thank you for adding tests!

He1DAr reviewed Jun 28, 2023

View reviewed changes

shamim-io added 6 commits September 20, 2023 10:30

fix: rectified the incorrect tuple value

223d7ea

refactor: added condition to check corner cases

8fe0443

fix: removed ? wherever not required

f5d06d4

feat: added types and handled variables

6a510a3

feat: modified functions and added tests

5fd240b

fix: lint fix

3db5913

He1DAr force-pushed the fix/tupleRes branch from 57c2b1b to 3db5913 Compare September 20, 2023 14:35

He1DAr merged commit fe80f8b into hirosystems:main Sep 20, 2023
3 of 5 checks passed

blockstack-devops added the released label Sep 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Rectified the incorrect tuple value #1166

fix: Rectified the incorrect tuple value #1166

shamim-io commented Jun 12, 2023 •

edited by andresgalante

Loading

vercel bot commented Jun 12, 2023

He1DAr Jun 13, 2023

He1DAr Jun 13, 2023

He1DAr commented Jun 13, 2023

shamim-io commented Jun 15, 2023

He1DAr commented Jun 16, 2023

shamim-io commented Jun 18, 2023

He1DAr left a comment

He1DAr Jun 21, 2023

He1DAr Jun 21, 2023

He1DAr Jun 21, 2023

He1DAr Jun 21, 2023

He1DAr Jun 21, 2023

vercel bot commented Jun 26, 2023 •

edited

Loading

shamim-io commented Jun 26, 2023

He1DAr Jun 28, 2023

He1DAr Jun 28, 2023

He1DAr Jun 28, 2023

He1DAr Jun 28, 2023

He1DAr commented Jun 28, 2023

blockstack-devops commented Sep 20, 2023

fix: Rectified the incorrect tuple value #1166

fix: Rectified the incorrect tuple value #1166

Conversation

shamim-io commented Jun 12, 2023 • edited by andresgalante Loading

vercel bot commented Jun 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

He1DAr commented Jun 13, 2023

shamim-io commented Jun 15, 2023

He1DAr commented Jun 16, 2023

shamim-io commented Jun 18, 2023

He1DAr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Jun 26, 2023 • edited Loading

shamim-io commented Jun 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

He1DAr commented Jun 28, 2023

blockstack-devops commented Sep 20, 2023

shamim-io commented Jun 12, 2023 •

edited by andresgalante

Loading

vercel bot commented Jun 26, 2023 •

edited

Loading