feat: dashboard for research experiments#2278
Conversation
jo-elimu
requested review from
SnehaHS65,
Souvik-Cyclic and
vrudas
and removed request for
a team
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2278 +/- ##
============================================
- Coverage 16.68% 16.33% -0.36%
- Complexity 448 450 +2
============================================
Files 252 254 +2
Lines 7372 7543 +171
Branches 845 857 +12
============================================
+ Hits 1230 1232 +2
- Misses 6077 6246 +169
Partials 65 65 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Caution Review failedThe pull request is closed. """ WalkthroughThis change introduces research experiment analytics functionality. It adds new DAO methods and JPA implementations to filter learning and assessment events by research experiment and experiment group. New controllers and JSP pages provide listing and detailed comparison of research experiments. The database population and test code are updated to use the new enum values and fields. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ExperimentListController
participant Model
participant View
User->>ExperimentListController: GET /analytics/research/experiments
ExperimentListController->>Model: Add researchExperiments
ExperimentListController->>View: Return "analytics/research/experiments/list"
sequenceDiagram
participant User
participant ExperimentController
participant DAOs
participant Model
participant View
User->>ExperimentController: GET /analytics/research/experiments/{researchExperiment}
ExperimentController->>DAOs: readAll(researchExperiment, CONTROL/TREATMENT)
DAOs-->>ExperimentController: Event lists by group
ExperimentController->>Model: Add event lists to model
ExperimentController->>View: Return "analytics/research/experiments/id"
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 3
🔭 Outside diff range comments (2)
src/main/webapp/WEB-INF/jsp/analytics/research/experiments/list.jsp (1)
1-32: Enhance security and add missing taglib declarations.The JSP structure is well-organized, but consider these improvements:
- Missing taglib declarations: Add the required taglib imports at the top of the file
- XSS prevention: Use
<c:out>tags to escape output values- Null safety: Add checks for empty experiment lists
Add these improvements to the JSP:
+<%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %> +<%@ taglib prefix="fn" uri="http://java.sun.com/jsp/jstl/functions" %> +<%@ taglib prefix="spring" uri="http://www.springframework.org/tags" %> +<%@ taglib prefix="content" tagdir="/WEB-INF/tags" %> + content:title> Research Experiments (${fn:length(researchExperiments)}) </content:title> <content:section cssId="experimentListPage"> <div class="row"> <h5><content:gettitle /></h5> </div> + <c:choose> + <c:when test="${not empty researchExperiments}"> <div class="row"> <table class="bordered highlight"> <thead> <th>Experiment ID</th> <th>Theory</th> </thead> <tbody> <c:forEach var="researchExperiment" items="${researchExperiments}"> <tr class="experiment"> <td> <a class="experimentLink" href="<spring:url value='/analytics/research/experiments/${researchExperiment}' />"> - <code>${researchExperiment}</code> + <code><c:out value="${researchExperiment}" /></code> </a> </td> <td> - <blockquote>${researchExperiment.theory}</blockquote> + <blockquote><c:out value="${researchExperiment.theory}" /></blockquote> </td> </tr> </c:forEach> </tbody> </table> </div> + </c:when> + <c:otherwise> + <div class="row"> + <p>No research experiments found.</p> + </div> + </c:otherwise> + </c:choose> </content:section>src/main/java/ai/elimu/dao/jpa/NumberLearningEventDaoJpa.java (1)
26-26: Fix critical typo in parameter name.There's a typo in the parameter name that will cause a runtime exception when this method is called.
- .setParameter("androidIdevent", androidId) + .setParameter("androidId", androidId)
🧹 Nitpick comments (2)
src/main/webapp/WEB-INF/jsp/analytics/research/experiments/id.jsp (1)
30-30: Fix: Inconsistent HTML structure between CONTROL and TREATMENT progress bars.CONTROL groups use
<div class="progress">while TREATMENT groups use<span class="progress">. This inconsistency may cause styling issues.-<span class="progress"> +<div class="progress">Also applies to: 45-45, 61-61, 76-76, 92-92, 107-107, 123-123, 139-139
src/main/java/ai/elimu/web/analytics/research/experiments/ExperimentController.java (1)
47-69: Suggest: Refactor repetitive DAO calls into a helper method.The controller has repetitive patterns for fetching CONTROL and TREATMENT data. Consider creating a helper method to reduce code duplication.
Example refactor:
private <T> void addExperimentGroupData(Model model, String eventType, Function<ExperimentGroup, List<T>> daoCall) { model.addAttribute(eventType + "_CONTROL", daoCall.apply(ExperimentGroup.CONTROL)); model.addAttribute(eventType + "_TREATMENT", daoCall.apply(ExperimentGroup.TREATMENT)); } // Usage: addExperimentGroupData(model, "letterSoundAssessmentEvents", group -> letterSoundAssessmentEventDao.readAll(researchExperiment, group));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pom.xmlis excluded by!**/*.xml
📒 Files selected for processing (23)
pom-dependency-tree.txt(1 hunks)src/main/java/ai/elimu/dao/LetterSoundAssessmentEventDao.java(1 hunks)src/main/java/ai/elimu/dao/LetterSoundLearningEventDao.java(1 hunks)src/main/java/ai/elimu/dao/NumberLearningEventDao.java(1 hunks)src/main/java/ai/elimu/dao/StoryBookLearningEventDao.java(2 hunks)src/main/java/ai/elimu/dao/VideoLearningEventDao.java(2 hunks)src/main/java/ai/elimu/dao/WordAssessmentEventDao.java(1 hunks)src/main/java/ai/elimu/dao/WordLearningEventDao.java(1 hunks)src/main/java/ai/elimu/dao/jpa/LetterSoundAssessmentEventDaoJpa.java(2 hunks)src/main/java/ai/elimu/dao/jpa/LetterSoundLearningEventDaoJpa.java(2 hunks)src/main/java/ai/elimu/dao/jpa/NumberLearningEventDaoJpa.java(2 hunks)src/main/java/ai/elimu/dao/jpa/StoryBookLearningEventDaoJpa.java(2 hunks)src/main/java/ai/elimu/dao/jpa/VideoLearningEventDaoJpa.java(2 hunks)src/main/java/ai/elimu/dao/jpa/WordAssessmentEventDaoJpa.java(2 hunks)src/main/java/ai/elimu/dao/jpa/WordLearningEventDaoJpa.java(2 hunks)src/main/java/ai/elimu/web/analytics/MainAnalyticsController.java(2 hunks)src/main/java/ai/elimu/web/analytics/research/experiments/ExperimentController.java(1 hunks)src/main/java/ai/elimu/web/analytics/research/experiments/ExperimentListController.java(1 hunks)src/main/java/ai/elimu/web/servlet/CustomDispatcherServlet.java(7 hunks)src/main/webapp/WEB-INF/jsp/analytics/main.jsp(1 hunks)src/main/webapp/WEB-INF/jsp/analytics/research/experiments/id.jsp(1 hunks)src/main/webapp/WEB-INF/jsp/analytics/research/experiments/list.jsp(1 hunks)src/test/java/ai/elimu/util/csv/CsvAnalyticsExtractionHelperTest.java(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: test_rest
- GitHub Check: test_rest
- GitHub Check: test_rest
- GitHub Check: test_rest
- GitHub Check: build (windows-latest, 17)
- GitHub Check: build (macos-latest, 21)
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (ubuntu-latest, 21)
- GitHub Check: test_rest
- GitHub Check: build (macos-latest, 17)
- GitHub Check: build (ubuntu-latest, 17)
- GitHub Check: test_rest
🔇 Additional comments (36)
pom-dependency-tree.txt (1)
1-2: Version bumps look consistent.The webapp and model version increments align appropriately with the new research experiment features being added.
src/main/java/ai/elimu/web/analytics/MainAnalyticsController.java (2)
11-11: Import statement correctly added.The ResearchExperiment enum import is properly placed and needed for the new functionality.
44-44: Model attribute implementation follows established patterns.The research experiment count is correctly calculated using the standard enum values() method and follows the naming convention of existing attributes like
studentCount.src/main/java/ai/elimu/dao/WordAssessmentEventDao.java (2)
9-10: Enum imports correctly added.The import statements for the research experiment enums are properly added to support the new filtering functionality.
18-18: Method signature follows DAO patterns.The new readAll method with ResearchExperiment and ExperimentGroup parameters follows established DAO conventions with appropriate return type and exception handling.
src/main/java/ai/elimu/dao/NumberLearningEventDao.java (2)
9-10: Consistent enum imports across DAO interfaces.The import statements match the pattern established in other DAO interfaces, maintaining consistency across the codebase.
18-18: Method signature maintains consistency with other DAOs.The readAll method follows the same pattern established in WordAssessmentEventDao, ensuring consistent interface design across different event types.
src/main/webapp/WEB-INF/jsp/analytics/main.jsp (1)
19-28: Research experiments card follows established UI patterns.The new card structure maintains consistency with the existing Students card design, uses appropriate Material Icons, and correctly references the model attribute from the controller.
src/main/java/ai/elimu/dao/LetterSoundAssessmentEventDao.java (2)
9-10: LGTM: Clean import additions for new enum types.The import statements are properly organized and support the new filtering functionality.
18-18: LGTM: Well-designed method signature for experiment filtering.The new method signature follows consistent DAO patterns and provides the filtering capability needed for research experiment analytics.
src/test/java/ai/elimu/util/csv/CsvAnalyticsExtractionHelperTest.java (1)
65-65: Verify enum refactoring consistency across the codebase.The systematic change from
ResearchExperiment.WORD_EMOJIStoResearchExperiment.EXP_0_WORD_EMOJISis consistent across all test methods, which is good. However, ensure this enum refactoring is applied consistently throughout the entire codebase to prevent runtime exceptions.#!/bin/bash # Description: Verify that the old enum value WORD_EMOJIS is not used anywhere else in the codebase # Expected: No occurrences of the old enum value should be found echo "Searching for any remaining usage of the old enum value..." rg "WORD_EMOJIS" --type java echo "Searching for the new enum value to confirm it exists..." rg "EXP_0_WORD_EMOJIS" --type java -A 2 -B 2Also applies to: 140-140, 187-187, 278-278, 326-326
src/main/java/ai/elimu/dao/VideoLearningEventDao.java (2)
5-6: LGTM: Consistent import organization.The new enum imports are properly placed and support the extended functionality.
21-21: LGTM: Method signature maintains DAO consistency.The new
readAllmethod follows the same pattern established in other DAO interfaces, ensuring consistency across the data access layer.src/main/java/ai/elimu/dao/WordLearningEventDao.java (2)
9-10: LGTM: Proper import organization.The enum imports are correctly placed and maintain the file's organization.
18-18: LGTM: Consistent method signature.The new filtering method maintains the established pattern across all DAO interfaces in this PR.
src/main/java/ai/elimu/dao/LetterSoundLearningEventDao.java (1)
9-11: LGTM: Clean interface extension for research experiment filtering.The new imports and method signature are well-structured and follow consistent DAO patterns. The method signature properly declares the expected parameters and exception handling.
src/main/java/ai/elimu/dao/jpa/NumberLearningEventDaoJpa.java (2)
5-6: LGTM: Appropriate imports for new functionality.The imports are correctly added to support the new research experiment filtering capability.
46-58: LGTM: Well-implemented research experiment filtering method.The new
readAllmethod implementation is correctly structured with:
- Proper JPQL query syntax
- Parameterized queries to prevent SQL injection
- Consistent parameter binding
- Appropriate ordering by timestamp
src/main/java/ai/elimu/dao/jpa/StoryBookLearningEventDaoJpa.java (2)
5-6: LGTM: Consistent imports for research experiment functionality.The imports are appropriately added to support the new filtering capabilities.
54-66: LGTM: Consistent and secure implementation.The new
readAllmethod implementation follows the established pattern with:
- Proper JPQL syntax and parameterization
- Correct parameter binding for security
- Consistent ordering and return type
- Appropriate exception handling
src/main/java/ai/elimu/dao/jpa/WordLearningEventDaoJpa.java (2)
5-6: LGTM: Proper imports for enhanced functionality.The imports correctly support the new research experiment filtering feature.
46-58: LGTM: Secure and consistent query implementation.The
readAllmethod implementation properly follows established patterns with:
- Parameterized JPQL query for security
- Correct parameter binding
- Consistent timestamp ordering
- Appropriate exception handling
src/main/java/ai/elimu/dao/jpa/LetterSoundLearningEventDaoJpa.java (2)
10-11: LGTM: Appropriate enum imports added.The imports correctly support the new research experiment filtering functionality.
46-58: LGTM: Well-implemented filtering method.The new
readAllmethod implementation is correctly structured with:
- Secure parameterized JPQL query
- Proper parameter binding
- Consistent ordering by timestamp
- Matching interface signature and exception handling
src/main/java/ai/elimu/dao/jpa/VideoLearningEventDaoJpa.java (2)
6-7: LGTM: Clean import additions.The imports for the research experiment enums are properly added.
56-68: LGTM: Well-implemented filtering method.The new
readAllmethod follows the established patterns in the class with proper parameter binding and consistent ordering. The JPQL query structure is clean and efficient.src/main/java/ai/elimu/web/servlet/CustomDispatcherServlet.java (1)
45-46: LGTM: Clean import additions.The imports for the research experiment enums are properly added.
src/main/java/ai/elimu/dao/jpa/LetterSoundAssessmentEventDaoJpa.java (2)
10-11: LGTM: Consistent import pattern.The imports follow the same pattern established in other DAO files.
46-58: LGTM: Consistent DAO implementation.The implementation follows the established pattern with proper JPQL query structure, named parameters, and consistent ordering. Good consistency across DAO implementations.
src/main/java/ai/elimu/dao/jpa/WordAssessmentEventDaoJpa.java (2)
10-11: LGTM: Consistent import additions.The imports are consistent with the pattern used across other DAO implementations.
46-58: LGTM: Well-structured and consistent implementation.The method implementation maintains excellent consistency with other DAO classes, using the same JPQL query pattern and parameter binding approach. This consistency aids maintainability.
src/main/java/ai/elimu/dao/StoryBookLearningEventDao.java (2)
8-9: LGTM: Proper interface imports.The enum imports are correctly added to support the new method signature.
19-19: LGTM: Clean interface method declaration.The method signature is well-defined with appropriate parameter types and exception declaration. This aligns perfectly with the implementations seen in the corresponding JPA classes.
src/main/java/ai/elimu/web/analytics/research/experiments/ExperimentListController.java (1)
11-25: LGTM! Clean and simple controller implementation.The controller follows Spring MVC best practices with appropriate annotations and dependency injection patterns. The logic is straightforward and correct for a list controller.
src/main/java/ai/elimu/web/analytics/research/experiments/ExperimentController.java (2)
20-38: LGTM! Well-structured controller with proper dependency injection.The controller follows Spring MVC best practices with appropriate annotations, constructor injection, and clear separation of concerns.
32-32: ```shell
#!/bin/bashLocate ExperimentController.java
EXP_PATH=$(fd ExperimentController.java --type f)
echo "ExperimentController.java file path: $EXP_PATH"echo "----- Lines with NumberAssessmentEventDao -----"
grep -n "NumberAssessmentEventDao" "$EXP_PATH" || echo "No occurrences in the controller."echo "----- model.addAttribute calls in ExperimentController -----"
grep -n "model.addAttribute" "$EXP_PATH"echo "----- JSP files containing 'Number assessment events' -----"
rg -n "Number assessment events"</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
1 participant
Issue Number
Purpose
Technical Details
Testing Instructions
Screenshots
Format Checks
Note
Files in PRs are automatically checked for format violations with
mvn spotless:check.If this PR contains files with format violations, run
mvn spotless:applyto fix them.