ui: unify cluster-ui stores for Statement and Transaction Page

[Azhng]

Previously, on CC Conosle, Statement Page and Transaction Page were backed by
their own Redux stores and their own Redux Sagas. However, both Statement
and Transaction Page were backed by a single API endpoint, namely
/_status/statements. This resulted in unnecessary API calls and bugs
like #72009, which was caused by Reset SQL Stats sagas failing to
reset the Transaction Page Redux store.

This commit unifies the following Redux store and sagas into a single
store / sagas:

1. Statement Page store / sagas
2. Transaciton Page store / sagas
3. Reset SQL Stats store / sagas

This greatly removed the code duplication and test duplication and simplify
the logic. Statement Page and Transaction Page can reuse the same store
by their own selectors.

Resolves #72009

Release note: None

CC Console:

cc.mov

DB Console:

Screen.Recording.2021-11-09.at.23.07.17.mov

[cockroach-teamcity]

This change is 

[maryliag]

Reviewed 19 of 23 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Azhng and @maryliag)

---

pkg/ui/workspaces/cluster-ui/src/store/sagas.ts, line 31 at r1 (raw file):

    fork(notifificationsSaga),
    fork(sqlStatsSaga),
    fork(terminateSaga),

what is this one for?

---

pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.spec.ts, line 54 at r1 (raw file):

    });

    it("requests combined SQL Stats if combined=true in the request message", () => {

nit: test also with combined false

---

pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.ts, line 33 at r1 (raw file):

    yield put(resetSQLStatsCompleteAction());
    yield put(invalidateStatements());
    yield put(refreshStatements() as any);

we don't want to refresh sql stats here?

[Azhng]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @maryliag)

---

pkg/ui/workspaces/cluster-ui/src/store/sagas.ts, line 31 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

what is this one for?

Removed, I noticed we had an unused import for terminateSaga and thought we just missed it. I just remembered that we disabled that button already.

---

pkg/ui/workspaces/cluster-ui/src/store/sqlStats/sqlStats.sagas.spec.ts, line 54 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

nit: test also with combined false

Done.

---

pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.ts, line 33 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

we don't want to refresh sql stats here?

I am a bit uncertain about this. If we invalidate the state, this would cause the selector to return null, which in turn will trigger the componentDidUpdate() React component hook, which in turn will invoke the refreshStatement sagas anyway.

That's why I'm a bit not so sure if we should let the React component to handle this or if we should let the Sagas to handle this. Thoughts ?

[maryliag]

Reviewed 1 of 23 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @Azhng and @maryliag)

---

pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.ts, line 33 at r1 (raw file):

Previously, Azhng (Archer Zhang) wrote…

I am a bit uncertain about this. If we invalidate the state, this would cause the selector to return null, which in turn will trigger the componentDidUpdate() React component hook, which in turn will invoke the refreshStatement sagas anyway.

That's why I'm a bit not so sure if we should let the React component to handle this or if we should let the Sagas to handle this. Thoughts ?

If we keep the refresh here, is the refreshStatement being called twice? (here and on the component did mount), or if we call it here it doesn't get called there again?
If is being called twice, there isn't need for the refresh here, otherwise we can keep this here

[Azhng]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @maryliag)

---

pkg/ui/workspaces/db-console/src/redux/sqlStats/sqlStatsSagas.ts, line 33 at r1 (raw file):

Previously, maryliag (Marylia Gutierrez) wrote…

If we keep the refresh here, is the refreshStatement being called twice? (here and on the component did mount), or if we call it here it doesn't get called there again?
If is being called twice, there isn't need for the refresh here, otherwise we can keep this here

Hmm thinking about this a bit more, if we refresh here, then yes refreshStatement() will called twice, (once here and once on the component did update).

However I think that is probably fine, since refreshStatement() is only issuing refresh redux actions. Since this is backed by a CachedDataReducer, two subsequent refresh actions will only result in API calls.

I think letting the saga to deal with the refresh would be ideal for the long term, as we move to use functional components and no longer rely on React lifecycle method in the future.

Added refreshStatements() call here.

[maryliag]

Reviewed 4 of 4 files at r3, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @Azhng)

[Azhng]

TFTR!

bors r=maryliag

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build failed (retrying...):

• GitHub CI (Cockroach)

[craig]

Build failed:

• GitHub CI (Cockroach)

[Azhng]

bors r=maryliag

[craig]

Build succeeded:

• GitHub CI (Cockroach)