Render error tx844 #898

sohammk08 · 2023-12-19T05:58:23Z

High Level Overview of Change

Made changes in translations.json to exclude connection.server.publicKey from server_ledgers_hint

Context of Change

This change was made because the connection.server.publicKey in server_ledgers_hint in translations.json wasn't able to function properly. Upon closer inspection, it was found that the reason for this occurrence was the publicKey field being empty. Removed the connection.server.publicKey part and restructured sentences accordingly.

Type of Change

Translation Updates

TypeScript/Hooks Update

Updated files to React Hooks
Updated files to TypeScript

Before / After

Before:

"server_ledgers_hint": "This node {{connection.server.publicKey}} only contains ledgers {{connection.ledger.validated}}",

After:

"server_ledgers_hint": "This node only contains ledgers {{connection.ledger.validated}}",

Test Plan

Changed server_ledgers_hint in translations.json in a manner such that only the 'connection.ledger.validated' is used as there was some error with displaying 'connection.server.publicKey'

mvadari · 2024-01-04T21:36:06Z

public/locales/en-US/translations.json

@@ -200,7 +200,7 @@
  "invalid_transaction_hash": "The transaction hash is invalid",
  "ledger_not_found": "Ledger not Found",
  "check_ledger_id": "Please check your ledger id",
-  "server_ledgers_hint": "This node ({{connection.server.publicKey, truncate(length: 10)}}) only contains ledgers {{connection.ledger.validated}}",


I think we still want the pubkey to be printed if it's available, with some sort of "This Clio node only..." alternate message

So, you're saying that the clio-node-part still needs to be present in the code. What if I change the code such that whenever the pubKey is not empty, it gets displayed. But when it is empty, print it as "This clio node only..."

Would that be viable?

Yes, something like that sounds good.

pdp2121 · 2024-02-13T16:50:34Z

public/locales/es-ES/translations.json

@@ -196,7 +196,7 @@
  "invalid_transaction_hash": "El hash de la transacción es inválido",
  "ledger_not_found": "Libro Contable no Encontrado",
  "check_ledger_id": "Por favor, comprueba el id de tu libro contable",
-  "server_ledgers_hint": "Este nodo ({{connection.server.publicKey, truncate(length: 10)}}) solo contiene libros contables {{connection.ledger.validated}}",
+  "server_ledgers_hint": "Este nodo solo contiene libros contables {{connection.ledger.validated}}",


Also, just a note, if you are unsure about the translations in other languages, you can set them to null and others can fill in later. It would use the default en-US if set to null

sohammk08 · 2024-03-09T16:10:43Z

@ckniffen @pdp2121 @mvadari
I finally have an update. I managed to get the publicKey part working.

However, I'm still not sure about the part where we display server_ledgers_hint if publicKey does not exist and server_ledgers_hint_with_key if it does.

Here is where I currently am, for reference:

pdp2121 · 2024-04-02T16:43:41Z

@ckniffen @pdp2121 @mvadari I finally have an update. I managed to get the publicKey part working.

However, I'm still not sure about the part where we display server_ledgers_hint if publicKey does not exist and server_ledgers_hint_with_key if it does.

Here is where I currently am, for reference:

@sohammk08 You could use ternary operator to condition on whether the publicKey exists and display different messages based on that.

sohammk08 · 2024-04-06T18:08:15Z

@pdp2121 @mvadari @ckniffen I have solved the error and now, it works properly. Just to make sure, I have also tested to see if it works, and it does. I am giving below the method I used to test. I will be making a commit containing the changes soon after writing this comment. Below is how I tested it to make sure if it works or not. Take a look:

Original file:

  const notFound = title.includes('not_found')
  const hintMsg = hints.map((hint) => (
    <div className="hint" key={hint}>
      {t(hint as any, values)}
    </div>
  ))
  const derivedWarning = warning ?? (notFound && t('not_found'))

  return (
    <div className="no-match">
      <Helmet title={t(title as any)} />
      {isError && <div className="uh-oh">{t('uh_oh')}</div>}
      <div className="title">{t(title as any, values)}</div>
      {hintMsg}
      {(derivedWarning || isError) && (
        <div className="warning">
          <InfoIcon title={derivedWarning} />
          &nbsp;
          <span>{derivedWarning}</span>
        </div>
      )}
    </div>
  )
}

export default NoMatch

