REK-155 Admin effectiveness analysis list in consultations and webinars, mocked details page#1136
Conversation
| <PageWrapper style={{boxShadow: "none"}}> | ||
| <TitleSection> | ||
| <Title level={3} style={{ margin: 0 }}> | ||
| Konsultacja Testowa |
| <ExpiryText type="secondary"> | ||
| <FormattedMessage | ||
| id="recording_available_until" | ||
| values={{ time: <ExpiryTime>9h 35m</ExpiryTime> }} |
| </Text> | ||
| <Space size="large"> | ||
| <RatingValue>6.75</RatingValue> | ||
| <RatingDescription type="secondary"> |
| }; | ||
|
|
||
| export const EMOTION_POOL = [ | ||
| { icon: '😳', key: 'surprised', labelId: 'emotion_surprised' }, |
| { icon: '🤢', key: 'disgusted', labelId: 'emotion_disgusted' }, | ||
| { icon: '🙁', key: 'sad', labelId: 'emotion_sad' }, | ||
| { icon: '😨', key: 'fearful', labelId: 'emotion_fearful' }, | ||
| { icon: '😆', key: 'happy', labelId: 'emotion_happy' }, |
| model_id: number; | ||
| model_name: string; | ||
| model_type: 'consultation' | string; | ||
| avg_attention: string; |
There was a problem hiding this comment.
type should be more type safety
|
|
||
| const getDetailsPath = useCallback((record: RecommenderTerm) => { | ||
| if (modelType === 'webinar') { | ||
| return `/courses/webinars/effectiveness-analysis/${record.model_id}/${record.id}`; |
There was a problem hiding this comment.
A dedicated effectiveness-analysis/:modelId/:id route was added only for consultations under /other/consultations/effectiveness-analysis/:modelId/:id. This looks like a real bug: clicking Details for a webinar will likely not navigate to the correct screen.
| font-size: 26px; | ||
| text-align: center; | ||
| `; | ||
| export const EffectivenessAnalysisDetails = () => { |
There was a problem hiding this comment.
You added a route with :modelId/:id, but the EffectivenessAnalysisDetails component itself appears to be fully mocked and does not use the route params at all. There is no sign of useParams, and the displayed data is generated locally using TOTAL_DURATION_SEC, getAttention, and Array.from(...).
| name: 'Name', | ||
| ID: 'ID', | ||
| effectiveness_analysis: 'Effectiveness analysis', | ||
| averange_attention: 'Average attention', |
| const actionRef = useRef<ActionType>(); | ||
| const [loading, setLoading] = useState(false); | ||
|
|
||
| const getRecommenderTerms = useCallback( |
| try { | ||
| const resFrames = await getAnalyticsChartFrames(analysisMeta.id, { interval: res }); | ||
| if (resFrames?.success && resFrames.data) { | ||
| const firstTs = new Date(resFrames.data[0].window_start).getTime(); |
There was a problem hiding this comment.
If resFrames.data is an empty array ([]), data[0] will be undefined and .window_start will throw a TypeError: Cannot read properties of undefined.
if (resFrames?.success && resFrames.data && resFrames.data.length > 0) { const firstTs = new Date(resFrames.data[0].window_start).getTime(); // ... }
| total: 0, | ||
| success: false, | ||
| }; | ||
| } |
There was a problem hiding this comment.
The new request handler has no error handling whatsoever. If the API returns an error related to the YouTube token, the user will not see a message and the TokenForm dialogue will not open.
| const [chartData, setChartData] = useState<ChartPoint[]>([]); | ||
| const Y_AXIS_WIDTH = 45; | ||
|
|
||
| const modelType = useMemo(() => { |
There was a problem hiding this comment.
Extracting information from a URL string is fragile. If someone changes the URL routing, it will break silently (returning “”). The route can pass the modelType as a parameter (it’s already in useParams but not in the path). Add modelType to the route path or pass it as a prop.
| { value: 300, label: <FormattedMessage id="time.minutes" values={{ value: 5 }} /> }, | ||
| ]; | ||
|
|
||
| const EffectivenessAnalysisDetails = () => { |
There was a problem hiding this comment.
The component is too large; it needs to be split into smaller ones with fewer responsibilities. A refactor is needed here.
| const [loading, setLoading] = useState<boolean>(false); | ||
| const [analysisMeta, setAnalysisMeta] = useState<AnalysisMeta | null>(null); | ||
| const [chartData, setChartData] = useState<ChartPoint[]>([]); | ||
| const Y_AXIS_WIDTH = 45; |
| width: '80px', | ||
| }, | ||
| { | ||
| title: <FormattedMessage id="dateRange" defaultMessage="Date Range" />, |
There was a problem hiding this comment.
A lot of code is being removed from the table here – are you sure that’s how it’s supposed t
2 participants
Close REK-155
Close REK-156
Close REK-157
Close REK-158