Page MenuHomePhabricator
Differential D17513

[Cashtab] [bugfix] Do not bring back old state on wallet change
ClosedPublic
Actions

Authored by bytesofman on Sat, Jan 11, 14:57.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC02f6a143cd87: [Cashtab] [bugfix] Do not bring back old state on wallet change
Summary

When wallet changing was initially implemented, there was an issue with the function being called too frequently -- for example if the user rapidly scrolls through wallets.

A complex debounce was implemented to prevent this from causing infinite looping between wallet switches.

However, the debounce uses useRef, which means that the function is defined with a certain associated cashtabState. When it is called and updates cashtab state, it changes the wallet on top of the old cashtab state (for example if the user has changed settings in the meantime, the old settings come back).

We can't accept this behavior. The problem of "user selects wallets too fast now there's an issue" is more tolerable -- also I am unable to repeat this legacy problem that caused the debounce implementation. Perhaps it was associated with some other past behavior the app that has been patched.

Test Plan

npm test

navigate to configure page, change send confirmations toggle
change wallet, note that settings change persists

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bytesofman created this revision.Sat, Jan 11, 14:57
Herald added a reviewer: Restricted Project. · View Herald TranscriptSat, Jan 11, 14:57
bytesofman updated this revision to Diff 52135.Sat, Jan 11, 14:59

remove debug log

bytesofman edited the summary of this revision. (Show Details)Sat, Jan 11, 15:02
Harbormaster completed remote builds in B32045: Diff 52134.Sat, Jan 11, 15:03
teamcity added a comment.Sat, Jan 11, 15:03
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Failed tests logs:

====== CashTab Unit Tests: <Agora /> We can see multiple offers, some we made, others we did not, and we can cancel an offer ======
Error: Unable to find an element with the text: 42.00 XEC. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

Ignored nodes: comments, script, style
<body>
  <div>
    <div
      class="sc-AnqlK jzmxJP"
    >
      <section
        aria-atomic="false"
        aria-label="Notifications Alt+T"
        aria-live="polite"
        aria-relevant="additions text"
        class="Toastify"
      />
      <div
        class="sc-keFjpB kpRoMq"
      >
        <div
          class="sc-jWojfa dWofVP"
        >
          <div
            class="sc-dTLGrV hVKRQa"
          >
            <img
              alt="cashtab"
              class="sc-ivVeuv evHkvQ"
              src="test-file-stub"
            />
          </div>
          <div
            class="sc-hdPSEv NMixZ"
          >
            <div
              class="sc-bqjOQT bJwGKq"
            >
              <select
                class="sc-jkCMRl kuXvHF"
                data-testid="wallet-select"
                id="wallets"
                name="wallets"
              >
                <option
                  class="sc-crNyjn fdrrlO"
                  value="Agora Partial Beta"
                >
                  Agora Partial Beta
                </option>
                <option
                  class="sc-crNyjn fdrrlO"
                  value="Agora Partial Alpha"
                >
                  Agora Partial Alpha
                </option>
              </select>
              <div
                class="sc-lcpuFF iSIgFF"
              >
                <button
                  aria-label="Copy ecash:qreq3mm4avxaw782g4qvhktx4qcv0w2tkqj3j5jaad"
                  class="sc-cJSrbW kZNvLw"
                >
                  <svg
                    title="copy-paste"
                  />
                </button>
                <div
                  class="sc-eerKOB cvXwwv"
                >
                  <div
                    class="sc-emmjRN bSWkUh"
                  >
                    <input
                      checked=""
                      class="sc-gFaPwZ iPJMNd"
                      id="show-hide-balance"
                      name="show-hide-balance"
                      title="show-hide-balance"
                      type="checkbox"
                    />
                    <label
                      class="sc-cpmLhU iUeRKn"
                      for="show-hide-balance"
                    >
                      <span
                        class="sc-dymIpo hGwQkt"
                        data-off=""
                        data-on=""
                      />
                      <span
                        class="sc-bnXvFD cpOMDW"
                      />
                    </label>
                  </div>
                </div>
              </div>
            </div>
            <div
              class="sc-cmIlrE dkSzlg"
              title="Wallet Info"
            >
              <div
                class="sc-cpHetk jEKVml"
              >
                <div
                  class="sc-nrwXf bsChRe"
                  title="Balance XEC"
                >
                  390.16
                   
                  XEC
                </div>
              </div>
              <div
                class="sc-eitiEO lmPAIm"
                title="Balance in Local Currency"
              >
                $
                0.01
                 
                USD
              </div>
              <p
                class="sc-bhlBdH umymk"
                title="Price in Local Currency"
              >
                1 
                XEC
                 = 
                0.00003000
                 
                USD
              </p>
            </div>
          </div>
          <div
            class="sc-chAAoq bzVCQv"
          >
            <div
              class="sc-yZwTr birQAH"
            >
              <h2>
                Token Offers
              </h2>
              <div>
                Sort by:
                <div
                  class="sc-hwcHae jnZGJM"
                  title="Sort by TokenId"
                >
                  TokenID
                </div>
                <div
                  class="sc-hwcHae itSnvW"
                  title="Sort by Offer Count"
                >
                  Offers
                </div>
                <span
                  class="sc-fjhmcy ceHQEJ"
                  title="Toggle Active Offers"
                >
                  My Listings
                </span>
              </div>
            </div>
            <div
              class="sc-gacfCG gpjPEu"
              title="Active Offers"
            >
              <div
                class="sc-cqPOvA jDuRbt"
              >
                <div
                  class="sc-drMfKT gHPyYN"
                >
                  ...
    at waitForWrapper (/work/cashtab/node_modules/@testing-library/dom/dist/wait-for.js:163:27)
    at /work/cashtab/node_modules/@testing-library/dom/dist/query-helpers.js:86:33
    at Object.findByText (/work/cashtab/src/components/Agora/__tests__/index.test.js:727:29)
    at runNextTicks (node:internal/process/task_queues:60:5)
    at listOnTimeout (node:internal/timers:545:9)
    at processTimers (node:internal/timers:519:7)

