enhance(apps/analytics): add illustrations for performance rates on activities and instances #4398
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces a comprehensive set of components and services for performance analytics in the KlickerUZH application. The changes focus on adding new functionality to track and visualize performance metrics for activities and instances within a course. The implementation includes new React components, GraphQL schema modifications, localization updates, and custom hooks to support detailed performance analysis. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (14)
packages/graphql/src/ops.ts (1)
2011-2022: Consider adding JSDoc comments to document the performance metrics.The
PerformanceRatestype contains various performance metrics that would benefit from clear documentation explaining what each rate represents and how it's calculated.Add JSDoc comments above the type and its fields:
+/** + * Represents various performance metrics for activities and instances. + */ export type PerformanceRates = { __typename?: 'PerformanceRates'; + /** Percentage of correct responses across all attempts */ correctRate: Scalars['Float']['output']; + /** Percentage of incorrect responses across all attempts */ errorRate: Scalars['Float']['output']; // Add similar comments for other fields... };packages/graphql/src/services/analytics.ts (1)
154-168: Reduce-based aggregation approach is well-structured.
The reducer pattern effectively handles merges from multiple arrays (practiceQuizzes, microLearnings). Consider splitting parts of this logic into helper functions if further complexity arises.packages/graphql/src/graphql/ops/QGetCoursePerformanceAnalytics.graphql (1)
29-45: Consider pagination for large datasetsThe query fetches all performances without pagination. For courses with many instances, this could impact performance.
Consider implementing pagination using cursor-based connections following the GraphQL Cursor Connections Specification.
Example structure:
instancePerformances(first: Int, after: String) { edges { cursor node { # current fields } } pageInfo { hasNextPage endCursor } }apps/frontend-manage/src/components/analytics/performance/PerformanceElementTypeFilter.tsx (1)
6-12: Consider using discriminated union for better type safetyThe type definition could be more explicit about the possible values.
Consider using a discriminated union:
type ElementTypeFilter = | { type: 'all' } | { type: 'specific'; value: ElementType }apps/frontend-manage/src/components/analytics/performance/PerformanceActivityTypeFilter.tsx (1)
6-34: Consider creating a generic filter componentThere's significant code duplication between PerformanceActivityTypeFilter and PerformanceElementTypeFilter.
Consider creating a generic filter component:
type FilterProps<T> = { value: T | 'all' setValue: Dispatch<SetStateAction<T | 'all'>> enumValues: Record<string, T> labelPrefix: string className?: string } function PerformanceFilter<T extends string>({ value, setValue, enumValues, labelPrefix, className, }: FilterProps<T>) { const t = useTranslations() return ( <SelectField label={t(`manage.analytics.${labelPrefix}`)} items={[ { value: 'all', label: t(`manage.analytics.all${labelPrefix}s`) }, ...Object.values(enumValues).map((value) => ({ value, label: t(`shared.generic.${value.toLowerCase()}s`), })), ]} value={value} onChange={(value) => setValue(value as T | 'all')} className={className} /> ) }apps/frontend-manage/src/components/analytics/performance/ActivitiesElementsSwitch.tsx (1)
12-12: Consider type-safe translationsThe translation keys are currently untyped, which could lead to runtime errors if keys are mistyped or missing.
Consider using a type-safe translation setup:
const t = useTranslations('analytics') // Namespace your translationsapps/frontend-manage/src/lib/hooks/usePerformanceRates.ts (2)
24-42: Simplify type filtering logicThe filtering logic uses nested conditionals which could be simplified for better readability.
Consider this more maintainable approach:
const filteredByType = searchResults.filter((entry) => { const isActivityMatch = entry.__typename === 'ActivityPerformance' && (activityType === 'all' || entry.activityType === activityType) const isInstanceMatch = entry.__typename === 'InstancePerformance' && (elementType === 'all' || entry.elementType === elementType) return isActivityMatch || isInstanceMatch })
9-15: Enhance type safety of PerformanceEntry interfaceThe interface could benefit from more precise types and documentation.
Consider these improvements:
/** * Represents a processed performance entry with calculated rates */ interface PerformanceEntry { /** Unique identifier of the performance entry */ id: number /** Display name of the activity or element */ name: string /** Percentage of incorrect attempts (0-100) */ incorrectRate: number /** Percentage of partially correct attempts (0-100) */ partialRate: number /** Percentage of correct attempts (0-100) */ correctRate: number }apps/frontend-manage/src/components/analytics/performance/PerformanceRatesBarChart.tsx (1)
3-27: Consider improving label contrast and responsivenessThe BarLabel component could be enhanced in two ways:
- The white text color might not be visible on light-colored bars
- The hardcoded font size might not scale well on different screen sizes
<text x={x + width / 2} y={y + height / 2 + 1} - fill="white" - fontSize={14} + fill={getContrastColor(backgroundColor)} + fontSize={Math.min(14, width / 5)} textAnchor="middle" dominantBaseline="middle" className="font-bold" >apps/frontend-manage/src/components/analytics/performance/PerformanceRates.tsx (2)
44-55: Consider using reducer for complex state managementThe component manages multiple related state variables. Using useReducer would make the state transitions more maintainable and testable.
- const [type, setType] = useState<'activity' | 'instance'>(defaultFilters.type) - const [attemptsType, setAttemptsType] = useState<'first' | 'last' | 'total'>( - defaultFilters.attemptsType - ) - const [activityType, setActivityType] = useState<ActivityType | 'all'>( - defaultFilters.activityType - ) - const [elementType, setElementType] = useState<ElementType | 'all'>( - defaultFilters.elementType - ) + const [filters, dispatch] = useReducer(filtersReducer, defaultFilters)
81-87: Simplify reset button disabled conditionThe current disabled condition is verbose and could be simplified for better readability.
- disabled={ - type === defaultFilters.type && - attemptsType === defaultFilters.attemptsType && - activityType === defaultFilters.activityType && - elementType === defaultFilters.elementType - } + disabled={Object.entries(defaultFilters).every( + ([key, value]) => filters[key] === value + )}packages/types/src/index.ts (1)
368-378: Consider adding JSDoc comments to document the rate calculations.The
PerformanceRatestype includes various performance metrics, but it's not immediately clear how these rates are calculated (e.g., what's the difference betweenerrorRateandfirstErrorRate).Add JSDoc comments to explain the calculation and meaning of each rate:
export type PerformanceRates = { + /** Percentage of first attempts that were incorrect */ firstErrorRate: number + /** Percentage of last attempts that were incorrect */ lastErrorRate: number + /** Percentage of all attempts that were incorrect */ errorRate: number // ... add similar comments for other rates }packages/graphql/src/public/server.json (1)
121-121: LGTM! Consider adding documentation for the new analytics fields.The enhanced GetCoursePerformanceAnalytics query provides comprehensive performance metrics. The structure looks good and aligns with the frontend requirements.
Consider adding documentation comments to describe:
- The calculation methodology for each rate type
- The differences between first/last/all attempts
- Any performance implications of the new fields
packages/graphql/src/ops.schema.json (1)
Line range hint
279-18517: Consider adding schema documentationWhile the schema structure is solid, consider adding descriptions to types and fields to improve schema documentation and developer experience.
Add descriptions to key types:
{ "kind": "OBJECT", "name": "ActivityPerformance", + "description": "Represents performance metrics for a course activity", ... } { "kind": "OBJECT", "name": "InstancePerformance", + "description": "Represents performance metrics for an activity instance", ... } { "kind": "OBJECT", "name": "PerformanceRates", + "description": "Contains detailed performance rate metrics including first attempt, last attempt and overall statistics", ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
apps/frontend-manage/src/components/analytics/performance/ActivitiesElementsSwitch.tsx(1 hunks)apps/frontend-manage/src/components/analytics/performance/PerformanceActivityTypeFilter.tsx(1 hunks)apps/frontend-manage/src/components/analytics/performance/PerformanceAttemptsFilter.tsx(1 hunks)apps/frontend-manage/src/components/analytics/performance/PerformanceElementTypeFilter.tsx(1 hunks)apps/frontend-manage/src/components/analytics/performance/PerformanceRates.tsx(1 hunks)apps/frontend-manage/src/components/analytics/performance/PerformanceRatesBarChart.tsx(1 hunks)apps/frontend-manage/src/components/analytics/performance/PerformanceSearchField.tsx(1 hunks)apps/frontend-manage/src/lib/hooks/usePerformanceRates.ts(1 hunks)apps/frontend-manage/src/lib/hooks/usePerformanceSearch.ts(1 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx(2 hunks)packages/graphql/src/graphql/ops/QGetCoursePerformanceAnalytics.graphql(1 hunks)packages/graphql/src/ops.schema.json(5 hunks)packages/graphql/src/ops.ts(6 hunks)packages/graphql/src/public/client.json(1 hunks)packages/graphql/src/public/schema.graphql(4 hunks)packages/graphql/src/public/server.json(1 hunks)packages/graphql/src/schema/analytics.ts(3 hunks)packages/graphql/src/services/analytics.ts(2 hunks)packages/i18n/messages/de.ts(1 hunks)packages/i18n/messages/en.ts(1 hunks)packages/types/src/index.ts(1 hunks)
🔥 Files not summarized due to errors (1)
- packages/graphql/src/ops.ts: Error: Server error: no LLM provider could handle the message
✅ Files skipped from review due to trivial changes (1)
- packages/graphql/src/public/client.json
🔇 Additional comments (39)
packages/graphql/src/ops.ts (3)
55-61: LGTM! Well-structured type definitions.
The ActivityPerformance and InstancePerformance types are well-defined with proper typing and consistent structure.
Also applies to: 874-880
362-364: LGTM! Consistent type extension.
The CoursePerformanceAnalytics type has been properly extended with the new performance-related fields.
4171-4171: LGTM! Well-structured GraphQL query document.
The GetCoursePerformanceAnalyticsDocument follows GraphQL best practices:
- Proper type safety with variables
- Comprehensive field selection
- Includes
__typenamefor proper Apollo cache management
packages/graphql/src/schema/analytics.ts (6)
2-7: Import statements look good.
These imports consolidate the newly introduced analytics types from "@klicker-uzh/types". Ensure that any future schema additions remain consistent with these type definitions.
170-184: New 'PerformanceRates' object type aligns well with the domain model.
Defining these fields (firstErrorRate, lastErrorRate, etc.) provides a granular breakdown of correct, partial, and error rates. Good work ensuring each rate is exposed as a float.
186-195: 'ActivityPerformance' object type is well-structured.
This enables a high-level view of performance metrics. Name and type fields help identify specific activities.
197-206: 'InstancePerformance' object type complements 'ActivityPerformance'.
Providing separate metrics for each element instance is a sound approach for more granular analytics.
212-213: New fields on ICoursePerformanceAnalytics enhance analytics coverage.
The addition of 'activityPerformances' and 'instancePerformances' arrays ensures that the schema fully exposes performance data at both the activity and instance level.
226-231: Exposing performance arrays in 'CoursePerformanceAnalytics' is consistent with the schema update.
These changes allow clients to query detailed performance data, rounding out the analytics experience.
packages/graphql/src/services/analytics.ts (9)
1-5: New imports for expanded performance metrics.
The added imports from "@klicker-uzh/types" show the extended analytics scope. Ensure that these types remain synchronized if upstream definitions change.
109-111: '_count' selection for participations is straightforward.
This approach efficiently retrieves participant totals. Make sure that other queries remain consistent if you rely on _count in other places.
128-142: Including 'progress', 'performance', and 'instancePerformance' for microLearnings as well.
Ensuring symmetry with practice quizzes is a solid design choice, enabling the same analytics structure for both. Keep an eye on performance if microLearnings scale.
147-150: Null check ensures the course is valid and has relevant analytics.
This prevents returning incomplete data when neither practiceQuizzes nor microLearnings exist.
170-193: Progress and basic performance data mapping look correct.
The approach properly distinguishes quiz and microlearning types (ActivityType.PRACTICE_QUIZ vs. ActivityType.MICRO_LEARNING). Great job.
195-218: Error, partial, and correct rate mapping is consistent.
You fallback to total rates if the first or last rate is null, which is logical. Good fallback strategy.
219-253: Instance performance extraction is handled correctly.
Flattening stacks to collect instancePerformance data ensures we capture all relevant metrics. Keep an eye on any future data growth for potential performance bottlenecks.
265-268: Returning new performance data fields.
Finally exposing the aggregated arrays (activityPerformances, instancePerformances) meets the updated schema’s contract. Looks good.
114-127: Including 'progress', 'performance', 'stacks' (with nested 'instancePerformance'), and ordering logic.
These additions expand the scope of data retrieval. Verify that any performance queries on practice quizzes remain performant, especially if the number of nested stacks grows large.
✅ Verification successful
Let me analyze the query patterns and performance implications. I'll search for any potential N+1 patterns or heavy nested queries.
Let me fix the regex and try again to analyze the query patterns.
Let me check one more aspect - the indexing on these tables to ensure the queries are optimized.
Based on the analysis of the codebase, I can now provide a final response about the performance implications of the practice quiz queries.
The nested queries are properly optimized and performant.
The implementation is safe for the following reasons:
- Proper indexing exists on critical fields (@@index on practiceQuizId, participantId)
- The nested includes are supported by composite indexes
- Queries use appropriate ordering (orderBy) clauses that match index patterns
- The stacks and elements queries are included in a single database round trip through Prisma's include mechanism, avoiding N+1 issues
The changes in the analytics service adding progress, performance, and nested stacks with instancePerformance are well-structured and follow the same optimized pattern used throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Checking for potential heavy queries or n+1 patterns in practice quiz queries
rg 'practiceQuizzes' -A 15
Length of output: 121166
Script:
#!/bin/bash
# Search for practice quiz queries with nested stacks/elements to check for N+1 patterns
ast-grep --pattern $'practiceQuiz.findMany({
$$$
include: {
stacks: {
include: {
elements: $$$
}
}
}
$$$
})'
# Also check for any direct stack/element queries that may cause N+1
rg -A 5 "stacks\s*:\s*{" packages/graphql/src/services/
Length of output: 295
Script:
#!/bin/bash
# Search for practice quiz queries with nested stacks/elements to check for N+1 patterns
ast-grep --pattern $'practiceQuiz.findMany({
$$$
include: {
stacks: {
include: {
elements: $$$
}
}
}
$$$
})'
# Also check for any direct stack/element queries that may cause N+1
rg "stacks: \{" -A 5 packages/graphql/src/services/
# Check for any batch loading or pagination implementations
rg "take:|skip:|cursor:" packages/graphql/src/services/
Length of output: 11066
Script:
#!/bin/bash
# Check for indexes in the Prisma schema
rg "@@index|@@unique" packages/prisma/src/prisma/schema/
# Look for any performance-related configurations
rg "orderBy|where|include" -A 3 packages/graphql/src/services/analytics.ts
Length of output: 6485
apps/frontend-manage/src/components/analytics/performance/PerformanceSearchField.tsx (3)
1-8: Props definition is clear and typed accurately.
Using discriminated unions ('activity' | 'instance') for the type prop is straightforward and reduces potential misuses.
10-14: Functional component signature is well-defined.
The destructured props improve readability. This is a typical React pattern done correctly.
15-31: Search field implementation is concise.
Good usage of TextField, translation labels, and placeholders. The styling is minimal and neat.
apps/frontend-manage/src/components/analytics/performance/PerformanceAttemptsFilter.tsx (2)
5-11: Prop definitions are type-safe.
Using 'first' | 'last' | 'total' enforces correct usage. The setter signature ensures consistent state management without confusion.
14-26: SelectField approach for attempts filtering is intuitive.
Translations are correctly used for labeling each option. The onChange handler casting is valid.
packages/graphql/src/graphql/ops/QGetCoursePerformanceAnalytics.graphql (1)
12-28: LGTM: Well-structured activity performance metrics
The activity performance metrics are well-organized with comprehensive rate tracking across different attempts (first/last) and outcomes (error/partial/correct).
apps/frontend-manage/src/components/analytics/performance/PerformanceRatesBarChart.tsx (1)
29-37: LGTM!
The props interface is well-structured with clear typing.
apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx (1)
62-65: LGTM!
The integration of PerformanceRates component is clean and maintains the existing pattern of error/loading states.
packages/types/src/index.ts (2)
365-367: LGTM! Well-organized section for learning analytics.
The new section is clearly marked and well-organized, following the existing pattern of section comments in the file.
380-385: LGTM! Well-structured performance types with consistent naming.
The ActivityPerformance and InstancePerformance types are well-structured and consistently use existing enums (ActivityType and ElementType). The naming is clear and follows the established patterns.
Also applies to: 387-392
packages/graphql/src/public/schema.graphql (4)
33-38: LGTM! Well-defined ActivityPerformance type.
The GraphQL type definition is consistent with the TypeScript interface and properly enforces non-nullable fields.
795-800: LGTM! Well-defined InstancePerformance type.
The GraphQL type definition is consistent with the TypeScript interface and properly enforces non-nullable fields.
1245-1255: LGTM! Well-defined PerformanceRates type.
The GraphQL type definition is consistent with the TypeScript interface and properly enforces non-nullable fields.
315-317: LGTM! Well-structured CoursePerformanceAnalytics modifications.
The new fields are properly added as non-nullable arrays of performance types, maintaining consistency with the rest of the schema.
packages/i18n/messages/en.ts (2)
1852-1855: LGTM! Clear and consistent performance rate labels.
The labels for performance rates are clear, concise, and follow existing terminology patterns.
1856-1871: LGTM! Comprehensive set of UI strings for analytics features.
The strings provide a complete set of labels for filters, categories, and actions in the analytics UI, with clear and user-friendly wording.
packages/i18n/messages/de.ts (1)
1864-1884: LGTM! Translations are accurate and comprehensive.
The German translations for the performance analytics features are well-structured and use appropriate technical terminology. They align well with the English counterparts while maintaining natural German language flow.
packages/graphql/src/ops.schema.json (4)
279-354: LGTM: Well-structured ActivityPerformance type definition
The ActivityPerformance type is well-defined with appropriate fields for tracking activity performance metrics. The structure includes essential metadata (id, name, type) and performance rates.
8817-8892: LGTM: Well-structured InstancePerformance type definition
The InstancePerformance type follows a consistent pattern with ActivityPerformance, providing instance-level performance tracking capabilities.
18362-18517: LGTM: Comprehensive PerformanceRates type definition
The PerformanceRates type provides a thorough set of metrics including:
- First attempt rates (correct/error/partial)
- Last attempt rates (correct/error/partial)
- Overall rates (correct/error/partial)
All fields are properly typed as non-null floats.
3636-3659: LGTM: CoursePerformanceAnalytics field additions
The new fields activityPerformances and instancePerformances are properly integrated into CoursePerformanceAnalytics, maintaining consistency with the existing schema structure.
Also applies to: 3684-3707
apps/frontend-manage/src/components/analytics/performance/PerformanceElementTypeFilter.tsx
Show resolved
Hide resolved
apps/frontend-manage/src/components/analytics/performance/PerformanceActivityTypeFilter.tsx
Show resolved
Hide resolved
apps/frontend-manage/src/components/analytics/performance/PerformanceActivityTypeFilter.tsx
Show resolved
Hide resolved
apps/frontend-manage/src/components/analytics/performance/ActivitiesElementsSwitch.tsx
Show resolved
Hide resolved
apps/frontend-manage/src/components/analytics/performance/PerformanceRatesBarChart.tsx
Show resolved
Hide resolved
apps/frontend-manage/src/components/analytics/performance/PerformanceRates.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/frontend-manage/src/components/analytics/performance/PerformanceRates.tsx (1)
158-169: 🛠️ Refactor suggestionAdd virtualization for performance optimization
The list of entries is rendered in a scrollable container without virtualization, which could cause performance issues with large datasets.
🧹 Nitpick comments (3)
apps/frontend-manage/src/components/analytics/performance/PerformanceRates.tsx (3)
27-57: Consider extracting constants for better maintainability.The chart colors and default filters could be moved to separate configuration files to improve maintainability and reusability.
Consider creating a theme configuration file and a constants file:
+// theme/colors.ts +export const CHART_COLORS = { + correct: '#064e3b', + partial: '#f59e0b', + incorrect: '#cc0000', +} +// constants/filters.ts +export const DEFAULT_PERFORMANCE_FILTERS = { + type: 'activity' as const, + attemptsType: 'total' as const, + activityType: 'all' as const, + elementType: 'all' as const, +}Then update the component:
+import { CHART_COLORS } from '@theme/colors' +import { DEFAULT_PERFORMANCE_FILTERS } from '@constants/filters' function PerformanceRates({ activityPerformances, instancePerformances, }: PerformanceRatesProps) { const t = useTranslations() - const chartColors = { - correct: '#064e3b', - partial: '#f59e0b', - incorrect: '#cc0000', - } - const defaultFilters = { - type: 'activity' as 'activity' | 'instance', - attemptsType: 'total' as 'first' | 'last' | 'total', - activityType: 'all' as ActivityType | 'all', - elementType: 'all' as ElementType | 'all', - }
137-156: Consider using responsive positioning for the legend.The legend's absolute positioning might cause layout issues on different screen sizes. Consider using a more responsive approach.
- <Legend - payload={[ - // ... payload items - ]} - wrapperStyle={{ top: 0, right: 0 }} - /> + <div className="flex justify-end mb-2"> + <Legend + payload={[ + // ... payload items + ]} + /> + </div>
158-158: Remove redundant condition check.The condition
entries.length > 0is redundant as it's already checked at line 135.- {entries.length > 0 && ( <div className="max-h-[13rem] overflow-y-scroll">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/frontend-manage/src/components/analytics/performance/PerformanceRates.tsx(1 hunks)apps/frontend-manage/src/lib/hooks/usePerformanceSearch.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/frontend-manage/src/lib/hooks/usePerformanceSearch.ts
🔇 Additional comments (2)
apps/frontend-manage/src/components/analytics/performance/PerformanceRates.tsx (2)
1-25: LGTM! Well-structured imports and type definitions.
The imports are organized logically, and the props interface is well-defined with appropriate TypeScript types.
58-72: LGTM! Well-organized search and filtering logic.
The use of custom hooks for search and performance rate calculations promotes code reusability and maintainability.
|
Summary by CodeRabbit
New Features
ActivitiesElementsSwitch,PerformanceActivityTypeFilter,PerformanceAttemptsFilter,PerformanceElementTypeFilter,PerformanceRates,PerformanceRatesBarChart, andPerformanceSearchField.Bug Fixes
Documentation
Chores