Changed file:

  const notFound = title.includes('not_found')

  // Determine which hint message to display based on the presence of publicKey
  const hintMsg = values.connection?.server?.publicKey
    ? t('server_ledgers_hint_with_key', values)
    : t('server_ledgers_hint', values)

  const derivedWarning = warning ?? (notFound && t('not_found'))

  return (
    <div className="no-match">
      <Helmet title={t(title as any)} />
      {isError && <div className="uh-oh">{t('uh_oh')}</div>}
      <div className="title">{t(title as any, values)}</div>
      {hintMsg && <div className="hint">{hintMsg}</div>}
      {(derivedWarning || isError) && (
        <div className="warning">
          <InfoIcon title={derivedWarning} />
          &nbsp;
          <span>{derivedWarning}</span>
        </div>
      )}
    </div>
  )
}

export default NoMatch

I also made changes to translations.json as such:

  "server_ledgers_hint": "This node only contains ledgers {{connection.ledger.validated}}",
  "server_ledgers_hint_with_key": "This node ({{connection.server.publicKey, truncate(length: 10)}}) only contains ledgers {{connection.ledger.validated}}",

Also, here is how I tested whether the change works or not;
First, I set the publicKey to null for debugging purposes. Then, I changed the hintMsg line. Here is how it looks in code:

  const notFound = title.includes('not_found')
  const publicKey = null

  // Determine which hint message to display based on the presence of publicKey
  const hintMsg = publicKey
    ? t('server_ledgers_hint_with_key', values)
    : t('server_ledgers_hint', values)

  const derivedWarning = warning ?? (notFound && t('not_found'))

I checked the logic after adding the above debugging code as said before. Please let me know if I should try and implement a test in NoMatch.test.tsx for the same. Also, as @pdp2121 suggested, I set the server_ledgers_hint_with_key for other language translations as null

Have a good day :)

…lorer into render-error-tx844

pdp2121 · 2024-04-16T15:36:30Z

@pdp2121 @mvadari @ckniffen I have solved the error and now, it works properly. Just to make sure, I have also tested to see if it works, and it does. I am giving below the method I used to test. I will be making a commit containing the changes soon after writing this comment. Below is how I tested it to make sure if it works or not. Take a look:

Original file:

  const notFound = title.includes('not_found')
  const hintMsg = hints.map((hint) => (
    <div className="hint" key={hint}>
      {t(hint as any, values)}
    </div>
  ))
  const derivedWarning = warning ?? (notFound && t('not_found'))

  return (
    <div className="no-match">
      <Helmet title={t(title as any)} />
      {isError && <div className="uh-oh">{t('uh_oh')}</div>}
      <div className="title">{t(title as any, values)}</div>
      {hintMsg}
      {(derivedWarning || isError) && (
        <div className="warning">
          <InfoIcon title={derivedWarning} />
          &nbsp;
          <span>{derivedWarning}</span>
        </div>
      )}
    </div>
  )
}

export default NoMatch

Changed file:

  const notFound = title.includes('not_found')

  // Determine which hint message to display based on the presence of publicKey
  const hintMsg = values.connection?.server?.publicKey
    ? t('server_ledgers_hint_with_key', values)
    : t('server_ledgers_hint', values)

  const derivedWarning = warning ?? (notFound && t('not_found'))

  return (
    <div className="no-match">
      <Helmet title={t(title as any)} />
      {isError && <div className="uh-oh">{t('uh_oh')}</div>}
      <div className="title">{t(title as any, values)}</div>
      {hintMsg && <div className="hint">{hintMsg}</div>}
      {(derivedWarning || isError) && (
        <div className="warning">
          <InfoIcon title={derivedWarning} />
          &nbsp;
          <span>{derivedWarning}</span>
        </div>
      )}
    </div>
  )
}

export default NoMatch

I also made changes to translations.json as such:

  "server_ledgers_hint": "This node only contains ledgers {{connection.ledger.validated}}",
  "server_ledgers_hint_with_key": "This node ({{connection.server.publicKey, truncate(length: 10)}}) only contains ledgers {{connection.ledger.validated}}",

Also, here is how I tested whether the change works or not; First, I set the publicKey to null for debugging purposes. Then, I changed the hintMsg line. Here is how it looks in code:

  const notFound = title.includes('not_found')
  const publicKey = null

  // Determine which hint message to display based on the presence of publicKey
  const hintMsg = publicKey
    ? t('server_ledgers_hint_with_key', values)
    : t('server_ledgers_hint', values)

  const derivedWarning = warning ?? (notFound && t('not_found'))

I checked the logic after adding the above debugging code as said before. Please let me know if I should try and implement a test in NoMatch.test.tsx for the same. Also, as @pdp2121 suggested, I set the server_ledgers_hint_with_key for other language translations as null

