Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: refactor txns pages to use generic RequestState #102029

Merged
merged 2 commits into from
May 8, 2023

Conversation

xinhaoz
Copy link
Member

@xinhaoz xinhaoz commented Apr 21, 2023

Commit 1

ui: refactor txns page to use generic RequestState

This commit refactors the txn fingerprints page component
and its redux store wrappers to use the new RequestState.
This allows us to nicely pass the entire request state to
the component instead of destructuring the response into
individual props. Related selectors are deleted.

Epic: none

Release note: None

Commit 2

ui: refactor txn details page to use RequestState typed prop

This commit Replaces the request state props (isDataValid, statements,
transaction, error, isLoading) in the txn details page with a single
prop containing the request state.

The logic that was previously in the selectTransaction (and being
duplicated in db-console and CC) has been moved to a new file,
transactionDetailsUtils with added unit testing.

Release note: None

Epic: CRDB-25476

@xinhaoz xinhaoz requested review from a team April 21, 2023 20:05
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@xinhaoz xinhaoz changed the title ui: refactor txns page to use generic RequestState ui: refactor txns pages to use generic RequestState Apr 21, 2023
@xinhaoz xinhaoz force-pushed the txns-request-refactor branch from 42bcb2a to b62de50 Compare April 21, 2023 20:33
xinhaoz added 2 commits May 8, 2023 15:00
This commit refactors the txn fingerprints page component
and its redux store wrappers to use the new RequestState.
This allows us to nicely pass the entire request state to
the component instead of destructuring the response into
individual props. Related selectors are deleted.

Epic: none

Release note: None
This commit Replaces the request state props (isDataValid, statements,
transaction, error, isLoading) in the txn details page with a single
prop containing the request state.

The logic that was previously in the selectTransaction (and being
duplicated in db-console and CC) has been moved to a new file,
`transactionDetailsUtils` with added unit testing.

Epic: none

Release note: None
@xinhaoz xinhaoz force-pushed the txns-request-refactor branch from b62de50 to 43b9763 Compare May 8, 2023 19:03
Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

Nit: Even if there are several commits, is still better to add all the info to the PR description, so all the information is in one place for reviewers to see.

Reviewed 9 of 11 files at r1, 2 of 2 files at r2, 7 of 7 files at r3, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @xinhaoz)

@xinhaoz
Copy link
Member Author

xinhaoz commented May 8, 2023

tftr!
bors r=maryliag

@craig
Copy link
Contributor

craig bot commented May 8, 2023

Build succeeded:

@craig craig bot merged commit 50b70df into cockroachdb:master May 8, 2023
@xinhaoz xinhaoz deleted the txns-request-refactor branch June 5, 2023 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants