Fix SDK interactive question showing no question error incorrectly #42400
Conversation
|
| @@ -65,7 +65,7 @@ export const _InteractiveQuestion = ({ | |||
| const hasQuestionChanges = | |||
| card && (!card.id || card.id !== card.original_card_id); | |||
|
|
|||
| const [loading, setLoading] = useState(true); | |||
| const [isQuestionLoading, setIsQuestionLoading] = useState(true); | |||
There was a problem hiding this comment.
Renaming to make is obvious this means it's loading a question not running a question query
| } | ||
| }, [queryResults]); | ||
|
|
||
| if (loading) { | ||
| if (isQuestionLoading || isRunning) { |
There was a problem hiding this comment.
here's the key fix. We should display loading sprinner when either
- the question is loading
- the query is running
| }), | ||
| storeInitialState: state, | ||
| }); | ||
| return renderWithProviders( |
There was a problem hiding this comment.
return the result from renderWithProvider so I could access store and use it to dispatch actions.
| const storeDispatch = store.dispatch as unknown as ThunkDispatch< | ||
| State, | ||
| void, | ||
| AnyAction | ||
| >; |
There was a problem hiding this comment.
I didn't manage to find a way to use this without casting, let me know if there's any better way.
| storeDispatch(clearQueryResult()); | ||
| storeDispatch(runQuestionQuery()); |
There was a problem hiding this comment.
These 2 actions do:
- clear the results so it's as if we're about to query a new question when we're drilling
- query again
| @@ -98,6 +107,37 @@ describe("InteractiveQuestion", () => { | |||
| ).toBeInTheDocument(); | |||
| }); | |||
|
|
|||
| it("should render loading state when drilling down", async () => { | |||
There was a problem hiding this comment.
This test is tested with the previous version and it failed as expected.
...ise/frontend/src/embedding-sdk/components/public/InteractiveQuestion/InteractiveQuestion.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
The fix works on the demo app, nice! Verified that all three cases (drill-down, reset view, navigate back and forth) does not result in a Question Not Found error.
The renaming is a good idea too - I like that it semantically distinguishes query vs question.
(Ignore the height of the second container)
CleanShot.2567-05-09.at.18.13.49.mp4
WiNloSt
deleted the
42352-fix-interactive-question-incorrect-loading-state
branch
Closes #42352
Description
The problem happened because we only relied on
loadingwhich indicates whether the initial question has been loaded or not, after that there could be another loading state when users drills down.How to verify
Demo
Before
CleanShot.2567-05-08.at.15.57.13.mp4
After
https://www.loom.com/share/65437958d8ff461796e73de630a763b6
Checklist