Have a good day :)

Hi @sohammk08, it would be nice if you could add a test inside NoMatch.test.tsx. Thanks for your contribution!

pdp2121 · 2024-04-16T15:39:16Z

src/containers/NoMatch/index.tsx

  const derivedWarning = warning ?? (notFound && t('not_found'))

  return (
    <div className="no-match">
      <Helmet title={t(title as any)} />
      {isError && <div className="uh-oh">{t('uh_oh')}</div>}
      <div className="title">{t(title as any, values)}</div>
-      {hintMsg}
+      {hintMsg && <div className="hint">{hintMsg}</div>}


I'm not sure why we need to change here. Would hintMsg ever be null? Also, are we not using hints array here at all?

Will implement the suggested change in the next commit

pdp2121 · 2024-05-02T18:19:04Z

Tests are failing since we are using node 18 instead.

sohammk08 · 2024-05-02T18:33:06Z

Tests are failing since we are using node 18 instead.

Yes. I checked the staging branch of the ripple/staging and all tests are passing. I ran npm run test and almost 11 tests are failing. Hopefully, I will be committing the revised code such that it doesn't fail any tests and.
Thanks for your input.

Good Day.

sohammk08 added 2 commits December 19, 2023 05:40

refactor: translations.json

9db41c9

Changed server_ledgers_hint in translations.json in a manner such that only the 'connection.ledger.validated' is used as there was some error with displaying 'connection.server.publicKey'

Merge branch 'ripple:staging' into render-error-tx844

908494f

sohammk08 mentioned this pull request Dec 19, 2023

Error rendering node's pubkey when there's no match on tx #844

Open

mvadari reviewed Jan 4, 2024

View reviewed changes

sohammk08 added 3 commits January 20, 2024 20:30

Merge branch 'ripple:staging' into render-error-tx844

32ef2c6

Merge branch 'staging' into render-error-tx844

f1f03d1

Merge branch 'ripple:staging' into render-error-tx844

59d96b7

pdp2121 reviewed Feb 13, 2024

View reviewed changes

ckeshava requested a review from pdp2121 February 26, 2024 17:46

Merge branch 'ripple:staging' into render-error-tx844

8dbf4b8

sohammk08 added 3 commits April 3, 2024 09:14

This commit is made to try and solve a issue in vercel deployment

e619909

pubKey displays properly

7839eb0

Merge branch 'staging' into render-error-tx844

2c74698

sohammk08 added 2 commits April 6, 2024 18:16

Resolved Issue ripple#844

5470806

Merge branch 'render-error-tx844' of https://github.com/sohammk08/exp…

df54121

…lorer into render-error-tx844

pdp2121 reviewed Apr 16, 2024

View reviewed changes

refactor NoMatch index.tsx

d05d2e1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Render error tx844 #898

Render error tx844 #898

sohammk08 commented Dec 19, 2023

mvadari Jan 4, 2024

sohammk08 Jan 5, 2024

mvadari Jan 31, 2024

pdp2121 Feb 13, 2024

sohammk08 commented Mar 9, 2024

pdp2121 commented Apr 2, 2024

sohammk08 commented Apr 6, 2024 •

edited

pdp2121 commented Apr 16, 2024

pdp2121 Apr 16, 2024

sohammk08 Apr 23, 2024 •

edited

pdp2121 commented May 2, 2024

sohammk08 commented May 2, 2024

Render error tx844 #898

Are you sure you want to change the base?

Render error tx844 #898

Conversation

sohammk08 commented Dec 19, 2023

High Level Overview of Change

Context of Change

Type of Change

TypeScript/Hooks Update

Before / After

Test Plan

mvadari Jan 4, 2024

Choose a reason for hiding this comment

sohammk08 Jan 5, 2024

Choose a reason for hiding this comment

mvadari Jan 31, 2024

Choose a reason for hiding this comment

pdp2121 Feb 13, 2024

Choose a reason for hiding this comment

sohammk08 commented Mar 9, 2024

pdp2121 commented Apr 2, 2024

sohammk08 commented Apr 6, 2024 • edited

pdp2121 commented Apr 16, 2024

pdp2121 Apr 16, 2024

Choose a reason for hiding this comment

sohammk08 Apr 23, 2024 • edited

Choose a reason for hiding this comment

pdp2121 commented May 2, 2024

sohammk08 commented May 2, 2024

sohammk08 commented Apr 6, 2024 •

edited

sohammk08 Apr 23, 2024 •

edited