-
Notifications
You must be signed in to change notification settings - Fork 14
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
enhance(apps/analytics): add illustration of participant activity performance as table #4412
enhance(apps/analytics): add illustration of participant activity performance as table #4412
Conversation
…e application header
Current Aviator status
This PR was merged manually (without Aviator). Merging manually can negatively impact the performance of the queue. Consider using Aviator next time.
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis pull request introduces a comprehensive update across multiple packages and applications, primarily focusing on updating the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
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 (
|
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
…nctionality to custom hooks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (19)
packages/graphql/src/services/analytics.ts (2)
438-450
: Building “participantActivityObject” is well-structured.
Storing participant info in a keyed record of participant ID for easy retrieval is a solid approach. Consider adding a short comment clarifying the aggregator’s purpose for future maintainers.
451-472
: Use forEach instead of map for side-effects.
Since the result of map is not used, using forEach would be clearer and avoids confusion.- activity.participantPerformances.map((performance) => { + activity.participantPerformances.forEach((performance) => {packages/graphql/src/ops.ts (2)
395-395
: Ensure naming consistency with existing properties.The new property "participantActivityPerformances" is well-named and matches the style of the surrounding properties. However, consider adding a short docstring to highlight its purpose, especially since there is a "participantPerformances" property as well. This will help future contributors quickly differentiate between the two arrays.
2012-2026
: Provide doc comments for newly introduced participant performance types.The new types "ParticipantActivityPerformance" and "ParticipantActivityPerformances" look consistent and well-structured. However, consider adding doc comments or inline comments describing each field’s purpose (e.g., explaining the scale of "completion"). This will improve clarity for maintainers and ensure correct interpretation in downstream integrations.
apps/frontend-manage/src/components/common/PreviewTag.tsx (1)
10-14
: Confirm alignment with design system's color palette.While the custom Tailwind classes are fine, consider verifying if the design system offers a built-in green variant for consistency.
apps/frontend-manage/src/pages/analytics/[courseId]/activity.tsx (1)
58-63
: Maintain consistent vertical alignment.The flex container around H1 and PreviewTag fosters a good layout. If there's a case where the text wraps to multiple lines, consider verifying the alignment.
apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/index.tsx (1)
113-118
: Ensure consistent styling between course name and tag
The combination of text sizing in the heading (H1) and the "text-base" class on PreviewTag may look misaligned on certain viewports. Verify the final layout to ensure consistent styling.apps/frontend-manage/src/components/common/Header.tsx (2)
54-54
: Conditional property usage
Setting “new: true” is a clean approach to conditionally add badges. Make sure the property name "new" is sufficiently clear or documented to avoid confusion with reserved words.
78-87
: Badge fosters discoverability
The "New" badge is clearly annotated. Ensure that "new" eventually transitions to a permanent label once the feature matures.apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx (1)
65-70
: Encourage feature toggle removal when stable
The PreviewTag within the Performance Dashboard may be hidden once the feature is generally available.apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx (4)
15-21
: Consider defining prop types with a TypeScript interface
While this works, a dedicated interface or type alias for StudentActivityPerformanceProps might be clearer for future maintainability.
31-38
: Activity toggle logic
The toggle logic is concise. Consider memoizing or extracting to a dedicated hook if reused in other components.
98-124
: Maintain user instructions
Providing user guidance while selecting activities is helpful. Consider adding a brief explanation about how to interpret the data table.
153-214
: Export CSV column naming
You left a TODO for better column naming in CSV exports. This is important for data clarity. Let me know if you want a plan to implement a custom CSV header logic.packages/types/src/index.ts (1)
398-403
: Check for potential extension fields.
Consider whether additional fields such as "maxScore" or "gradedAt" might be needed in the future. If not, the current shape looks concise and covers the essential metrics for performance tracking.packages/graphql/src/schema/analytics.ts (1)
332-337
: Check for large data returns.
Returning potentially large arrays of "ParticipantActivityPerformances" might affect performance. Consider pagination or lazy loading if the dataset grows significantly.packages/graphql/src/ops.schema.json (3)
4072-4095
: Add field description for better schema documentation.The
participantActivityPerformances
field is missing a description. Consider adding one to improve schema documentation and API discoverability.{ "name": "participantActivityPerformances", - "description": null, + "description": "Returns a list of activity performances grouped by participant", "args": [], "type": {
18561-18575
: Consider using ID scalar type instead of Int for id field.For consistency with GraphQL best practices, consider using the
ID
scalar type for theid
field instead ofInt
. TheID
scalar is designed specifically for unique identifiers.{ "name": "id", "description": null, "args": [], "type": { "kind": "NON_NULL", "name": null, "ofType": { - "kind": "SCALAR", - "name": "Int", + "kind": "SCALAR", + "name": "ID", "ofType": null } } }
18522-18677
: Add descriptions to all fields in new types for better documentation.Both new types and their fields are missing descriptions. Adding descriptions will improve schema documentation and make the API more self-documenting.
Key fields that would benefit from descriptions:
completion
: Explain the range (0-100%) and what it representstotalScore
: Clarify what this score represents and its possible rangeparticipantEmail
: Explain why this is nullableperformances
: Describe the relationship to ParticipantActivityPerformance
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
apps/auth/package.json
(1 hunks)apps/backend-docker/package.json
(1 hunks)apps/docs/package.json
(1 hunks)apps/frontend-control/package.json
(1 hunks)apps/frontend-manage/package.json
(1 hunks)apps/frontend-manage/src/components/analytics/overview/CourseDashboardList.tsx
(2 hunks)apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
(1 hunks)apps/frontend-manage/src/components/common/Header.tsx
(3 hunks)apps/frontend-manage/src/components/common/PreviewTag.tsx
(1 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/activity.tsx
(2 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx
(3 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/[id].tsx
(2 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/index.tsx
(2 hunks)apps/frontend-pwa/package.json
(1 hunks)apps/func-incoming-responses/package.json
(1 hunks)apps/func-response-processor/package.json
(1 hunks)apps/office-addin/package.json
(1 hunks)packages/graphql/src/graphql/ops/QGetCoursePerformanceAnalytics.graphql
(1 hunks)packages/graphql/src/ops.schema.json
(2 hunks)packages/graphql/src/ops.ts
(4 hunks)packages/graphql/src/public/client.json
(1 hunks)packages/graphql/src/public/schema.graphql
(2 hunks)packages/graphql/src/public/server.json
(1 hunks)packages/graphql/src/schema/analytics.ts
(4 hunks)packages/graphql/src/services/analytics.ts
(10 hunks)packages/i18n/messages/de.ts
(2 hunks)packages/i18n/messages/en.ts
(2 hunks)packages/markdown/package.json
(1 hunks)packages/shared-components/package.json
(1 hunks)packages/transactional/package.json
(1 hunks)packages/types/src/index.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (9)
- apps/frontend-manage/package.json
- apps/office-addin/package.json
- packages/transactional/package.json
- apps/func-response-processor/package.json
- apps/auth/package.json
- apps/backend-docker/package.json
- apps/frontend-control/package.json
- apps/docs/package.json
- packages/graphql/src/public/client.json
🔇 Additional comments (43)
apps/func-incoming-responses/package.json (1)
19-19
: Verify design system version consistency across all packages
The upgrade to @uzh-bf/design-system@3.0.0-alpha.36
should be consistent across all dependent packages to avoid version conflicts.
✅ Verification successful
Design system version is consistent across all packages
All packages in the repository are using the same version 3.0.0-alpha.36
of @uzh-bf/design-system
. The version upgrade is synchronized across all dependent packages, which maintains consistency and prevents potential version conflicts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for version consistency of @uzh-bf/design-system across all package.json files
echo "Checking @uzh-bf/design-system versions across all packages..."
fd package.json | xargs grep -l "@uzh-bf/design-system" | while read -r file; do
echo "=== $file ==="
jq -r '.dependencies["@uzh-bf/design-system"] // .devDependencies["@uzh-bf/design-system"] // .peerDependencies["@uzh-bf/design-system"]' "$file"
done
Length of output: 3748
packages/shared-components/package.json (1)
42-42
: Review design system changes for breaking updates
Since this is a shared components package and the design system is in alpha, please verify that the upgrade doesn't introduce breaking changes that could affect dependent packages.
✅ Verification successful
Design system version is consistent across packages
The version 3.0.0-alpha.36
of @uzh-bf/design-system
is consistently used across all dependent packages in the monorepo, including:
- packages/shared-components
- packages/transactional
- packages/markdown
- apps/frontend-pwa
- apps/frontend-manage
- apps/frontend-control
- apps/docs
- apps/auth
and other packages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find components that might be affected by design system changes
echo "Searching for design system component usage..."
rg -l "@uzh-bf/design-system" --type ts --type tsx
Length of output: 181
Script:
#!/bin/bash
# Description: Find design system component usage with correct file extensions
# Note: Using --type-add to support tsx files
echo "Searching for design system component usage..."
rg --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' -l "@uzh-bf/design-system"
echo -e "\nChecking package.json files for design system dependency..."
fd "package.json" --type f --exec grep -l "@uzh-bf/design-system" {}
Length of output: 20887
Script:
#!/bin/bash
# Description: Check the version of @uzh-bf/design-system in other packages to verify consistency
echo "Checking @uzh-bf/design-system versions across packages..."
fd "package.json" --type f --exec grep -A 1 -B 1 '"@uzh-bf/design-system"' {}
Length of output: 1535
packages/markdown/package.json (1)
43-43
: Test markdown rendering with updated design system
Since this package handles markdown rendering, please verify that the design system upgrade doesn't affect markdown component styling or functionality.
apps/frontend-pwa/package.json (1)
29-29
: Verify the design system upgrade impact on new features
The upgrade of @uzh-bf/design-system
to 3.0.0-alpha.36
looks good, but since this is an alpha version and the PR introduces new participant activity performance features:
- Please verify there are no breaking changes in the changelog that might affect the new table implementation
- Ensure the new participant activity performance table renders correctly with the updated design system components
Run this script to check for relevant changes:
✅ Verification successful
Based on the results from the shell scripts, I can see that the design system is used in several frontend components, but I need to check the changelog to understand if there are any breaking changes that might affect tables or other components used in the participant activity performance feature.
Based on the search results, I can see that the participant activity performance feature is primarily implemented in the analytics service and doesn't directly depend on the design system's table components. However, let me check one more aspect to ensure there are no breaking changes in the design system that might affect the frontend display.
Based on the search results, I can see that the participant activity performance feature uses components from the design system, but it's primarily implemented in the analytics service with frontend visualization components. The upgrade from 3.0.0-alpha.35 to 3.0.0-alpha.36 appears to be a minor version bump within the same alpha series, and the table implementations shown in the codebase don't indicate any breaking changes.
Proceed with the design system upgrade
The upgrade from alpha.35 to alpha.36 should be safe as:
- The participant activity performance feature uses standard design system components
- The changes are within the same alpha series (3.0.0-alpha.x)
- The table implementations in the shared components remain stable
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for breaking changes in the design system that might affect tables
# Look for table-related changes in the codebase
rg -g '!node_modules' -g '!*.lock' -g '!dist' -l 'Table|table'
# Check for design system migration guides or changelogs
fd -g 'CHANGELOG.md' -g 'MIGRATION.md'
Length of output: 13060
Script:
#!/bin/bash
# Let's check the actual changes in the participant activity performance components
# and their usage of design system components
# Check for recent changes in activity performance related files
rg -g '!node_modules' -g '!*.lock' -g '!dist' -A 5 '@uzh-bf/design-system' apps/frontend-pwa/src/components/course/
# Check the CHANGELOG.md in the repository root
cat CHANGELOG.md
Length of output: 66856
Script:
#!/bin/bash
# Check for any recent changes in participant activity performance related files
rg -g '!node_modules' -g '!*.lock' -g '!dist' -A 5 'participant.*activity.*performance' apps/analytics/src/
# Check for any recent changes in table-related components in the design system package
rg -g '!node_modules' -g '!*.lock' -g '!dist' -A 5 '@uzh-bf/design-system.*Table' packages/shared-components/src/
Length of output: 9941
Script:
#!/bin/bash
# Check for any frontend components that might be using the design system for participant activity performance
rg -g '!node_modules' -g '!*.lock' -g '!dist' -A 5 'StudentActivityPerformance|TotalStudentPerformancePlot' apps/frontend-manage/src/
# Check for any recent changes in table-related components that might be affected
rg -g '!node_modules' -g '!*.lock' -g '!dist' -A 5 'DataTable' packages/shared-components/src/
Length of output: 7838
packages/graphql/src/services/analytics.ts (9)
9-9
: Imports look good.
The new imports for Participant and ParticipantActivityPerformance are consistent with the added logic. Ensure that any references in other files are likewise updated or remain compatible.
Also applies to: 13-13, 22-22
330-333
: No issues with the reduce approach.
Merging practiceQuizzes and microLearnings followed by reduce is a clean, concise way to collate analytics data.
477-484
: Good final step of transforming the record to an array.
Converting the object to a list ensures a stable format for front-end consumption.
559-559
: Retaining an empty array is correct.
Returning an empty participantActivityPerformances array for courses with no activities prevents runtime crashes and maintains a consistent API shape.
568-575
: computeActivityInstancePerformance usage is clear.
Destructuring the returned analytics avoids confusion and keeps the codebase organized.
585-585
: Participant activity performances are properly returned.
Exposing participantActivityPerformances from getCoursePerformanceAnalytics merges seamlessly with the rest of the analytics return type.
445-446
: Reevaluate exposing user emails.
You are returning participantEmail, which may be sensitive information. Verify that sharing this personal detail is acceptable, or provide a mechanism to restrict access to authorized users only.
✅ Verification successful
Let me gather more information about how this email field is being used and what access controls might be in place.
Let me search for more context about how this email field is used and any authentication mechanisms in place.
Based on the search results, I can now provide a final response:
Email access is appropriately restricted to authorized users only.
The getCoursePerformanceAnalytics
endpoint is protected by authentication and role-based access control. It's only accessible to authenticated users with the USER
role (course owners/instructors), as evidenced by the asUser
authorization check in the schema. The service additionally verifies course ownership by checking ownerId: ctx.user.sub
when fetching course data. This ensures participant emails are only exposed to legitimate course instructors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# First, let's see the context around this analytics service
rg -B 5 -A 10 "participantEmail" packages/graphql/src/services/analytics.ts
# Let's also check for any auth/permission checks related to this
rg -B 3 -A 3 "hasPermission|isAuthorized|auth" packages/graphql/src/services/analytics.ts
# Check GraphQL schema to see how this field is exposed
fd "schema.ts|schema.graphql" --exec cat {}
Length of output: 48166
Script:
#!/bin/bash
# Check for authentication/authorization mechanisms around analytics
rg -B 5 -A 5 "analytics" packages/graphql/src/resolvers/
# Look for the resolver that uses this analytics service
rg -B 5 -A 5 "getCoursePerformanceAnalytics" packages/graphql/src/
# Check for any access control middleware or authentication checks
rg -B 3 -A 3 "middleware|authenticate|authorize" packages/graphql/src/
Length of output: 56452
312-314
: Check for null or undefined participantPerformances.
The additions of participantPerformances to both PracticeQuiz and MicroLearning seem valid. However, if participantPerformances is ever null or undefined, this code may fail. Consider ensuring Prisma always returns an empty array if no records exist.
Also applies to: 324-326
✅ Verification successful
Let me verify how participantPerformances is being handled by examining the code.
These scripts will help us understand:
- How participantPerformances is used throughout the codebase
- How it's defined in Prisma models
- Whether there are any existing null checks
- The context of its usage in queries
Based on the search results, I can now provide a conclusive response about the participantPerformances handling:
Prisma schema ensures participantPerformances is non-null array.
The Prisma schema defines participantPerformances as a non-null array relation (ParticipantActivityPerformance[]
) in both PracticeQuiz and MicroLearning models. Additionally, the GraphQL schema enforces this with [ParticipantPerformance!]!
type definition. The implementation in analytics.ts also shows that the field is always initialized as an array, even when no records exist. No additional null checks are needed.
The evidence for this conclusion comes from:
- Prisma schema showing non-null array relation:
participantPerformances ParticipantActivityPerformance[]
- GraphQL schema defining it as non-null array:
participantPerformances: [ParticipantPerformance!]!
- Service implementation in analytics.ts always including participantPerformances in the return object
- No nullable types or optional chaining found in any usage of participantPerformances
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for participantPerformances usage and its context
rg "participantPerformances" -A 5 -B 5
Length of output: 62008
Script:
#!/bin/bash
# Look for Prisma query implementations related to participantPerformances
ast-grep --pattern 'participantPerformances: $_'
Length of output: 50
Script:
#!/bin/bash
# Search for Prisma model definitions related to participantPerformances
fd "schema.prisma" --exec cat {}
Length of output: 34
Script:
#!/bin/bash
# Look for any null checks on participantPerformances
rg "participantPerformances(\s+)?\?" -A 2 -B 2
Length of output: 48
502-506
: Including participant data ensures consistency of the analytics.
Fetching participant details is necessary for the new functionality. Ensure any queries that do not include participantPerformances gracefully handle missing data.
Also applies to: 524-528
✅ Verification successful
Let me gather more information about the analytics queries and participant data handling in the codebase.
Let me search specifically for the analytics queries and their usage to better understand the participant data handling.
Let me search for the specific analytics service implementation to understand how participant data is handled.
Based on the comprehensive review of the analytics service implementation and related code, I can now generate the final response.
Including participant data is essential for analytics functionality and is handled appropriately.
The participant data inclusion is a core requirement for the analytics service as it:
- Enables computation of participant performance metrics
- Is used to track individual progress and completion rates
- Provides necessary data for aggregating course-wide analytics
- Has proper error handling and null checks in place
The code shows that queries without participantPerformances will naturally handle missing data through null checks and default values in the analytics computation functions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for queries related to participantPerformances
rg -A 5 "participantPerformances.*include" --type ts
# Look for other analytics queries that might need participant data
ast-grep --pattern 'prisma.analytics.findMany({
$$$
})'
# Search for any error handling related to participant data
rg -A 3 "participant.*undefined|participant.*null" --type ts
Length of output: 32723
Script:
#!/bin/bash
# Search for analytics-related queries and their participant data handling
ast-grep --pattern 'prisma.analytics.findMany({
include: {
participantPerformances: {
$$$
}
}
})'
# Look for the schema definition of analytics
rg -A 10 "analytics.*prisma" --type ts
# Search for any error handling or data processing related to participantPerformances
rg -A 5 "participantPerformances.*null|participantPerformances.*undefined" --type ts
Length of output: 2665
Script:
#!/bin/bash
# Search for the analytics service implementation
rg -A 15 "class.*Analytics|function.*Analytics" --type ts
# Look for the specific file and surrounding context
cat packages/graphql/src/services/analytics.ts
# Search for any error handling in analytics queries
rg -A 5 "try.*catch.*analytics" --type ts
Length of output: 46629
packages/graphql/src/ops.ts (1)
3795-3795
: Verify usage of the new field in the query responses.
The "GetCoursePerformanceAnalyticsQuery" type has been updated to include "participantActivityPerformances." Ensure all consumers of this query handle or display the new data properly. In addition, confirm that existing parts of the application aren’t affected by this addition, such as UI components or transformations that previously assumed certain data structures.
✅ Verification successful
Let me search for usages of this query to verify its impact.
Let me try another search to find UI components and usages with different file extensions.
Let me search for any UI components or code that might be consuming this data.
Let me check the performance component to see how it's using the data.
Let me check the StudentActivityPerformance component since it's consuming the participantActivityPerformances data.
Based on the code analysis, I can now generate the final response:
The new participantActivityPerformances field is properly integrated and handled in the UI.
The field is correctly consumed in the StudentActivityPerformance component, which:
- Displays participant activity data in a data table with username and email
- Allows selection of specific activities to view
- Shows both score and completion percentage for each activity
- Includes proper loading states and empty state handling
- Provides CSV export functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for GetCoursePerformanceAnalyticsQuery usages
rg "GetCoursePerformanceAnalyticsQuery" -B 2 -A 2
# Search for any GraphQL query operations with getCoursePerformanceAnalytics
ast-grep --pattern 'query {
getCoursePerformanceAnalytics {
$$$
}
}'
# Search for potential UI components that might consume this data
rg "getCoursePerformanceAnalytics" --type=tsx --type=jsx -B 2 -A 2
Length of output: 29616
Script:
#!/bin/bash
# Search for files containing getCoursePerformanceAnalytics
fd -e ts -e tsx -e jsx -e js | xargs rg "getCoursePerformanceAnalytics" -B 2 -A 2
# Search for any imports or uses of the query type
rg "GetCoursePerformanceAnalyticsQuery" -g '*.{ts,tsx,jsx,js}' -B 2 -A 2
Length of output: 58191
Script:
#!/bin/bash
# Search for files in the performance analytics related paths
fd -e tsx -e ts "performance" apps/frontend-manage/src/pages/analytics
# Search for any components that might be using the participant activity performances
rg -g '*.{tsx,ts}' "participantActivityPerformances" apps/frontend-manage/src/
Length of output: 353
Script:
#!/bin/bash
# Get the content of the performance component
cat apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx
# Look for any related components that might be consuming the performances data
rg -g '*.{tsx,ts}' "performances: Array<" apps/frontend-manage/src/
Length of output: 7612
Script:
#!/bin/bash
# Get the content of the StudentActivityPerformance component
cat apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
# Also look for any types or interfaces related to this component
rg -g '*.{ts,tsx}' "interface StudentActivityPerformance" apps/frontend-manage/src/
Length of output: 7770
apps/frontend-manage/src/components/common/PreviewTag.tsx (1)
5-6
: Ensure translations exist for "featurePreview".
You’re using t('shared.generic.featurePreview'), so please verify that all relevant locales (e.g., de, en, fr, etc.) have a corresponding key to avoid runtime translation errors.
apps/frontend-manage/src/components/analytics/overview/CourseDashboardList.tsx (2)
4-4
: Module import path is correct.
The import from '~/components/common/PreviewTag' is properly set. Make sure the alias ~/ resolves correctly in this context.
18-23
: Header layout improvements.
Grouping the heading and the PreviewTag in a flex container is a solid approach. This enhances the presentation without complicating the JSX.
apps/frontend-manage/src/pages/analytics/[courseId]/activity.tsx (1)
7-7
: Validate import usage.
PreviewTag is correctly imported from the common directory. Ensure any future re-exports do not conflict with the default import structure.
apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/[id].tsx (2)
15-15
: PreviewTag import synergy.
Verifies consistent usage across different analytics views. This unified approach keeps your UI consistent.
67-72
: Flex container for heading and badge.
Good usage of gap-5 ensures readable spacing. The text-base class on PreviewTag ensures a balanced size relative to the H1.
apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/index.tsx (1)
20-20
: Confirm that PreviewTag is properly localized
PreviewTag leverages translations; ensure that any text within it is available in the translation files to prevent rendering fallback strings.
✅ Verification successful
Let me verify the localization of PreviewTag by first checking its implementation and usage of translations.
Let me check the translation files to verify if the PreviewTag's translation key is properly defined.
PreviewTag component is properly localized
The PreviewTag component uses the translation key 'shared.generic.featurePreview' which is correctly defined in both English ('Feature Preview') and German ('Feature-Vorschau') translation files at packages/i18n/messages/
. The component properly uses the useTranslations
hook to access these translations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# First, let's find and examine the PreviewTag component implementation
ast-grep --pattern 'const PreviewTag = $_'
# Also check for any other PreviewTag component definition
ast-grep --pattern 'function PreviewTag($_) {
$$$
}'
# Let's also search for translation usage within files containing PreviewTag
rg -l "PreviewTag" | xargs rg "useTranslation|t\(" -A 2 -B 2
Length of output: 26208
Script:
#!/bin/bash
# Let's check the translation files for the key used in PreviewTag
rg "featurePreview" packages/i18n/messages/
Length of output: 184
apps/frontend-manage/src/components/common/Header.tsx (1)
13-13
: Check usage of future Badge
You are importing Badge from "@uzh-bf/design-system/dist/future." Ensure that this new entry point is stable and documented.
✅ Verification successful
Let me search for usage and documentation of the Badge component from the future entry point.
Based on the search results, I can see that the @uzh-bf/design-system/dist/future
entry point is being consistently used across multiple components in the codebase, including several instances of the Badge component. The import is used in both the frontend-manage application and shared components, indicating it's an established pattern. Let me check the design system repository for more context.
Based on the evidence gathered, I can now provide a final response:
Badge import from future entry point appears to be stable
The /dist/future
entry point is consistently used across multiple components in the codebase, including shared components and frontend applications. The design system is officially documented as a core dependency in the project's README, and the Badge component from this entry point is used in several locations, indicating it's an established pattern rather than an experimental feature.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for other imports from the future entry point
rg "@uzh-bf/design-system/dist/future" -A 2
# Search for Badge usage in the codebase
ast-grep --pattern 'Badge $$$'
# Check if there's any documentation about the future entry point
fd README.md --exec cat {}
Length of output: 15950
Script:
#!/bin/bash
# Check if there's any documentation about the design system
rg "design-system" -A 2 README.md
# Look for any migration or changelog files that might document the future entry point
fd -g "*CHANGELOG*" -g "*MIGRATION*" -g "*UPGRADE*"
Length of output: 523
apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx (4)
176-179
: Good approach to modular design
Injecting StudentActivityPerformance as a self-contained component helps maintain the Single Responsibility Principle.
16-16
: Imported StudentActivityPerformance
Ensure the newly introduced component is tested thoroughly for edge cases (e.g., zero participants, invalid courseId).
✅ Verification successful
Let me verify if there are tests for the StudentActivityPerformance component.
Let me try a different search strategy to locate the component and its tests.
Let me check the actual implementation of the StudentActivityPerformance component to understand how it handles edge cases.
Edge cases are properly handled in the StudentActivityPerformance component
The component already includes proper handling of edge cases:
- Empty performances array: Shows "noStudentActivityPerformanceData" message
- No selected activities: Shows "noActivitySelected" message
- Loading state: Shows a loader
- Missing email: Falls back to "emailMissing" translation
- Invalid courseId: Skips the query with
skip: !courseId
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for test files related to StudentActivityPerformance
fd "StudentActivityPerformance.*test|StudentActivityPerformance.*spec"
# Search for the component implementation to understand its complexity
ast-grep --pattern 'const StudentActivityPerformance = $_'
# Search for test files in the analytics/performance directory
fd "test|spec" "components/analytics/performance"
Length of output: 671
Script:
#!/bin/bash
# Search for the component file
fd "StudentActivityPerformance" "apps/frontend-manage"
# Search for any usage or implementation of StudentActivityPerformance
rg "StudentActivityPerformance" -A 5 "apps/frontend-manage"
# Look for test files in the analytics directory
fd "test|spec" "apps/frontend-manage/src/components/analytics"
Length of output: 3089
Script:
#!/bin/bash
# Get the full content of StudentActivityPerformance component
cat "apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx"
# Look for any error handling or empty state handling patterns in the component
rg "NoData|Error|empty|zero|!performances|performances\." -A 3 "apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx"
Length of output: 8374
9-9
: Imported PreviewTag
Confirm usage consistency with the newly imported PreviewTag for branding and layout synergy across all analytics pages.
✅ Verification successful
Let me gather information about the PreviewTag usage across analytics pages.
Let me check the actual usage of PreviewTag in these files to verify consistency.
Let me check the PreviewTag component's implementation location since it wasn't found in the expected path.
Let me check the actual implementation and usage of PreviewTag component.
PreviewTag usage is consistent across analytics pages
The PreviewTag component is consistently used across all analytics pages with the same styling (className="text-base"
), positioning (next to the H1 title in a flex row with gap-5), and purpose (indicating feature preview status). The implementation and usage maintain branding and layout synergy.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# First, let's find all analytics-related pages
fd ".*\.tsx?$" apps/frontend-manage/src/pages/analytics
# Then, let's search for PreviewTag usage in these files
rg -l "PreviewTag" apps/frontend-manage/src/pages/analytics
# Let's also check the actual implementation of PreviewTag component
ast-grep --pattern 'const PreviewTag = $_'
Length of output: 1295
Script:
#!/bin/bash
# Let's examine how PreviewTag is used in each analytics file
echo "=== performance.tsx ==="
rg -A 5 "PreviewTag" apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx
echo "=== quizzes/index.tsx ==="
rg -A 5 "PreviewTag" apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/index.tsx
echo "=== quizzes/[id].tsx ==="
rg -A 5 "PreviewTag" apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/[id].tsx
echo "=== activity.tsx ==="
rg -A 5 "PreviewTag" apps/frontend-manage/src/pages/analytics/[courseId]/activity.tsx
# Let's also check the actual PreviewTag component implementation
rg -A 10 "PreviewTag" "apps/frontend-manage/src/components/common/PreviewTag"
Length of output: 3471
Script:
#!/bin/bash
# Find the PreviewTag component file
fd "PreviewTag" apps/frontend-manage/src
# Let's also check its implementation
rg -A 10 "export.*PreviewTag" apps/frontend-manage/src
Length of output: 239
Script:
#!/bin/bash
# Check the PreviewTag component implementation
cat apps/frontend-manage/src/components/common/PreviewTag.tsx
# Let's see how it's rendered in each analytics file with more context
echo "=== performance.tsx usage ==="
rg -B 5 -A 5 "<PreviewTag" apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx
echo "=== quizzes/index.tsx usage ==="
rg -B 5 -A 5 "<PreviewTag" apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/index.tsx
echo "=== quizzes/[id].tsx usage ==="
rg -B 5 -A 5 "<PreviewTag" apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/[id].tsx
echo "=== activity.tsx usage ==="
rg -B 5 -A 5 "<PreviewTag" apps/frontend-manage/src/pages/analytics/[courseId]/activity.tsx
Length of output: 3226
168-170
: Check for large data sets
Changing TabContent to use a flex layout with gap properties is nice, but confirm responsiveness under large amounts of data.
✅ Verification successful
Let me analyze the code to verify the concern about large data sets and responsiveness in the TabContent component.
This will help us understand:
- The TabContent component implementation
- How data is being loaded and rendered
- Any existing performance optimizations
Let me try a different search strategy to find the relevant code.
This will help us understand:
- The complete component implementation
- Whether any virtualization is being used
- How data is being managed and rendered in this component
Let me check the content of the components being rendered inside the TabContent to understand the data volume and any existing optimizations.
This will help us understand:
- How the performance plots handle data rendering
- Whether there are any built-in optimizations in the child components
- The structure of the data being fetched
Based on the code analysis, I can now generate the final response. The code shows that:
- The TabContent with flex layout is used in the "studentPerformance" tab which contains two components: TotalStudentPerformancePlot and StudentActivityPerformance
- Both components use DataTable with pagination enabled (
isPaginated
prop) - The data is fetched through GraphQL and the components handle data display with proper loading states
- The StudentActivityPerformance component includes checkboxes for activity selection, preventing all data from being displayed at once
- The TotalStudentPerformancePlot uses ResponsiveContainer for the chart and a paginated table for detailed data
Flex layout is well-supported by data handling mechanisms
The implementation already includes pagination and selective data loading features that will maintain responsiveness even with large datasets. No issues found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Let's first find the TabContent component definition and its usage
ast-grep --pattern 'const TabContent = $_'
# Also search for any performance-related imports or components
rg "TabContent" -A 5 -B 5
# Look for any data fetching or array mapping related to TabContent
rg "map\(" apps/frontend-manage/src/pages/analytics/
Length of output: 582
Script:
#!/bin/bash
# Search for the specific file and its content
cat "apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx"
# Look for any virtualization or performance-related imports
rg "import.*virtualized|import.*windowing" apps/frontend-manage/src/pages/analytics/
# Search for any data loading or state management related to this component
rg "useQuery|useState|useEffect" apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx
Length of output: 7898
Script:
#!/bin/bash
# Check the implementation of components rendered in TabContent
cat "apps/frontend-manage/src/components/analytics/performance/TotalStudentPerformancePlot.tsx"
cat "apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx"
# Look for any pagination or data limiting in the GraphQL query
rg "GetCoursePerformanceAnalyticsDocument" -A 10 -B 10
Length of output: 80104
apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx (5)
25-28
: Skip pattern
Using “skip: !courseId” is a good approach to delay the query until courseId is available. Validate that the rest of the UI gracefully handles the interim empty/no data scenario.
92-95
: Existing loader component
Ensure the loading state is fully tested or that any custom hooks do not override the loader behavior.
126-151
: Checkbox labeling
Labeling each checkbox with the activity name is good for accessibility. Double-check that the “htmlFor” matches the “id.”
63-91
: Check aggregator logic
The aggregator handles user performance data. Confirm that data with partially missing performances (e.g., no completions) is rendered safely without throwing exceptions.
✅ Verification successful
Let me generate shell scripts to verify the handling of missing performance data.
The scripts will help us:
- Find test files that might verify handling of partial/missing data
- Understand the performance data structure through type definitions
- Check for existing null checks or error handling in the component
Let me try different search patterns to analyze the performance data handling.
These scripts will help us:
- Understand the performance data structure
- See the full component implementation for context
- Find existing null checks in the code
Based on the code analysis, I can now provide the final response:
The aggregator logic safely handles missing performance data
The implementation includes multiple safeguards:
- Null checks using optional chaining for course data (
data?.getCourseActivities
) - Fallback for missing email (
studentPerformance.participantEmail ?? t('manage.analytics.emailMissing')
) - Performance filtering based on
selectedActivities
ensures only selected activities are processed - Early return with empty array if loading or no course data (
if (loading || !course) { return [] }
) - UI feedback for edge cases (no selected activities, no performance data)
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for test files related to StudentActivityPerformance component
fd "StudentActivityPerformance.*test" --type f
# Search for performance data type definitions and interfaces
ast-grep --pattern 'interface $_Performance {
$$$
}'
# Search for any error handling or null checks in the component
rg -A 3 "performance\?" apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
Length of output: 220
Script:
#!/bin/bash
# Search for the performance types and interfaces
rg -A 10 "type.*Performance|interface.*Performance" apps/frontend-manage/src/components/analytics/performance/
# Look for the component's full implementation to understand data flow
cat apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
# Search for any null coalescing or optional chaining in the file
rg "\?\." apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
Length of output: 12214
1-14
: Validate improved user experience with new component
This is a new file that provides a dedicated component for displaying performance data. Ensure thorough testing for different data shapes and states (e.g., no data, partial data, large data sets) to confirm a smooth user experience.
✅ Verification successful
Let me gather more information about this component and its usage to verify the data handling scenarios.
Let me gather more information about the component's implementation to understand how it handles different data states.
Let me check one more aspect to see how the component handles the data display and table implementation.
Based on the gathered information, I can now provide a final response about the component's handling of different data states and scenarios.
Component handles data states appropriately
The StudentActivityPerformance component demonstrates robust handling of different data scenarios:
- Loading states are properly managed with loading checks
- Empty data states are handled (performances.length > 0 check)
- Data transformation is optimized using useMemo for activity mappings
- Table data is processed safely with null coalescing operators
- Pagination is implemented for handling large datasets
- Dynamic column generation based on selected activities
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for the implementation of StudentActivityPerformance component
ast-grep --pattern 'const StudentActivityPerformance = $_'
# Look for test files related to this component
fd "StudentActivityPerformance.test" -t f
# Search for usage of this component in other files
rg "StudentActivityPerformance" -l
# Look for GraphQL query implementation to understand data structure
rg "GetCourseActivitiesDocument" -A 5
Length of output: 25308
Script:
#!/bin/bash
# Look for the full implementation of StudentActivityPerformance component
rg -A 50 "const StudentActivityPerformance = " apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
# Search for any error handling or loading states in the component
rg "loading|error|!data|undefined|null" apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
Length of output: 458
Script:
#!/bin/bash
# Look for DataTable usage and implementation in the component
rg -B 5 -A 20 "DataTable" apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
# Check for any memoized data transformations or table configurations
rg -B 5 -A 15 "useMemo" apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
Length of output: 4428
packages/types/src/index.ts (1)
391-396
: Ensure naming consistency and clarity.
The name "ParticipantActivityPerformances" might be more intuitively singular (e.g., "ParticipantActivityPerformanceSet") or might just remain as-is. However, the current naming is consistent with the structure describing multiple activity performances. It's acceptable if the naming matches usage throughout the codebase.
packages/graphql/src/schema/analytics.ts (3)
308-308
: No issues found.
The introduction of the "participantActivityPerformances" array in "CoursePerformanceAnalytics" is consistent with the new type.
240-254
: Validate the newly introduced fields for null-safety.
The fields "totalScore" and "completion" are presumably always available. Ensure that any GraphQL resolvers gracefully handle potential null values and that the Prisma or data layer does not return null for these fields unintentionally.
✅ Verification successful
Let me gather information about the schema definition and resolvers to verify the null-safety concerns.
This will help us understand:
- How these fields are defined in the Prisma schema
- How they are handled in resolvers
- Any existing null checks or error handling for these fields
Let me search for the Prisma schema to understand how these fields are defined at the database level.
Let me try a different approach to find the Prisma schema definition.
Based on the analysis of the codebase, I can now generate the final response:
The fields are correctly defined as non-nullable in the GraphQL schema and type definitions.
The evidence shows that:
- In the types definition (
packages/types/src/index.ts
), both fields are defined as non-nullable:
export type ParticipantActivityPerformance = {
totalScore: number
completion: number
}
- In the GraphQL schema (
packages/graphql/src/schema/analytics.ts
), both fields are exposed without nullable options:
totalScore: t.exposeInt('totalScore'),
completion: t.exposeFloat('completion'),
- The fields are consistently used as non-nullable throughout the codebase, including in services and frontend components, with proper handling of the values.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for Prisma schema definition related to ParticipantActivityPerformance
fd "schema.prisma" --exec cat {}
# Search for resolvers or queries related to ParticipantActivityPerformance
rg -A 5 "ParticipantActivityPerformance" --type ts
# Search for any null checks or error handling related to these fields
rg "totalScore|completion" --type ts -A 3
Length of output: 52754
Script:
#!/bin/bash
# Search for Prisma schema definition for ParticipantActivityPerformance
rg "model ParticipantActivityPerformance" -A 10 --type prisma
Length of output: 94
Script:
#!/bin/bash
# Find and examine the Prisma schema file
fd "schema.prisma" --exec cat {}
# Also search for any resolver implementations that handle these fields
rg -B 5 -A 5 "ParticipantActivityPerformance.*create|update" --type ts
Length of output: 81614
256-272
: Confirm whether "participantUsername" is mandatory.
We see "participantUsername" is a non-null field. Double-check logic for participants that may not have a username yet (e.g., if they only provided an email). A fallback or a safe default might be necessary in some user creation flows.
✅ Verification successful
Let me gather information about the participant username field and its usage in the codebase.
This will help us understand:
- How the participant username is defined in types/interfaces
- Where and how participant username is used
- The participant creation flow to see if username is mandatory during creation
Let me search specifically for participant creation and type definitions to understand if username is mandatory.
This will help us understand:
- How the Participant interface/type is defined
- What fields are required during participant creation
- Any type definitions that specify field requirements
Based on the schema definitions found, I can now generate the final response:
Username is indeed mandatory in the system
The GraphQL schema shows that username
is a required field in multiple contexts:
- In
ParticipantActivityPerformances
type,participantUsername
is marked as non-nullable (String!
) - In all participant-related queries and mutations (like
CreateParticipantAccountMutation
), the username is consistently returned as a non-null field - The field appears as a core identifier across group activities, leaderboards, and participant interactions
This design choice appears intentional as usernames serve as a primary way to identify participants in collaborative features like group activities and leaderboards.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for participant username related type definitions and implementations
ast-grep --pattern 'type $_Participant {
$$$
username: $_
$$$
}'
# Search for participant interface or type definitions
rg "participant.*username" -A 5 -B 5
# Search for participant creation or user flows
rg "createParticipant|newParticipant" -A 5 -B 5
Length of output: 65697
Script:
#!/bin/bash
# Search for participant type definitions
ast-grep --pattern 'interface $_Participant {
$$$
}'
# Search for participant creation mutations
ast-grep --pattern 'mutation $_CreateParticipant {
$$$
}'
# Search for participant model or type definitions
rg "type Participant" -A 10 -B 2
Length of output: 17271
packages/i18n/messages/en.ts (1)
234-235
: New localization keys conform to naming.
The new keys ("featurePreview", "new") are consistent with the existing structure of the localization file.
packages/i18n/messages/de.ts (2)
234-235
: Matching German localization keys for new features.
The newly added "featurePreview" and "new" translations align well with the English counterpart.
1940-1951
: Ensure documentation references are updated if new analytics features are introduced.
These newly added or updated strings (e.g., "noStudentActivityPerformanceData", "studentActivityPerformance", etc.) look consistent with the analytics expansions. Verify that front-end references are in place so the messages appear correctly.
✅ Verification successful
Let me gather information about the analytics-related string references in the frontend code.
Let me try different search patterns to find the frontend usage of these translations.
Based on the search results, I can see that:
- The translation keys are being used in
apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
- The analytics feature is documented in
apps/analytics/README.md
and referenced in various documentation files - The frontend component properly uses these translations through the
t()
function
Let me generate the final response:
The analytics-related translations are properly referenced in the frontend code.
The new translation strings are correctly integrated into the student activity performance analytics component and are documented in the analytics module. No further documentation updates are needed as the feature is already covered in the existing documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for references to these new analytics-related translation keys
rg -l "noStudentActivityPerformanceData|studentActivityPerformance|studentUsername|studentEmail|emailMissing|selectAllActivities|deselectAllActivities|noActivitySelected|participantActivityPerformanceDescription" --type ts --type tsx
Length of output: 263
Script:
#!/bin/bash
# Look for any documentation files that might need updates for analytics features
fd -e md -e mdx . | xargs rg -l "student.*performance|activity.*performance|analytics"
Length of output: 177
Script:
#!/bin/bash
# Check for component files that might be using these translations
ast-grep --pattern 'useTranslation($$$).t($$$"studentActivityPerformance"$$$)'
ast-grep --pattern 'useTranslation($$$).t($$$"noStudentActivityPerformanceData"$$$)'
Length of output: 167
Script:
#!/bin/bash
# Search for TypeScript/JavaScript files containing these strings with more context
rg "noStudentActivityPerformanceData|studentActivityPerformance|studentUsername|studentEmail" -t js -t ts -A 2 -B 2
# Also check for files in potential analytics-related directories
fd . "apps/analytics" "packages/analytics" -t f -e ts -e js -e tsx -e jsx
Length of output: 5641
Script:
#!/bin/bash
# Let's examine the content of the analytics documentation to understand the feature context
cat apps/analytics/README.md
Length of output: 1519
Script:
#!/bin/bash
# Check recent changes in analytics-related files
fd . -t f -e ts -e tsx -e js -e jsx | xargs rg "activity.*performance|student.*performance" -A 2 -B 2
Length of output: 101469
packages/graphql/src/ops.schema.json (1)
4072-4095
: Verify schema changes integration with existing operations.
The schema additions look well-structured. Let's verify their integration with existing queries and mutations.
Also applies to: 18522-18677
✅ Verification successful
Schema changes are properly integrated and used in the codebase
The type ParticipantActivityPerformances
is well integrated across the codebase:
- Defined in the GraphQL schema with proper field types
- Used in the course performance analytics query
- Properly typed in the frontend components
- Has corresponding TypeScript types and schema builder definitions
The schema changes are consistent with their usage in queries and components, particularly in:
GetCoursePerformanceAnalyticsQuery
- Frontend component
StudentActivityPerformance
- Schema builder in
analytics.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for queries and mutations using the new types
# Search for operations using ParticipantActivityPerformances
echo "Checking operations using ParticipantActivityPerformances..."
rg -t graphql "ParticipantActivityPerformances" --type-add 'graphql:*.{graphql,gql}'
# Search for fragments using the new types
echo "Checking fragments using new types..."
ast-grep --pattern 'fragment $_ on ParticipantActivityPerformance { $_$ }' --lang graphql
ast-grep --pattern 'fragment $_ on ParticipantActivityPerformances { $_$ }' --lang graphql
Length of output: 911
Script:
#!/bin/bash
# Let's try a different approach to find GraphQL operations and fragments
# Search for any usage of ParticipantActivityPerformances in all files
echo "Searching for ParticipantActivityPerformances usage..."
rg "ParticipantActivityPerformances" -A 5 -B 5
# Look for query/mutation definitions that might use this type
echo "Searching for queries/mutations that might reference this type..."
rg "query.*\{" -A 10 --type graphql --type typescript --type javascript
rg "mutation.*\{" -A 10 --type graphql --type typescript --type javascript
# Check the schema definition for more context
echo "Checking schema definition..."
rg "type ParticipantActivityPerformances" -A 15 packages/graphql/src/public/schema.graphql
Length of output: 15632
packages/graphql/src/graphql/ops/QGetCoursePerformanceAnalytics.graphql (1)
56-67
: LGTM! Well-structured addition for participant activity performance tracking
The new participantActivityPerformances
field provides a comprehensive structure for tracking individual participant performance, including identification and detailed performance metrics. This aligns perfectly with the PR objective of adding participant activity performance illustration as a table.
packages/graphql/src/public/schema.graphql (2)
1249-1254
: LGTM! Well-defined types for participant activity performance tracking
The new types ParticipantActivityPerformance
and ParticipantActivityPerformances
are well-structured with:
- Clear field definitions with appropriate types
- Required fields properly marked
- Consistent naming conventions
Also applies to: 1256-1261
346-346
: LGTM! Schema updated to include participant activity performances
The CoursePerformanceAnalytics
type has been correctly updated to include the new participantActivityPerformances
field.
packages/graphql/src/public/server.json (1)
124-124
: LGTM! Server operations updated to support participant activity performance tracking
The GetCoursePerformanceAnalytics query operation has been properly updated to include the new participantActivityPerformances field with all required subfields.
apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/frontend-manage/src/components/analytics/performance/useActivityMap.ts (2)
4-10
: Ensure extensibility for additional activity types
Currently, the hook only accounts for "practiceQuizzes" and "microLearnings." If the codebase supports other activity types (e.g., "groupActivities", "liveQuizzes"), consider allowing their data to flow through this hook for consistency.
11-23
: Avoid repeated reduce logic
Both reduce statements are nearly identical. To promote clarity and maintainability, you can merge them or extract a helper function.-const activityNameMap = useMemo( - () => ({ - ...(practiceQuizzes?.reduce<Record<string, string>>((acc, pq) => { - acc[pq.id] = pq.name - return acc - }, {}) ?? {}), - ...(microLearnings?.reduce<Record<string, string>>((acc, ml) => { - acc[ml.id] = ml.name - return acc - }, {}) ?? {}), - }), - [practiceQuizzes, microLearnings] - ) +const activityNameMap = useMemo(() => { + const mapFn = (list?: { id: string; name: string }[] | null) => + list?.reduce<Record<string, string>>((acc, item) => { + acc[item.id] = item.name + return acc + }, {}) ?? {} + return { + ...mapFn(practiceQuizzes), + ...mapFn(microLearnings), + } +}, [practiceQuizzes, microLearnings])apps/frontend-manage/src/components/analytics/performance/useStudentActivityPerformanceTableData.ts (1)
16-43
: Consider memo key for stable data outputs
The memo output depends ondataAvailable
,performances
,t
, andselectedActivities
. Ensure that the combination of these dependencies accurately reflects changes in the underlying data and that the cost of recalculating is justified.apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx (2)
57-57
: Offer assistance for the TODO
If you need help improving CSV export column naming or adding completion percentage to the CSV, let me know, and I can propose an update.
89-114
: Checkbox rendering is correct
This approach effectively renders the checkboxes for practice quizzes and micro-learnings. For future expansions, unify any repeated listing logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/frontend-manage/src/components/analytics/overview/CourseDashboardList.tsx
(2 hunks)apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx
(1 hunks)apps/frontend-manage/src/components/analytics/performance/useActivityMap.ts
(1 hunks)apps/frontend-manage/src/components/analytics/performance/useStudentActivityPerformanceTableData.ts
(1 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/activity.tsx
(2 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx
(3 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/[id].tsx
(2 hunks)apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/index.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/frontend-manage/src/components/analytics/overview/CourseDashboardList.tsx
- apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/[id].tsx
- apps/frontend-manage/src/pages/analytics/[courseId]/quizzes/index.tsx
- apps/frontend-manage/src/pages/analytics/[courseId]/activity.tsx
- apps/frontend-manage/src/pages/analytics/[courseId]/performance.tsx
🔇 Additional comments (11)
apps/frontend-manage/src/components/analytics/performance/useActivityMap.ts (2)
33-34
: Return object is clearly defined
Returning both the name map and ID list keeps the interface simple.
25-31
: Validate potential large data sizes
When using array mapping inside useMemo
, ensure the arrays won't grow too large and cause performance issues during each re-render. If so, consider pagination or virtualization strategies.
apps/frontend-manage/src/components/analytics/performance/useStudentActivityPerformanceTableData.ts (2)
46-46
: Exporting default is consistent
Exporting the hook as default is aligned with typical React patterns, so this is acceptable.
5-13
: Optional: Add type guards or default fallback
If there's any chance the prop values could be undefined or malformed, consider type guards or default fallbacks to avoid errors.
apps/frontend-manage/src/components/analytics/performance/StudentActivityPerformance.tsx (7)
27-30
: Conditional skip of query
Using skip: !courseId
is a valid approach for avoiding unnecessary queries when the courseId is absent. Good practice.
33-40
: Activity toggle is straightforward
The toggling logic is clear and manages the selected activity IDs effectively.
42-45
: Include other activity types if needed
This hook only references "practiceQuizzes" and "microLearnings." If group activities or live quizzes are expected, they should also be included.
47-51
: Clean integration with table data
The hook usage for table data is clear and properly scoped to the selected activities.
133-147
: Concise cell formatting
Combining total score and completion percentage in a single cell is user-friendly.
163-172
: User notifications for missing data
Providing clear messages when no activities are selected or no performance data is found is a great UX improvement.
177-177
: Default export is consistent
Exporting the component as the default helps keep import paths concise.
klicker-uzh Run #3834
Run Properties:
|
Project |
klicker-uzh
|
Branch Review |
participant-activity-performance-frontend
|
Run status |
Passed #3834
|
Run duration | 11m 57s |
Commit |
e9b75e911c ℹ️: Merge bc0ec8d10745e52232e8da5e9d8abc884598e7cf into 645930c995c1ef8a7d152d38a80a...
|
Committer | Julius Schlapbach |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
148
|
View all changes introduced in this branch ↗︎ |
Quality Gate failedFailed conditions |
klicker-uzh Run #3835
Run Properties:
|
Project |
klicker-uzh
|
Branch Review |
v3-analytics
|
Run status |
Passed #3835
|
Run duration | 11m 56s |
Commit |
f8c8cb1e99: enhance(apps/analytics): add illustration of participant activity performance as...
|
Committer | Julius Schlapbach |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
148
|
View all changes introduced in this branch ↗︎ |
Summary by CodeRabbit
New Features
PreviewTag
andStudentActivityPerformance
.Bug Fixes
Documentation
Chores
@uzh-bf/design-system
dependency across multiple projects.