Each failure log is accessible here:
CashTab Unit Tests: <Agora /> We can see multiple offers, some we made, others we did not, and we can cancel an offer

Harbormaster completed remote builds in B32046: Diff 52135.Sat, Jan 11, 15:05
teamcity added a comment.Sat, Jan 11, 15:05
Build Bitcoin ABC Diffs / Diff Testing (cashtab-tests) failed.

Failed tests logs:

====== CashTab Unit Tests: <Agora /> Screen loads as expected if there are no agora partial listings ======
TestingLibraryElementError: Unable to find an element with the text: No whitelisted tokens are currently listed for sale. Try loading all offers.. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

Ignored nodes: comments, script, style
<body>
  <div>
    <div
      class="sc-AnqlK jzmxJP"
    >
      <section
        aria-atomic="false"
        aria-label="Notifications Alt+T"
        aria-live="polite"
        aria-relevant="additions text"
        class="Toastify"
      />
      <div
        class="sc-keFjpB kpRoMq"
      >
        <div
          class="sc-jWojfa dWofVP"
        >
          <div
            class="sc-dTLGrV hVKRQa"
          >
            <img
              alt="cashtab"
              class="sc-ivVeuv evHkvQ"
              src="test-file-stub"
            />
          </div>
          <div
            class="sc-hdPSEv NMixZ"
          >
            <div
              class="sc-bqjOQT bJwGKq"
            >
              <select
                class="sc-jkCMRl fVHViG"
                data-testid="wallet-select"
                id="wallets"
                name="wallets"
              >
                <option
                  class="sc-crNyjn fdrrlO"
                  value="Agora Partial Alpha"
                >
                  Agora Partial Alpha
                </option>
                <option
                  class="sc-crNyjn fdrrlO"
                  value="Agora Partial Beta"
                >
                  Agora Partial Beta
                </option>
              </select>
              <div
                class="sc-lcpuFF iSIgFF"
              >
                <button
                  aria-label="Copy ecash:qqpmsv8yh8wwx3lnf92rrc0e6yq97j6zqs8av8vx8h"
                  class="sc-cJSrbW kZNvLw"
                >
                  <svg
                    title="copy-paste"
                  />
                </button>
                <div
                  class="sc-eerKOB cvXwwv"
                >
                  <div
                    class="sc-emmjRN bSWkUh"
                  >
                    <input
                      checked=""
                      class="sc-gFaPwZ iPJMNd"
                      id="show-hide-balance"
                      name="show-hide-balance"
                      title="show-hide-balance"
                      type="checkbox"
                    />
                    <label
                      class="sc-cpmLhU iUeRKn"
                      for="show-hide-balance"
                    >
                      <span
                        class="sc-dymIpo hGwQkt"
                        data-off=""
                        data-on=""
                      />
                      <span
                        class="sc-bnXvFD cpOMDW"
                      />
                    </label>
                  </div>
                </div>
              </div>
            </div>
            <div
              class="sc-cmIlrE dkSzlg"
              title="Wallet Info"
            >
              <div
                class="sc-cpHetk jEKVml"
              >
                <div
                  class="sc-nrwXf bsChRe"
                  title="Balance XEC"
                >
                  4,200.00
                   
                  XEC
                </div>
              </div>
              <div
                class="sc-eitiEO lmPAIm"
                title="Balance in Local Currency"
              >
                $
                0.13
                 
                USD
              </div>
              <p
                class="sc-bhlBdH umymk"
                title="Price in Local Currency"
              >
                1 
                XEC
                 = 
                0.00003000
                 
                USD
              </p>
            </div>
          </div>
          <div
            class="sc-chAAoq bzVCQv"
          >
            <div
              class="sc-yZwTr birQAH"
            >
              <h2>
                Token Offers
              </h2>
              <div>
                Sort by:
                <div
                  class="sc-hwcHae jnZGJM"
                  title="Sort by TokenId"
                >
                  TokenID
                </div>
                <div
                  class="sc-hwcHae dfnqEL"
                  disabled=""
                  title="Sort by Offer Count"
                >
                  Offers
                  <div>
                    <div
                      class="sc-jzJRlG eQTBdJ"
                      title="Loading OrderBook info..."
                    >
                      <div />
                      <div />
                      <div />
                      <div />
                    </div>
                  </div>
                </div>
                <span
                  class="sc-fjhmcy ceHQEJ"
                  title="Toggle Active Offers"
                >
                  My Listings
                </span>
              </div>
            </div>
            <div
              class="sc-gacfCG gpjPEu"
              title="Active Offers"
            >
              <div
                class="sc-cqPOvA jDuRbt"
              >
                <div
                  class="sc-uJMKN fiyLOR"
                >
                  Error querying agora for active offers. Try again later.
                </div>
                <div
                  class="sc-uJMKN fiyLOR"
                >
                  Error querying agora for active offers. Try again later.
                </div>
              </div>
            </div>
            <button
              class="sc-caSCKo sc-kjoXOD DHATx"
              style="margin-top: 12px;"
            >
              Load all offers
            </button>
          </div>
        </div>
        <div
          class="sc-kVrTmx exLrES"
        >
          <div
            class="sc-fHSTwm jBcETl"
          >
            <img
              alt="cashtab"
              class="sc-ivVeuv evHkvQ"
              src="test-file-stub"
            />
          </div>
          <button
            class="sc-jvEmr hqZIZM"
          >
            <span>
              Transactions
            </span>
            <svg />
          </button>
          <button
            aria-label="Send Screen"
            class="sc-jvEmr hqZIZM"
          >
            <span>
              Send
            </span>
            <svg
              title="tx-sent"
            />
          </button>
          <button
            aria-label="Tokens"
            class="sc-jvEmr hqZIZM"
          >
            <span>
              Tokens
            </span>
            <svg
              title="Tokens"
            />
          </button>
          <button
            aria-label="Receive"
            class="sc-jvEmr hqZIZM"
          >
            <span>
              Receive
            </span>
            <svg
              title="tx-received"
            />
          </button>
          <div
            class="sc-ekkqgF dcQEnG"
            title="Show Other Screens"
          >
            <span
              class="sc-iBmynh dPVmzy"
            />
            <div
              class="sc-fKGOjr buAdOz"
              title="Other Screens"
            >
              <button
                class="sc-hycgNl bNyILw"
              >
                 
                <p>
                  Wallet Backup
                </p>
                <svg
                  title="wallet"
                />
              </button>
              <button
                class="sc-hycgNl bNyILw"
              >
                 
                <p>
                  Wallets
                </p>
                <svg
                  title="wallets"
                />
              </button>
              <button
                class="sc-hycgNl bNyILw"
              >
                 
                <p>
                  Listed NFTs
                </p>
                <svg
                  title="NFT"
                />
              </button>
              <button
                class="sc-hycgNl bNyILw"
              >
                 
                <p>
                  Agora
                </p>
                <svg
                  title="Meme Agora"
                />
              </button>
              <button
                class="sc-hycgNl bNyILw"
              >
                 
                <p>
                  Contacts
                </p>
                <svg
                  title="Contact List"
                />
              </button>
              <button
                class="sc-hycgNl bNyILw"
              >
                 
                <p>
                  Airdrop
                </p>
                <svg
                  title="tx-airdrop"
                />
              </button>
              <button
                class="sc-hycgNl bNyILw"
              >
                 
                <p>
                  Rewards
                </p>
                <svg
                  title="Cashtab Rewards"
                />
              </button>
              <button
                class="sc-hycgNl bNyILw"
              >
                <p>
                  Sign & Verify
                </p>
                <svg
                  class="sc-htpNat bPFBeM"
                />
              </button>
              <button
                class="sc-hycgNl bNyILw"
              >
                <p>
                  Settings
                </p>
                <svg
                  title="settings"
                />
              </button>
            </div>
          </div>
        </div>
      </div>
    </div>
  </div>
