[TO MAIN] DESENG-513 - Add poll results to results tab #2422
Conversation
CHANGELOG.MD
Outdated
| @@ -1,4 +1,10 @@ | |||
| <<<<<<< HEAD | |||
|
|
||
|
|
||
| def test_get_poll_response(client, jwt, session, setup_admin_user_and_claims): | ||
| """Assert that a response for a poll widget can be retrived.""" |
There was a problem hiding this comment.
| """Assert that a response for a poll widget can be retrived.""" | |
| """Assert that a response for a poll widget can be retrieved.""" |
| } catch (err) { | ||
| setIsWidgetsLoading(false); | ||
| dispatch(openNotification({ severity: 'error', text: 'Error fetching engagement widgets' })); | ||
| } |
There was a problem hiding this comment.
| } | |
| } finally { | |
| setItWidgetsLoading(false); | |
| } |
You could add this block in to make sure you set this state regardless of whether try or catch runs
There was a problem hiding this comment.
Doable. Will update other useEffect functions too.
| } | ||
| try { | ||
| const result = await fetchPollWidgets(widget.id); | ||
| setPollWidget(result[result.length - 1]); |
There was a problem hiding this comment.
Another way to get the last item in an array which is a bit more pythonic:
| setPollWidget(result[result.length - 1]); | |
| setPollWidget(result.at(-1)); |
There was a problem hiding this comment.
Doable, i need to update the interface type also to accept undefined
| <Box sx={{ mb: 2 }}> | ||
| <Typography variant="subtitle1">{label}</Typography> | ||
| <Box sx={{ display: 'flex', alignItems: 'center' }}> | ||
| <LinearProgress variant="determinate" value={percentage} sx={{ flexGrow: 1, mr: 1 }} /> |
There was a problem hiding this comment.
I'd be curious what a screenreader would say if you were to navigate over this component with a keyboard
| const response = await http.GetRequest<PollResultResponse>(url); | ||
| return response.data || Promise.reject('Failed to fetch Poll Results'); | ||
| } catch (err) { | ||
| return Promise.reject(err); |
There was a problem hiding this comment.
You shouldn't need to call Promise.reject() within an async prefixed function as this happens automatically. To reject the promise, you can just return the result of the catch block (or throw it if there's other code you want to execute within the catch block.)
| return Promise.reject(err); | |
| return err; |
There was a problem hiding this comment.
Agree. I followed the convention used in other service files.
| export const fetchPollResults = async (widget_id: number, poll_id: number): Promise<PollResultResponse> => { | ||
| try { | ||
| let url = replaceUrl(Endpoints.PollWidgets.RECORD_RESPONSE, 'widget_id', String(widget_id)); | ||
| url = replaceUrl(url, 'poll_id', String(poll_id)); |
There was a problem hiding this comment.
Why does url get immediately reassigned?
There was a problem hiding this comment.
As replaceUrl only replaces one string at a time and here we need to replace both widget_id and poll_id, we need to call it twice.
There was a problem hiding this comment.
Oh okay got it. Could replaceAllInURL be used to make it just one function call?
|
Issue #: https://apps.itsm.gov.bc.ca/jira/browse/DESENG-513
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the met-public license (Apache 2.0).