feat: add student entity #2186
Merged
feat: add student entity #2186
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jo-elimu
requested review from
alexander-kuruvilla,
eymaal and
vrudas
and removed request for
a team
4 tasks
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2186 +/- ##
============================================
- Coverage 15.06% 14.62% -0.45%
- Complexity 391 397 +6
============================================
Files 232 238 +6
Lines 6258 6491 +233
Branches 717 753 +36
============================================
+ Hits 943 949 +6
- Misses 5264 5491 +227
Partials 51 51 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
|
Caution Review failedThe pull request is closed. ## Walkthrough
This change introduces a new `Student` entity, associated DAO interfaces and implementations, and a controller/view for listing students in the analytics dashboard. The legacy `Device` entity and related DAOs are marked as deprecated. The analytics layout JSP and related CSS are removed, and a new JSP is added for the student list. Several REST controllers handling CSV uploads were modified to write files using `FileUtils.writeByteArrayToFile` and log file existence. New scheduled importers for word, storybook, and video learning events create `Student` entities if missing.
## Changes
| File(s) | Change Summary |
|---------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `src/main/java/ai/elimu/entity/analytics/students/Student.java`,<br>`src/main/resources/META-INF/jpa-schema-export.sql` | Added new `Student` entity class and corresponding table schema, with a unique `androidId` field. |
| `src/main/java/ai/elimu/dao/StudentDao.java`,<br>`src/main/java/ai/elimu/dao/jpa/StudentDaoJpa.java` | Introduced new DAO interface and JPA implementation for `Student` entity, including a method to read by `androidId`. |
| `src/main/java/ai/elimu/web/analytics/students/StudentsListController.java`,<br>`src/main/webapp/WEB-INF/jsp/analytics/students/list.jsp` | Added Spring MVC controller and JSP view for listing students in the analytics dashboard, displaying redacted Android IDs. |
| `src/main/java/ai/elimu/web/analytics/students/StudentController.java`,<br>`src/main/webapp/WEB-INF/jsp/analytics/students/id.jsp` | Added Spring MVC controller and JSP view for detailed analytics of individual students, showing monthly event counts with Chart.js visualizations. |
| `src/main/java/ai/elimu/web/servlet/CustomDispatcherServlet.java` | Modified database population logic to create and persist three sample `Student` entities. |
| `src/main/java/ai/elimu/entity/Device.java`,<br>`src/main/java/ai/elimu/dao/DeviceDao.java`,<br>`src/main/java/ai/elimu/dao/jpa/DeviceDaoJpa.java` | Marked legacy `Device` entity and DAO classes as deprecated. |
| `src/main/webapp/WEB-INF/jsp/analytics/layout.jsp`,<br>`src/main/webapp/static/css/analytics/styles.css` | Deleted analytics layout JSP and associated CSS file. |
| `src/main/webapp/WEB-INF/jsp/layout.jsp` | Removed conditional inclusion of the deleted analytics layout JSP. |
| `pom-dependency-tree.txt` | Updated project artifact version from `2.6.10-SNAPSHOT` to `2.6.11-SNAPSHOT`. |
| `src/main/java/ai/elimu/rest/v2/analytics/LetterSoundLearningEventsRestController.java`,<br>`src/main/java/ai/elimu/rest/v2/analytics/StoryBookLearningEventsRestController.java`,<br>`src/main/java/ai/elimu/rest/v2/analytics/VideoLearningEventsRestController.java`,<br>`src/main/java/ai/elimu/rest/v2/analytics/WordLearningEventsRestController.java`,<br>`src/main/java/ai/elimu/rest/v2/analytics/LetterAssessmentEventsRestController.java`,<br>`src/main/java/ai/elimu/rest/v2/analytics/WordAssessmentEventsRestController.java` | Changed CSV file saving method from `multipartFile.transferTo()` to `FileUtils.writeByteArrayToFile()` and added log statements to confirm file existence after writing. No other logic changes. |
| `src/main/java/ai/elimu/tasks/analytics/WordLearningEventImportScheduler.java`,<br>`src/main/java/ai/elimu/tasks/analytics/StoryBookLearningEventImportScheduler.java`,<br>`src/main/java/ai/elimu/tasks/analytics/VideoLearningEventImportScheduler.java` | Added new scheduled importers for word, storybook, and video learning events that read CSVs, check for duplicates, create missing `Student` entities, and persist events. Modified video event importer to create `Student` if missing and changed its schedule time. |
| `src/main/java/ai/elimu/util/csv/CsvAnalyticsExtractionHelper.java` | Refactored CSV extraction helper: added method to extract word learning events, renamed and simplified storybook event extraction, removed deprecated DAO dependencies, and improved timestamp handling with UTC timezone. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant StudentsListController
participant StudentDao
participant WordLearningEventDao
participant StoryBookLearningEventDao
participant DB
participant View
User->>StudentsListController: GET /analytics/students
StudentsListController->>WordLearningEventDao: readAll()
WordLearningEventDao->>DB: Query all WordLearningEvents
DB-->>WordLearningEventDao: List<WordLearningEvent>
StudentsListController->>StudentDao: read(androidId) for each WordLearningEvent
StudentDao->>DB: Query Student by androidId
DB-->>StudentDao: Student or null
alt Student not found
StudentsListController->>StudentDao: create new Student with androidId
StudentDao->>DB: Insert Student
end
StudentsListController->>StoryBookLearningEventDao: readAll()
StoryBookLearningEventDao->>DB: Query all StoryBookLearningEvents
DB-->>StoryBookLearningEventDao: List<StoryBookLearningEvent>
StudentsListController->>StudentDao: read(androidId) for each StoryBookLearningEvent
StudentDao->>DB: Query Student by androidId
DB-->>StudentDao: Student or null
alt Student not found
StudentsListController->>StudentDao: create new Student with androidId
StudentDao->>DB: Insert Student
end
StudentDao->>DB: readAll()
DB-->>StudentDao: List<Student>
StudentsListController->>View: Render list.jsp with redacted students
View-->>User: HTML page with student listAssessment against linked issues
Possibly related PRs
Suggested reviewers
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
src/main/java/ai/elimu/dao/DeviceDao.java (1)
7-7: Add JavaDoc @deprecated tag: The@Deprecatedannotation is applied correctly, but consider updating the interface Javadoc with a@deprecatedtag that referencesStudentDaoso consumers know the preferred replacement.src/main/java/ai/elimu/entity/Device.java (1)
15-15: Document entity deprecation: You’ve marked theDeviceentity as deprecated; consider adding a Javadoc@deprecatedcomment to indicate migration toStudentand any relevant data migration steps.src/main/java/ai/elimu/dao/jpa/DeviceDaoJpa.java (1)
9-9: Enhance deprecation annotation: Consider using@Deprecated(forRemoval = true)and adding a Javadoc@deprecatednote referencingStudentDaoJpato guide users toward the new implementation and signal eventual removal.src/main/java/ai/elimu/entity/analytics/students/Student.java (2)
10-13: Explicit table mapping recommended: TheStudententity relies on default table naming. For clarity and to avoid naming collisions, consider adding@Table(name = "students"). A brief Javadoc summary of the entity’s purpose would also aid future maintainers.
21-23: Specify column constraints for androidId: You enforce uniqueness and non-nullability onandroidId, but consider adding alengthattribute to@Columnto match the expected hexadecimal string size and ensure a database index exists for faster lookups.src/main/java/ai/elimu/dao/StudentDao.java (1)
1-9: Well-structured DAO interfaceThe StudentDao interface properly extends GenericDao and includes a method to retrieve a Student by androidId. This follows the same pattern used by other DAO interfaces in the system.
Consider using Java 8's Optional for the return type to better represent the possible absence of a value:
-Student read(String androidId) throws DataAccessException; +Optional<Student> read(String androidId) throws DataAccessException;src/main/java/ai/elimu/dao/jpa/StudentDaoJpa.java (1)
1-23: Clean implementation of StudentDaoThe implementation correctly extends GenericDaoJpa and provides a proper JPQL query to find a Student by androidId. The error handling for "no result found" is implemented correctly.
Two suggestions for improvement:
- Use a TypedQuery to avoid casting:
-return (Student) em.createQuery( +return em.createQuery( "SELECT s " + "FROM Student s " + "WHERE s.androidId = :androidId") .setParameter("androidId", androidId) + .getSingleResult();
- Consider returning Optional instead of null for better null safety:
-public Student read(String androidId) throws DataAccessException { +public Optional<Student> read(String androidId) throws DataAccessException { try { - return (Student) em.createQuery( + Student result = em.createQuery( "SELECT s " + "FROM Student s " + "WHERE s.androidId = :androidId", Student.class) .setParameter("androidId", androidId) .getSingleResult(); + return Optional.of(result); } catch (NoResultException e) { - return null; + return Optional.empty(); } }src/main/resources/META-INF/jpa-schema-export.sql (1)
484-488: Student table creationThe Student table is created with an id primary key and androidId column. Consider adding a NOT NULL constraint to the androidId column if this is a required field:
create table Student ( id bigint not null auto_increment, - androidId varchar(255), + androidId varchar(255) not null, primary key (id) ) type=MyISAM;Also, consider adding metadata columns such as creation timestamp if you need to track when student records were created.
src/main/java/ai/elimu/web/analytics/students/StudentsListController.java (1)
24-35: Mark controller method as read-only transactional
AnnotatehandleRequestwith@Transactional(readOnly = true)to optimize data access and document intent.- @GetMapping + @GetMapping + @Transactional(readOnly = true)Don’t forget to import
org.springframework.transaction.annotation.Transactional.src/main/webapp/WEB-INF/jsp/analytics/students/list.jsp (1)
5-22: Replace<br />hacks with CSS spacing
A sequence of<br />for vertical gaps is brittle. Instead, define a CSS utility class:/* in a shared stylesheet */ .banner-spacer { padding-top: 4rem; /* adjust as needed */ }Then simplify JSP:
<content:banner cssClass="banner-spacer" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/main/webapp/WEB-INF/spring/applicationContext-jpa.xmlis excluded by!**/*.xml
📒 Files selected for processing (14)
pom-dependency-tree.txt(1 hunks)src/main/java/ai/elimu/dao/DeviceDao.java(1 hunks)src/main/java/ai/elimu/dao/StudentDao.java(1 hunks)src/main/java/ai/elimu/dao/jpa/DeviceDaoJpa.java(1 hunks)src/main/java/ai/elimu/dao/jpa/StudentDaoJpa.java(1 hunks)src/main/java/ai/elimu/entity/Device.java(1 hunks)src/main/java/ai/elimu/entity/analytics/students/Student.java(1 hunks)src/main/java/ai/elimu/web/analytics/students/StudentsListController.java(1 hunks)src/main/java/ai/elimu/web/servlet/CustomDispatcherServlet.java(2 hunks)src/main/resources/META-INF/jpa-schema-export.sql(3 hunks)src/main/webapp/WEB-INF/jsp/analytics/layout.jsp(0 hunks)src/main/webapp/WEB-INF/jsp/analytics/students/list.jsp(1 hunks)src/main/webapp/WEB-INF/jsp/layout.jsp(0 hunks)src/main/webapp/static/css/analytics/styles.css(0 hunks)
💤 Files with no reviewable changes (3)
- src/main/webapp/static/css/analytics/styles.css
- src/main/webapp/WEB-INF/jsp/layout.jsp
- src/main/webapp/WEB-INF/jsp/analytics/layout.jsp
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/main/java/ai/elimu/entity/Device.java (1)
src/main/java/ai/elimu/dao/jpa/DeviceDaoJpa.java (1)
Deprecated(9-25)
src/main/java/ai/elimu/dao/DeviceDao.java (1)
src/main/java/ai/elimu/dao/jpa/DeviceDaoJpa.java (1)
Deprecated(9-25)
src/main/java/ai/elimu/web/analytics/students/StudentsListController.java (2)
src/main/java/ai/elimu/util/AnalyticsHelper.java (1)
AnalyticsHelper(6-43)src/main/java/ai/elimu/web/servlet/CustomDispatcherServlet.java (1)
Slf4j(67-315)
src/main/java/ai/elimu/entity/analytics/students/Student.java (1)
src/main/java/ai/elimu/entity/Device.java (1)
Entity(12-40)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: test_ui
- GitHub Check: build (windows-latest, 17)
- GitHub Check: test_rest
- GitHub Check: test_rest
- GitHub Check: build (windows-latest, 21)
- GitHub Check: build (ubuntu-latest, 17)
- GitHub Check: build (macos-latest, 17)
- GitHub Check: build (macos-latest, 21)
- GitHub Check: build (ubuntu-latest, 21)
- GitHub Check: test_rest
🔇 Additional comments (6)
pom-dependency-tree.txt (1)
1-1: Project version updated: bumping artifact version to 2.6.11-SNAPSHOT appropriately reflects the newStudententity functionality.src/main/java/ai/elimu/web/servlet/CustomDispatcherServlet.java (2)
13-16: New entity imports added for Student functionalityThe import statements for
StudentDaoandStudententity have been correctly added to support the new student entity functionality.
299-313: Properly implemented database population for Student entitiesThe implementation creates three Student entities with unique Android IDs, following the same pattern as other entities in this method. This ensures that the test data is properly populated for the new Student entity.
src/main/resources/META-INF/jpa-schema-export.sql (2)
78-78: Student table drop statement addedThe drop statement for the Student table has been correctly added to the schema.
635-636: Unique constraint for androidIdThe unique constraint on the androidId column has been properly added, which ensures each Student has a unique identifier.
src/main/java/ai/elimu/web/analytics/students/StudentsListController.java (1)
16-23: Constructor injection with Lombok is clean
Using@RequiredArgsConstructorfor the finalstudentDaofield promotes immutability and testability.
src/main/java/ai/elimu/web/analytics/students/StudentsListController.java
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
src/main/java/ai/elimu/web/analytics/students/StudentsListController.java (2)
36-56: Consider refactoring duplicate student creation logic.The student creation code is duplicated for both word learning events and storybook learning events. Consider extracting this logic into a helper method to improve maintainability.
@GetMapping public String handleRequest(Model model) { log.info("handleRequest"); // Generate Student IDs for pre-existing learning events - for (WordLearningEvent wordLearningEvent : wordLearningEventDao.readAll()) { - log.info("wordLearningEvent.getAndroidId(): " + wordLearningEvent.getAndroidId()); - Student existingStudent = studentDao.read(wordLearningEvent.getAndroidId()); - if (existingStudent == null) { - Student student = new Student(); - student.setAndroidId(wordLearningEvent.getAndroidId()); - studentDao.create(student); - log.info("Stored Student in database with ID " + student.getId()); - } - } - for (StoryBookLearningEvent storyBookLearningEvent : storyBookLearningEventDao.readAll()) { - log.info("storyBookLearningEvent.getAndroidId(): " + storyBookLearningEvent.getAndroidId()); - Student existingStudent = studentDao.read(storyBookLearningEvent.getAndroidId()); - if (existingStudent == null) { - Student student = new Student(); - student.setAndroidId(storyBookLearningEvent.getAndroidId()); - studentDao.create(student); - log.info("Stored Student in database with ID " + student.getId()); - } - } + createStudentsFromWordLearningEvents(); + createStudentsFromStoryBookLearningEvents(); List<Student> students = studentDao.readAll(); for (Student student : students) { student.setAndroidId(AnalyticsHelper.redactAndroidId(student.getAndroidId())); } model.addAttribute("students", students); return "analytics/students/list"; } + private void createStudentsFromWordLearningEvents() { + for (WordLearningEvent wordLearningEvent : wordLearningEventDao.readAll()) { + createStudentIfMissing(wordLearningEvent.getAndroidId()); + } + } + + private void createStudentsFromStoryBookLearningEvents() { + for (StoryBookLearningEvent storyBookLearningEvent : storyBookLearningEventDao.readAll()) { + createStudentIfMissing(storyBookLearningEvent.getAndroidId()); + } + } + + private void createStudentIfMissing(String androidId) { + log.info("Checking for student with androidId: " + androidId); + Student existingStudent = studentDao.read(androidId); + if (existingStudent == null) { + Student student = new Student(); + student.setAndroidId(androidId); + studentDao.create(student); + log.info("Stored Student in database with ID " + student.getId()); + } + }
32-65: Consider separating data initialization from display logic.The current implementation mixes two concerns: initializing missing students and displaying student data. Consider separating these concerns by moving the student creation logic to a service class or a separate initialization method that could be called during application startup.
This would allow the controller to focus solely on its primary responsibility - preparing and displaying data.
src/main/java/ai/elimu/util/csv/CsvAnalyticsExtractionHelper.java (1)
30-92: Consider refactoring to reduce code duplicationThe three extraction methods share a lot of similar code for file handling, CSV parsing, and error handling. Consider refactoring to extract common functionality into private helper methods to improve maintainability and reduce duplication.
For example, you could create a method like:
private static <T> List<T> extractEventsFromCsv( File csvFile, String methodName, String[] headers, Function<CSVRecord, T> recordProcessor) { log.info(methodName); List<T> events = new ArrayList<>(); Path csvFilePath = Paths.get(csvFile.toURI()); log.info("csvFilePath: " + csvFilePath); try (Reader reader = Files.newBufferedReader(csvFilePath); CSVParser csvParser = new CSVParser(reader, CSVFormat.DEFAULT .withHeader(headers) .withSkipHeaderRecord())) { log.info("header: " + Arrays.toString(csvParser.getHeaderMap().keySet().toArray())); for (CSVRecord csvRecord : csvParser) { log.info("csvRecord: " + csvRecord); T event = recordProcessor.apply(csvRecord); events.add(event); } } catch (IOException ex) { log.error("Failed to process CSV file: " + csvFilePath, ex); } return events; }Then each specific extraction method could call this helper with the appropriate record processor.
Also applies to: 94-156, 158-220
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/ai/elimu/tasks/analytics/StoryBookLearningEventImportScheduler.java(1 hunks)src/main/java/ai/elimu/tasks/analytics/VideoLearningEventImportScheduler.java(3 hunks)src/main/java/ai/elimu/tasks/analytics/WordLearningEventImportScheduler.java(1 hunks)src/main/java/ai/elimu/util/csv/CsvAnalyticsExtractionHelper.java(2 hunks)src/main/java/ai/elimu/web/analytics/students/StudentsListController.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/main/java/ai/elimu/web/analytics/students/StudentsListController.java (1)
Learnt from: jo-elimu
PR: elimu-ai/webapp#2186
File: src/main/java/ai/elimu/web/analytics/students/StudentsListController.java:28-31
Timestamp: 2025-05-10T08:18:07.214Z
Learning: In the elimu-ai webapp project, temporary modifications to entity objects in read-only controllers (like redacting Android IDs in StudentsListController) are acceptable as they won't be persisted back to the database.
🧬 Code Graph Analysis (2)
src/main/java/ai/elimu/tasks/analytics/StoryBookLearningEventImportScheduler.java (3)
src/main/java/ai/elimu/util/csv/CsvAnalyticsExtractionHelper.java (1)
Slf4j(27-221)src/main/java/ai/elimu/tasks/analytics/VideoLearningEventImportScheduler.java (1)
Service(43-110)src/main/java/ai/elimu/tasks/analytics/WordLearningEventImportScheduler.java (1)
Service(43-110)
src/main/java/ai/elimu/web/analytics/students/StudentsListController.java (1)
src/main/java/ai/elimu/util/AnalyticsHelper.java (1)
AnalyticsHelper(6-43)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: test_rest
- GitHub Check: test_rest
- GitHub Check: test_rest
- GitHub Check: test_rest
🔇 Additional comments (16)
src/main/java/ai/elimu/tasks/analytics/VideoLearningEventImportScheduler.java (4)
3-3: Imports look good for Student integration.The necessary imports for
StudentDaoandStudententity have been added to support the new Student entity integration.Also applies to: 6-6
50-51: Dependency injection for StudentDao is properly configured.Good use of final field with Lombok's
@RequiredArgsConstructorfor dependency injection of the StudentDao.
52-52: Scheduler timing adjusted to stagger importers.The scheduling time has been modified from 30 to 40 minutes past the hour, which helps stagger the different import jobs (Word at 30, StoryBook at 35, Video at 40 minutes past the hour), avoiding potential resource contention.
87-95: Student entity creation looks good.The implementation follows the same pattern used in the other importers to create Student entities when they don't exist. The code properly checks for an existing student before creating a new one, and includes appropriate logging.
src/main/java/ai/elimu/web/analytics/students/StudentsListController.java (3)
20-24: Controller setup is well structured.Good use of Spring annotations and Lombok to create a clean controller definition with proper request mapping.
26-31: Dependencies are properly injected.Good use of final fields with Lombok's
@RequiredArgsConstructorfor dependency injection of all required DAOs.
58-61: Redacting Android IDs on entity objects is acceptable per project standards.Based on the retrieved learning, temporarily modifying entity objects in read-only controllers (like redacting Android IDs) is acceptable in this project as they won't be persisted back to the database.
src/main/java/ai/elimu/tasks/analytics/WordLearningEventImportScheduler.java (3)
43-51: Service structure looks good.The class is properly annotated with Spring and Lombok annotations, and dependencies are correctly injected via constructor injection.
52-52: Scheduler timing is well coordinated.The cron expression is set to run at 30 minutes past every hour, which works well with the staggered approach (Word at 30, StoryBook at 35, Video at 40 minutes past the hour).
87-95: Student entity creation is properly implemented.The code correctly checks for an existing student before creating a new one, and includes appropriate logging.
src/main/java/ai/elimu/tasks/analytics/StoryBookLearningEventImportScheduler.java (3)
43-51: Service structure looks good.The class is properly annotated with Spring and Lombok annotations, and dependencies are correctly injected via constructor injection.
52-52: Scheduler timing is well coordinated.The cron expression is set to run at 35 minutes past every hour, which works well with the staggered approach (Word at 30, StoryBook at 35, Video at 40 minutes past the hour).
87-95: Student entity creation is properly implemented.The code correctly checks for an existing student before creating a new one, and includes appropriate logging.
src/main/java/ai/elimu/util/csv/CsvAnalyticsExtractionHelper.java (3)
23-26: Good documentation addition!Adding clear documentation that explains the purpose of the helper class and its relationship to the Analytics Android app is a helpful improvement.
145-147: Good addition of additional_data handlingThe addition of support for the
additional_datafield in the StoryBookLearningEvent extraction is good and makes the implementation consistent with the other event extraction methods.
150-150: Good resource managementProperly closing the CSVParser after use is a good practice to prevent resource leaks.
Comment on lines
+53
to
+108
| public synchronized void execute() { | ||
| log.info("execute"); | ||
|
|
||
| // Lookup CSV files stored on the filesystem | ||
| File elimuAiDir = new File(System.getProperty("user.home"), ".elimu-ai"); | ||
| File languageDir = new File(elimuAiDir, "lang-" + Language.valueOf(ConfigHelper.getProperty("content.language"))); | ||
| File analyticsDir = new File(languageDir, "analytics"); | ||
| log.info("analyticsDir: " + analyticsDir); | ||
| analyticsDir.mkdirs(); | ||
| for (File analyticsDirFile : analyticsDir.listFiles()) { | ||
| if (analyticsDirFile.getName().startsWith("android-id-")) { | ||
| File androidIdDir = new File(analyticsDir, analyticsDirFile.getName()); | ||
| for (File androidIdDirFile : androidIdDir.listFiles()) { | ||
| if (androidIdDirFile.getName().startsWith("version-code-")) { | ||
| File versionCodeDir = new File(androidIdDir, androidIdDirFile.getName()); | ||
| for (File versionCodeDirFile : versionCodeDir.listFiles()) { | ||
| if (versionCodeDirFile.getName().equals("word-learning-events")) { | ||
| File wordLearningEventsDir = new File(versionCodeDir, versionCodeDirFile.getName()); | ||
| for (File csvFile : wordLearningEventsDir.listFiles()) { | ||
| log.info("csvFile: " + csvFile); | ||
|
|
||
| // Convert from CSV to Java | ||
| List<WordLearningEvent> events = CsvAnalyticsExtractionHelper.extractWordLearningEvents(csvFile); | ||
| log.info("events.size(): " + events.size()); | ||
|
|
||
| // Store in database | ||
| for (WordLearningEvent event : events) { | ||
| // Check if the event has already been stored in the database | ||
| WordLearningEvent existingWordLearningEvent = wordLearningEventDao.read(event.getTimestamp(), event.getAndroidId(), event.getApplication(), event.getWord()); | ||
| if (existingWordLearningEvent != null) { | ||
| log.warn("The event has already been stored in the database. Skipping data import."); | ||
| continue; | ||
| } | ||
|
|
||
| // Generate Student ID | ||
| Student existingStudent = studentDao.read(event.getAndroidId()); | ||
| if (existingStudent == null) { | ||
| Student student = new Student(); | ||
| student.setAndroidId(event.getAndroidId()); | ||
| studentDao.create(student); | ||
| log.info("Stored Student in database with ID " + student.getId()); | ||
| } | ||
|
|
||
| // Store the event in the database | ||
| wordLearningEventDao.create(event); | ||
| log.info("Stored event in database with ID " + event.getId()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| log.info("execute complete"); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting the nested directory traversal logic.
The file contains deeply nested loops (7 levels deep) to traverse the directory structure, which makes the code hard to read and maintain. This pattern is also duplicated across all three scheduler classes.
Consider extracting this logic into a reusable utility method that takes a directory type (e.g., "word-learning-events") and a callback function to process each CSV file found. This would significantly improve code readability and maintainability.
// Example utility method signature
private void processAnalyticsFiles(String eventType, BiConsumer<File, List<T>> processor) {
// Directory traversal code with callback for each CSV file
}
// Example usage
@Scheduled(cron = "00 30 * * * *")
public synchronized void execute() {
log.info("execute");
processAnalyticsFiles("word-learning-events", (csvFile, events) -> {
// Process each event from the CSV file
for (WordLearningEvent event : events) {
// Existing event processing logic
}
});
log.info("execute complete");
}
Comment on lines
+62
to
+106
| for (File analyticsDirFile : analyticsDir.listFiles()) { | ||
| if (analyticsDirFile.getName().startsWith("android-id-")) { | ||
| File androidIdDir = new File(analyticsDir, analyticsDirFile.getName()); | ||
| for (File androidIdDirFile : androidIdDir.listFiles()) { | ||
| if (androidIdDirFile.getName().startsWith("version-code-")) { | ||
| File versionCodeDir = new File(androidIdDir, androidIdDirFile.getName()); | ||
| for (File versionCodeDirFile : versionCodeDir.listFiles()) { | ||
| if (versionCodeDirFile.getName().equals("word-learning-events")) { | ||
| File wordLearningEventsDir = new File(versionCodeDir, versionCodeDirFile.getName()); | ||
| for (File csvFile : wordLearningEventsDir.listFiles()) { | ||
| log.info("csvFile: " + csvFile); | ||
|
|
||
| // Convert from CSV to Java | ||
| List<WordLearningEvent> events = CsvAnalyticsExtractionHelper.extractWordLearningEvents(csvFile); | ||
| log.info("events.size(): " + events.size()); | ||
|
|
||
| // Store in database | ||
| for (WordLearningEvent event : events) { | ||
| // Check if the event has already been stored in the database | ||
| WordLearningEvent existingWordLearningEvent = wordLearningEventDao.read(event.getTimestamp(), event.getAndroidId(), event.getApplication(), event.getWord()); | ||
| if (existingWordLearningEvent != null) { | ||
| log.warn("The event has already been stored in the database. Skipping data import."); | ||
| continue; | ||
| } | ||
|
|
||
| // Generate Student ID | ||
| Student existingStudent = studentDao.read(event.getAndroidId()); | ||
| if (existingStudent == null) { | ||
| Student student = new Student(); | ||
| student.setAndroidId(event.getAndroidId()); | ||
| studentDao.create(student); | ||
| log.info("Stored Student in database with ID " + student.getId()); | ||
| } | ||
|
|
||
| // Store the event in the database | ||
| wordLearningEventDao.create(event); | ||
| log.info("Stored event in database with ID " + event.getId()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Add null checks for listFiles() results.
The code calls listFiles() multiple times without checking if the result is null, which could lead to NullPointerExceptions if a directory is empty or inaccessible.
- for (File analyticsDirFile : analyticsDir.listFiles()) {
+ File[] analyticsDirFiles = analyticsDir.listFiles();
+ if (analyticsDirFiles != null) {
+ for (File analyticsDirFile : analyticsDirFiles) {
// existing code
- }
+ }
+ }Similar changes should be made for all other listFiles() calls in this file.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (File analyticsDirFile : analyticsDir.listFiles()) { | |
| if (analyticsDirFile.getName().startsWith("android-id-")) { | |
| File androidIdDir = new File(analyticsDir, analyticsDirFile.getName()); | |
| for (File androidIdDirFile : androidIdDir.listFiles()) { | |
| if (androidIdDirFile.getName().startsWith("version-code-")) { | |
| File versionCodeDir = new File(androidIdDir, androidIdDirFile.getName()); | |
| for (File versionCodeDirFile : versionCodeDir.listFiles()) { | |
| if (versionCodeDirFile.getName().equals("word-learning-events")) { | |
| File wordLearningEventsDir = new File(versionCodeDir, versionCodeDirFile.getName()); | |
| for (File csvFile : wordLearningEventsDir.listFiles()) { | |
| log.info("csvFile: " + csvFile); | |
| // Convert from CSV to Java | |
| List<WordLearningEvent> events = CsvAnalyticsExtractionHelper.extractWordLearningEvents(csvFile); | |
| log.info("events.size(): " + events.size()); | |
| // Store in database | |
| for (WordLearningEvent event : events) { | |
| // Check if the event has already been stored in the database | |
| WordLearningEvent existingWordLearningEvent = wordLearningEventDao.read(event.getTimestamp(), event.getAndroidId(), event.getApplication(), event.getWord()); | |
| if (existingWordLearningEvent != null) { | |
| log.warn("The event has already been stored in the database. Skipping data import."); | |
| continue; | |
| } | |
| // Generate Student ID | |
| Student existingStudent = studentDao.read(event.getAndroidId()); | |
| if (existingStudent == null) { | |
| Student student = new Student(); | |
| student.setAndroidId(event.getAndroidId()); | |
| studentDao.create(student); | |
| log.info("Stored Student in database with ID " + student.getId()); | |
| } | |
| // Store the event in the database | |
| wordLearningEventDao.create(event); | |
| log.info("Stored event in database with ID " + event.getId()); | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| // Add null-check for analyticsDir.listFiles() | |
| File[] analyticsDirFiles = analyticsDir.listFiles(); | |
| if (analyticsDirFiles != null) { | |
| for (File analyticsDirFile : analyticsDirFiles) { | |
| if (analyticsDirFile.getName().startsWith("android-id-")) { | |
| File androidIdDir = new File(analyticsDir, analyticsDirFile.getName()); | |
| for (File androidIdDirFile : androidIdDir.listFiles()) { | |
| if (androidIdDirFile.getName().startsWith("version-code-")) { | |
| File versionCodeDir = new File(androidIdDir, androidIdDirFile.getName()); | |
| for (File versionCodeDirFile : versionCodeDir.listFiles()) { | |
| if (versionCodeDirFile.getName().equals("word-learning-events")) { | |
| File wordLearningEventsDir = new File(versionCodeDir, versionCodeDirFile.getName()); | |
| for (File csvFile : wordLearningEventsDir.listFiles()) { | |
| log.info("csvFile: " + csvFile); | |
| // Convert from CSV to Java | |
| List<WordLearningEvent> events = CsvAnalyticsExtractionHelper.extractWordLearningEvents(csvFile); | |
| log.info("events.size(): " + events.size()); | |
| // Store in database | |
| for (WordLearningEvent event : events) { | |
| // Check if the event has already been stored in the database | |
| WordLearningEvent existingWordLearningEvent = wordLearningEventDao.read( | |
| event.getTimestamp(), | |
| event.getAndroidId(), | |
| event.getApplication(), | |
| event.getWord() | |
| ); | |
| if (existingWordLearningEvent != null) { | |
| log.warn("The event has already been stored in the database. Skipping data import."); | |
| continue; | |
| } | |
| // Generate Student ID | |
| Student existingStudent = studentDao.read(event.getAndroidId()); | |
| if (existingStudent == null) { | |
| Student student = new Student(); | |
| student.setAndroidId(event.getAndroidId()); | |
| studentDao.create(student); | |
| log.info("Stored Student in database with ID " + student.getId()); | |
| } | |
| // Store the event in the database | |
| wordLearningEventDao.create(event); | |
| log.info("Stored event in database with ID " + event.getId()); | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } | |
| } |
Comment on lines
+1
to
+110
| package ai.elimu.tasks.analytics; | ||
|
|
||
| import ai.elimu.dao.StudentDao; | ||
| import ai.elimu.dao.StoryBookLearningEventDao; | ||
| import ai.elimu.entity.analytics.StoryBookLearningEvent; | ||
| import ai.elimu.entity.analytics.students.Student; | ||
| import ai.elimu.model.v2.enums.Language; | ||
| import ai.elimu.rest.v2.analytics.StoryBookLearningEventsRestController; | ||
| import ai.elimu.util.ConfigHelper; | ||
| import ai.elimu.util.csv.CsvAnalyticsExtractionHelper; | ||
| import java.io.File; | ||
| import java.util.List; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.scheduling.annotation.Scheduled; | ||
| import org.springframework.stereotype.Service; | ||
|
|
||
| /** | ||
| * Extracts learning events from CSV files previously received by the {@link StoryBookLearningEventsRestController}, and imports them into the database. | ||
| * <p/> | ||
| * <p> | ||
| * Expected folder structure: | ||
| * <pre> | ||
| * ├── lang-ENG | ||
| * │ ├── analytics | ||
| * │ │ ├── android-id-e387e38700000001 | ||
| * │ │ │ └── version-code-3001018 | ||
| * │ │ │ └── storybook-learning-events | ||
| * │ │ │ ├── e387e38700000001_3001018_storybook-learning-events_2024-10-09.csv | ||
| * │ │ │ ├── e387e38700000001_3001018_storybook-learning-events_2024-10-10.csv | ||
| * │ │ │ ├── e387e38700000001_3001018_storybook-learning-events_2024-10-11.csv | ||
| * │ │ │ ├── e387e38700000001_3001018_storybook-learning-events_2024-10-14.csv | ||
| * │ │ │ ├── e387e38700000001_3001018_storybook-learning-events_2024-10-18.csv | ||
| * │ │ │ └── e387e38700000001_3001018_storybook-learning-events_2024-10-20.csv | ||
| * │ │ ├── android-id-e387e38700000002 | ||
| * │ │ │ └── version-code-3001018 | ||
| * │ │ │ └── storybook-learning-events | ||
| * │ │ │ ├── e387e38700000002_3001018_storybook-learning-events_2024-10-09.csv | ||
| * │ │ │ ├── e387e38700000002_3001018_storybook-learning-events_2024-10-10.csv | ||
| * │ │ │ ├── e387e38700000002_3001018_storybook-learning-events_2024-10-11.csv | ||
| * </pre> | ||
| */ | ||
| @Service | ||
| @RequiredArgsConstructor | ||
| @Slf4j | ||
| public class StoryBookLearningEventImportScheduler { | ||
|
|
||
| private final StoryBookLearningEventDao storyBookLearningEventDao; | ||
|
|
||
| private final StudentDao studentDao; | ||
|
|
||
| @Scheduled(cron = "00 35 * * * *") // 35 minutes past every hour | ||
| public synchronized void execute() { | ||
| log.info("execute"); | ||
|
|
||
| // Lookup CSV files stored on the filesystem | ||
| File elimuAiDir = new File(System.getProperty("user.home"), ".elimu-ai"); | ||
| File languageDir = new File(elimuAiDir, "lang-" + Language.valueOf(ConfigHelper.getProperty("content.language"))); | ||
| File analyticsDir = new File(languageDir, "analytics"); | ||
| log.info("analyticsDir: " + analyticsDir); | ||
| analyticsDir.mkdirs(); | ||
| for (File analyticsDirFile : analyticsDir.listFiles()) { | ||
| if (analyticsDirFile.getName().startsWith("android-id-")) { | ||
| File androidIdDir = new File(analyticsDir, analyticsDirFile.getName()); | ||
| for (File androidIdDirFile : androidIdDir.listFiles()) { | ||
| if (androidIdDirFile.getName().startsWith("version-code-")) { | ||
| File versionCodeDir = new File(androidIdDir, androidIdDirFile.getName()); | ||
| for (File versionCodeDirFile : versionCodeDir.listFiles()) { | ||
| if (versionCodeDirFile.getName().equals("storybook-learning-events")) { | ||
| File storyBookLearningEventsDir = new File(versionCodeDir, versionCodeDirFile.getName()); | ||
| for (File csvFile : storyBookLearningEventsDir.listFiles()) { | ||
| log.info("csvFile: " + csvFile); | ||
|
|
||
| // Convert from CSV to Java | ||
| List<StoryBookLearningEvent> events = CsvAnalyticsExtractionHelper.extractStoryBookLearningEvents(csvFile); | ||
| log.info("events.size(): " + events.size()); | ||
|
|
||
| // Store in database | ||
| for (StoryBookLearningEvent event : events) { | ||
| // Check if the event has already been stored in the database | ||
| StoryBookLearningEvent existingStoryBookLearningEvent = storyBookLearningEventDao.read(event.getTimestamp(), event.getAndroidId(), event.getApplication(), event.getStoryBook()); | ||
| if (existingStoryBookLearningEvent != null) { | ||
| log.warn("The event has already been stored in the database. Skipping data import."); | ||
| continue; | ||
| } | ||
|
|
||
| // Generate Student ID | ||
| Student existingStudent = studentDao.read(event.getAndroidId()); | ||
| if (existingStudent == null) { | ||
| Student student = new Student(); | ||
| student.setAndroidId(event.getAndroidId()); | ||
| studentDao.create(student); | ||
| log.info("Stored Student in database with ID " + student.getId()); | ||
| } | ||
|
|
||
| // Store the event in the database | ||
| storyBookLearningEventDao.create(event); | ||
| log.info("Stored event in database with ID " + event.getId()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| log.info("execute complete"); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Significant code duplication across scheduler classes.
This class is almost identical to WordLearningEventImportScheduler and very similar to VideoLearningEventImportScheduler, with just minor differences in event types and variable names.
Consider creating an abstract base class that handles the common functionality, with concrete implementations for each event type. This would eliminate most of the duplication and improve maintainability.
public abstract class LearningEventImportScheduler<T> {
protected abstract String getEventDirectoryName();
protected abstract List<T> extractEventsFromCsv(File csvFile);
protected abstract T findExistingEvent(T event);
protected abstract void createEvent(T event);
protected final StudentDao studentDao;
// Common execute method with template pattern
public synchronized void execute() {
// Common directory traversal code
// Call abstract methods for type-specific behavior
}
// Common method to create Student if needed
protected void ensureStudentExists(String androidId) {
// Implementation
}
}
// Concrete implementation example
@Service
@RequiredArgsConstructor
@Slf4j
public class StoryBookLearningEventImportScheduler extends LearningEventImportScheduler<StoryBookLearningEvent> {
private final StoryBookLearningEventDao storyBookLearningEventDao;
@Override
protected String getEventDirectoryName() {
return "storybook-learning-events";
}
@Override
protected List<StoryBookLearningEvent> extractEventsFromCsv(File csvFile) {
return CsvAnalyticsExtractionHelper.extractStoryBookLearningEvents(csvFile);
}
// Other overridden methods
@Scheduled(cron = "00 35 * * * *")
@Override
public synchronized void execute() {
super.execute();
}
}
Comment on lines
+62
to
+106
| for (File analyticsDirFile : analyticsDir.listFiles()) { | ||
| if (analyticsDirFile.getName().startsWith("android-id-")) { | ||
| File androidIdDir = new File(analyticsDir, analyticsDirFile.getName()); | ||
| for (File androidIdDirFile : androidIdDir.listFiles()) { | ||
| if (androidIdDirFile.getName().startsWith("version-code-")) { | ||
| File versionCodeDir = new File(androidIdDir, androidIdDirFile.getName()); | ||
| for (File versionCodeDirFile : versionCodeDir.listFiles()) { | ||
| if (versionCodeDirFile.getName().equals("storybook-learning-events")) { | ||
| File storyBookLearningEventsDir = new File(versionCodeDir, versionCodeDirFile.getName()); | ||
| for (File csvFile : storyBookLearningEventsDir.listFiles()) { | ||
| log.info("csvFile: " + csvFile); | ||
|
|
||
| // Convert from CSV to Java | ||
| List<StoryBookLearningEvent> events = CsvAnalyticsExtractionHelper.extractStoryBookLearningEvents(csvFile); | ||
| log.info("events.size(): " + events.size()); | ||
|
|
||
| // Store in database | ||
| for (StoryBookLearningEvent event : events) { | ||
| // Check if the event has already been stored in the database | ||
| StoryBookLearningEvent existingStoryBookLearningEvent = storyBookLearningEventDao.read(event.getTimestamp(), event.getAndroidId(), event.getApplication(), event.getStoryBook()); | ||
| if (existingStoryBookLearningEvent != null) { | ||
| log.warn("The event has already been stored in the database. Skipping data import."); | ||
| continue; | ||
| } | ||
|
|
||
| // Generate Student ID | ||
| Student existingStudent = studentDao.read(event.getAndroidId()); | ||
| if (existingStudent == null) { | ||
| Student student = new Student(); | ||
| student.setAndroidId(event.getAndroidId()); | ||
| studentDao.create(student); | ||
| log.info("Stored Student in database with ID " + student.getId()); | ||
| } | ||
|
|
||
| // Store the event in the database | ||
| storyBookLearningEventDao.create(event); | ||
| log.info("Stored event in database with ID " + event.getId()); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Add null checks for listFiles() results.
The code calls listFiles() multiple times without checking if the result is null, which could lead to NullPointerExceptions if a directory is empty or inaccessible.
- for (File analyticsDirFile : analyticsDir.listFiles()) {
+ File[] analyticsDirFiles = analyticsDir.listFiles();
+ if (analyticsDirFiles != null) {
+ for (File analyticsDirFile : analyticsDirFiles) {
// existing code
- }
+ }
+ }Similar changes should be made for all other listFiles() calls in this file.
Comment on lines
+30
to
+92
| public static List<WordLearningEvent> extractWordLearningEvents(File csvFile) { | ||
| log.info("extractWordLearningEvents"); | ||
|
|
||
| List<WordLearningEvent> wordLearningEvents = new ArrayList<>(); | ||
|
|
||
| // Iterate each row in the CSV file | ||
| Path csvFilePath = Paths.get(csvFile.toURI()); | ||
| log.info("csvFilePath: " + csvFilePath); | ||
| try { | ||
| Reader reader = Files.newBufferedReader(csvFilePath); | ||
| CSVFormat csvFormat = CSVFormat.DEFAULT | ||
| .withHeader( | ||
| "id", // The Android database ID | ||
| "timestamp", | ||
| "android_id", | ||
| "package_name", | ||
| "word_id", | ||
| "word_text", | ||
| "learning_event_type", | ||
| "additional_data" | ||
| ) | ||
| .withSkipHeaderRecord(); | ||
| log.info("header: " + Arrays.toString(csvFormat.getHeader())); | ||
| CSVParser csvParser = new CSVParser(reader, csvFormat); | ||
| for (CSVRecord csvRecord : csvParser) { | ||
| log.info("csvRecord: " + csvRecord); | ||
|
|
||
| // Convert from CSV to Java | ||
|
|
||
| WordLearningEvent wordLearningEvent = new WordLearningEvent(); | ||
|
|
||
| long timestampInMillis = Long.valueOf(csvRecord.get("time")); | ||
| Calendar timestamp = Calendar.getInstance(TimeZone.getTimeZone("UTC")); | ||
| timestamp.setTimeInMillis(timestampInMillis); | ||
| wordLearningEvent.setTimestamp(timestamp); | ||
|
|
||
| String androidId = csvRecord.get("android_id"); | ||
| wordLearningEvent.setAndroidId(androidId); | ||
|
|
||
| String packageName = csvRecord.get("package_name"); | ||
| wordLearningEvent.setPackageName(packageName); | ||
|
|
||
| Long wordId = Long.valueOf(csvRecord.get("word_id")); | ||
| // wordLearningEvent.setWordId(wordId); | ||
|
|
||
| String wordText = csvRecord.get("word_text"); | ||
| wordLearningEvent.setWordText(wordText); | ||
|
|
||
| LearningEventType learningEventType = LearningEventType.valueOf(csvRecord.get("learning_event_type")); | ||
| wordLearningEvent.setLearningEventType(learningEventType); | ||
|
|
||
| String additionalData = csvRecord.get("additional_data"); | ||
| wordLearningEvent.setAdditionalData(additionalData); | ||
|
|
||
| wordLearningEvents.add(wordLearningEvent); | ||
| } | ||
| csvParser.close(); | ||
| } catch (IOException ex) { | ||
| log.error(ex.getMessage()); | ||
| } | ||
|
|
||
| return wordLearningEvents; | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing error handling
All three extraction methods catch IOExceptions but only log the error message. Consider either re-throwing the exception or implementing more robust error handling, especially since this code is likely part of a data import pipeline where failures need to be properly reported.
} catch (IOException ex) {
- log.error(ex.getMessage());
+ log.error("Failed to parse CSV file: " + csvFilePath, ex);
+ // Consider handling the error more robustly, e.g.:
+ // throw new RuntimeException("Failed to parse CSV file: " + csvFilePath, ex);
}Also applies to: 94-156, 158-220
💡 Verification agent
❓ Verification inconclusive
Fix timestamp field name mismatch in WordLearningEvent extraction
There's a field name mismatch in the extractWordLearningEvents method. Line 61 tries to retrieve the timestamp using csvRecord.get("time"), but the header defined in lines 41-50 uses "timestamp" instead.
- long timestampInMillis = Long.valueOf(csvRecord.get("time"));
+ long timestampInMillis = Long.valueOf(csvRecord.get("timestamp"));Additionally, I notice that line 73 parses the word ID but it's commented out: // wordLearningEvent.setWordId(wordId);. Is this intentional? If the ID is needed for future functionality, consider adding a TODO comment to explain why it's parsed but not used.
Fix timestamp header key mismatch in extractWordLearningEvents
The CSV header defines "timestamp", but the code is calling csvRecord.get("time"), which will fail at runtime. Also, you’re parsing wordId but never setting it on the event—either set it or add a comment explaining why it’s omitted.
Please update in src/main/java/ai/elimu/util/csv/CsvAnalyticsExtractionHelper.java:
- Line ~61: change the key from
"time"to"timestamp". - Line ~73: either uncomment and call
wordLearningEvent.setWordId(wordId);or add a// TODOexplaining why you’re parsing but not using it.
Suggested diff:
- long timestampInMillis = Long.valueOf(csvRecord.get("time"));
+ long timestampInMillis = Long.valueOf(csvRecord.get("timestamp"));
- // wordLearningEvent.setWordId(wordId);
+ wordLearningEvent.setWordId(wordId);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| public static List<WordLearningEvent> extractWordLearningEvents(File csvFile) { | |
| log.info("extractWordLearningEvents"); | |
| List<WordLearningEvent> wordLearningEvents = new ArrayList<>(); | |
| // Iterate each row in the CSV file | |
| Path csvFilePath = Paths.get(csvFile.toURI()); | |
| log.info("csvFilePath: " + csvFilePath); | |
| try { | |
| Reader reader = Files.newBufferedReader(csvFilePath); | |
| CSVFormat csvFormat = CSVFormat.DEFAULT | |
| .withHeader( | |
| "id", // The Android database ID | |
| "timestamp", | |
| "android_id", | |
| "package_name", | |
| "word_id", | |
| "word_text", | |
| "learning_event_type", | |
| "additional_data" | |
| ) | |
| .withSkipHeaderRecord(); | |
| log.info("header: " + Arrays.toString(csvFormat.getHeader())); | |
| CSVParser csvParser = new CSVParser(reader, csvFormat); | |
| for (CSVRecord csvRecord : csvParser) { | |
| log.info("csvRecord: " + csvRecord); | |
| // Convert from CSV to Java | |
| WordLearningEvent wordLearningEvent = new WordLearningEvent(); | |
| long timestampInMillis = Long.valueOf(csvRecord.get("time")); | |
| Calendar timestamp = Calendar.getInstance(TimeZone.getTimeZone("UTC")); | |
| timestamp.setTimeInMillis(timestampInMillis); | |
| wordLearningEvent.setTimestamp(timestamp); | |
| String androidId = csvRecord.get("android_id"); | |
| wordLearningEvent.setAndroidId(androidId); | |
| String packageName = csvRecord.get("package_name"); | |
| wordLearningEvent.setPackageName(packageName); | |
| Long wordId = Long.valueOf(csvRecord.get("word_id")); | |
| // wordLearningEvent.setWordId(wordId); | |
| String wordText = csvRecord.get("word_text"); | |
| wordLearningEvent.setWordText(wordText); | |
| LearningEventType learningEventType = LearningEventType.valueOf(csvRecord.get("learning_event_type")); | |
| wordLearningEvent.setLearningEventType(learningEventType); | |
| String additionalData = csvRecord.get("additional_data"); | |
| wordLearningEvent.setAdditionalData(additionalData); | |
| wordLearningEvents.add(wordLearningEvent); | |
| } | |
| csvParser.close(); | |
| } catch (IOException ex) { | |
| log.error(ex.getMessage()); | |
| } | |
| return wordLearningEvents; | |
| } | |
| public static List<WordLearningEvent> extractWordLearningEvents(File csvFile) { | |
| log.info("extractWordLearningEvents"); | |
| List<WordLearningEvent> wordLearningEvents = new ArrayList<>(); | |
| // Iterate each row in the CSV file | |
| Path csvFilePath = Paths.get(csvFile.toURI()); | |
| log.info("csvFilePath: " + csvFilePath); | |
| try { | |
| Reader reader = Files.newBufferedReader(csvFilePath); | |
| CSVFormat csvFormat = CSVFormat.DEFAULT | |
| .withHeader( | |
| "id", // The Android database ID | |
| "timestamp", | |
| "android_id", | |
| "package_name", | |
| "word_id", | |
| "word_text", | |
| "learning_event_type", | |
| "additional_data" | |
| ) | |
| .withSkipHeaderRecord(); | |
| log.info("header: " + Arrays.toString(csvFormat.getHeader())); | |
| CSVParser csvParser = new CSVParser(reader, csvFormat); | |
| for (CSVRecord csvRecord : csvParser) { | |
| log.info("csvRecord: " + csvRecord); | |
| // Convert from CSV to Java | |
| WordLearningEvent wordLearningEvent = new WordLearningEvent(); | |
| - long timestampInMillis = Long.valueOf(csvRecord.get("time")); | |
| + long timestampInMillis = Long.valueOf(csvRecord.get("timestamp")); | |
| Calendar timestamp = Calendar.getInstance(TimeZone.getTimeZone("UTC")); | |
| timestamp.setTimeInMillis(timestampInMillis); | |
| wordLearningEvent.setTimestamp(timestamp); | |
| String androidId = csvRecord.get("android_id"); | |
| wordLearningEvent.setAndroidId(androidId); | |
| String packageName = csvRecord.get("package_name"); | |
| wordLearningEvent.setPackageName(packageName); | |
| Long wordId = Long.valueOf(csvRecord.get("word_id")); | |
| - // wordLearningEvent.setWordId(wordId); | |
| + wordLearningEvent.setWordId(wordId); | |
| String wordText = csvRecord.get("word_text"); | |
| wordLearningEvent.setWordText(wordText); | |
| LearningEventType learningEventType = LearningEventType.valueOf(csvRecord.get("learning_event_type")); | |
| wordLearningEvent.setLearningEventType(learningEventType); | |
| String additionalData = csvRecord.get("additional_data"); | |
| wordLearningEvent.setAdditionalData(additionalData); | |
| wordLearningEvents.add(wordLearningEvent); | |
| } | |
| csvParser.close(); | |
| } catch (IOException ex) { | |
| log.error(ex.getMessage()); | |
| } | |
| return wordLearningEvents; | |
| } |
This was referenced Jun 1, 2025
Merged
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Number
android_idwithstudent_id#2103Purpose
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.