</body>
    at Object.getElementError (/work/cashtab/node_modules/@testing-library/dom/dist/config.js:37:19)
    at /work/cashtab/node_modules/@testing-library/dom/dist/query-helpers.js:76:38
    at /work/cashtab/node_modules/@testing-library/dom/dist/query-helpers.js:52:17
    at /work/cashtab/node_modules/@testing-library/dom/dist/query-helpers.js:95:19
    at Object.getByText (/work/cashtab/src/components/Agora/__tests__/index.test.js:143:20)
    at runNextTicks (node:internal/process/task_queues:60:5)
    at listOnTimeout (node:internal/timers:545:9)
    at processTimers (node:internal/timers:519:7)
====== CashTab Unit Tests: <Agora /> We can see multiple offers, some we made, others we did not, and we can cancel an offer ======
Error: Unable to find an element with the text: 42.00 XEC. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

Ignored nodes: comments, script, style
<body>
  <div>
    <div
      class="sc-AnqlK jzmxJP"
    >
      <section
        aria-atomic="false"
        aria-label="Notifications Alt+T"
        aria-live="polite"
        aria-relevant="additions text"
        class="Toastify"
      />
      <div
        class="sc-keFjpB kpRoMq"
      >
        <div
          class="sc-jWojfa dWofVP"
        >
          <div
            class="sc-dTLGrV hVKRQa"
          >
            <img
              alt="cashtab"
              class="sc-ivVeuv evHkvQ"
              src="test-file-stub"
            />
          </div>
          <div
            class="sc-hdPSEv NMixZ"
          >
            <div
              class="sc-bqjOQT bJwGKq"
            >
              <select
                class="sc-jkCMRl kuXvHF"
                data-testid="wallet-select"
                id="wallets"
                name="wallets"
              >
                <option
                  class="sc-crNyjn fdrrlO"
                  value="Agora Partial Beta"
                >
                  Agora Partial Beta
                </option>
                <option
                  class="sc-crNyjn fdrrlO"
                  value="Agora Partial Alpha"
                >
                  Agora Partial Alpha
                </option>
              </select>
              <div
                class="sc-lcpuFF iSIgFF"
              >
                <button
                  aria-label="Copy ecash:qreq3mm4avxaw782g4qvhktx4qcv0w2tkqj3j5jaad"
                  class="sc-cJSrbW kZNvLw"
                >
                  <svg
                    title="copy-paste"
                  />
                </button>
                <div
                  class="sc-eerKOB cvXwwv"
                >
                  <div
                    class="sc-emmjRN bSWkUh"
                  >
                    <input
                      checked=""
                      class="sc-gFaPwZ iPJMNd"
                      id="show-hide-balance"
                      name="show-hide-balance"
                      title="show-hide-balance"
                      type="checkbox"
                    />
                    <label
                      class="sc-cpmLhU iUeRKn"
                      for="show-hide-balance"
                    >
                      <span
                        class="sc-dymIpo hGwQkt"
                        data-off=""
                        data-on=""
                      />
                      <span
                        class="sc-bnXvFD cpOMDW"
                      />
                    </label>
                  </div>
                </div>
              </div>
            </div>
            <div
              class="sc-cmIlrE dkSzlg"
              title="Wallet Info"
            >
              <div
                class="sc-cpHetk jEKVml"
              >
                <div
                  class="sc-nrwXf bsChRe"
                  title="Balance XEC"
                >
                  390.16
                   
                  XEC
                </div>
              </div>
              <div
                class="sc-eitiEO lmPAIm"
                title="Balance in Local Currency"
              >
                $
                0.01
                 
                USD
              </div>
              <p
                class="sc-bhlBdH umymk"
                title="Price in Local Currency"
              >
                1 
                XEC
                 = 
                0.00003000
                 
                USD
              </p>
            </div>
          </div>
          <div
            class="sc-chAAoq bzVCQv"
          >
            <div
              class="sc-yZwTr birQAH"
            >
              <h2>
                Token Offers
              </h2>
              <div>
                Sort by:
                <div
                  class="sc-hwcHae jnZGJM"
                  title="Sort by TokenId"
                >
                  TokenID
                </div>
                <div
                  class="sc-hwcHae itSnvW"
                  title="Sort by Offer Count"
                >
                  Offers
                </div>
                <span
                  class="sc-fjhmcy ceHQEJ"
                  title="Toggle Active Offers"
                >
                  My Listings
                </span>
              </div>
            </div>
            <div
              class="sc-gacfCG gpjPEu"
              title="Active Offers"
            >
              <div
                class="sc-cqPOvA jDuRbt"
              >
                <div
                  class="sc-drMfKT gHPyYN"
                >
                  ...
    at waitForWrapper (/work/cashtab/node_modules/@testing-library/dom/dist/wait-for.js:163:27)
    at /work/cashtab/node_modules/@testing-library/dom/dist/query-helpers.js:86:33
    at Object.findByText (/work/cashtab/src/components/Agora/__tests__/index.test.js:727:29)
    at runNextTicks (node:internal/process/task_queues:60:5)
    at listOnTimeout (node:internal/timers:545:9)
    at processTimers (node:internal/timers:519:7)

Each failure log is accessible here:
CashTab Unit Tests: <Agora /> Screen loads as expected if there are no agora partial listings
CashTab Unit Tests: <Agora /> We can see multiple offers, some we made, others we did not, and we can cancel an offer

bytesofman updated this revision to Diff 52136.Sat, Jan 11, 15:08

patch test

bytesofman added inline comments.Sat, Jan 11, 15:10
cashtab/src/components/Agora/__tests__/index.test.js
727 ↗(On Diff #52136)

This is the actual balance of the agora partial beta wallet. The test before was "passing" because the alpha wallet has a balance of 42 (both wallets used to have the same balance, but the beta wallet balance was later changed to support xecx buy tests).

So, shows that the wallet change method is more performant than the use ref debounce. clearing up some other issues.

Harbormaster completed remote builds in B32047: Diff 52136.Sat, Jan 11, 15:14
bytesofman published this revision for review.Sat, Jan 11, 15:16
bytesofman added inline comments.Sat, Jan 11, 15:38
cashtab/src/components/Common/WalletLabel.js
87 ↗(On Diff #52136)

this is a dumb event to track so we do not persist this behavior

Fabien accepted this revision.Sat, Jan 11, 19:56
This revision is now accepted and ready to land.Sat, Jan 11, 19:56
Closed by commit rABC02f6a143cd87: [Cashtab] [bugfix] Do not bring back old state on wallet change (authored by bytesofman). · Explain WhySat, Jan 11, 20:20
This revision was automatically updated to reflect the committed changes.
bytesofman added a commit: rABC02f6a143cd87: [Cashtab] [bugfix] Do not bring back old state on wallet change.

Revision Contents
Changeset List

PathSize
cashtab/
4 lines
2 lines
src/
components/
Agora/
__tests__/
2 lines
Common/
33 lines

Diff 52138

View Options

cashtab/package-lock.json

Loading...
View Options

cashtab/package.json

Loading...
View Options

cashtab/src/components/Agora/__tests__/index.test.js

Loading...
View Options

cashtab/src/components/Common/WalletLabel.js

Loading...