-
-
Notifications
You must be signed in to change notification settings - Fork 70
feat: export csv - storybook learning events, video learning events #2205
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -40,7 +40,7 @@ public List<WordLearningEvent> readAll(String androidId) throws DataAccessExcept | |||||||
| "SELECT event " + | ||||||||
| "FROM WordLearningEvent event " + | ||||||||
| "WHERE event.androidId = :androidId " + | ||||||||
| "ORDER BY event.timestamp") | ||||||||
| "ORDER BY event.id") | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Ordering by ID instead of timestamp may affect chronological sequence. This change mirrors the same pattern in other DAO classes, but ordering by Consider maintaining chronological ordering: - "ORDER BY event.id")
+ "ORDER BY event.timestamp ASC")📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||
| .setParameter("androidId", androidId) | ||||||||
| .getResultList(); | ||||||||
| } | ||||||||
|
|
||||||||
This file was deleted.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,9 @@ | ||||||||||||||||||||||||||||||
| package ai.elimu.web.analytics; | ||||||||||||||||||||||||||||||
| package ai.elimu.web.analytics.students; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| import ai.elimu.dao.StoryBookLearningEventDao; | ||||||||||||||||||||||||||||||
| import ai.elimu.dao.StudentDao; | ||||||||||||||||||||||||||||||
| import ai.elimu.entity.analytics.StoryBookLearningEvent; | ||||||||||||||||||||||||||||||
| import ai.elimu.entity.analytics.students.Student; | ||||||||||||||||||||||||||||||
| import ai.elimu.util.AnalyticsHelper; | ||||||||||||||||||||||||||||||
| import jakarta.servlet.http.HttpServletResponse; | ||||||||||||||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||||||||||||||
|
|
@@ -14,38 +16,45 @@ | |||||||||||||||||||||||||||||
| import org.apache.commons.csv.CSVPrinter; | ||||||||||||||||||||||||||||||
| import org.springframework.stereotype.Controller; | ||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.GetMapping; | ||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.PathVariable; | ||||||||||||||||||||||||||||||
| import org.springframework.web.bind.annotation.RequestMapping; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @Controller | ||||||||||||||||||||||||||||||
| @RequestMapping("/analytics/storybook-learning-event/list/storybook-learning-events.csv") | ||||||||||||||||||||||||||||||
| @RequestMapping("/analytics/students/{studentId}/storybook-learning-events.csv") | ||||||||||||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||||||||||||
| @Slf4j | ||||||||||||||||||||||||||||||
| public class StoryBookLearningEventCsvExportController { | ||||||||||||||||||||||||||||||
| public class StoryBookLearningEventsCsvExportController { | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| private final StudentDao studentDao; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| private final StoryBookLearningEventDao storyBookLearningEventDao; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| @GetMapping | ||||||||||||||||||||||||||||||
| public void handleRequest( | ||||||||||||||||||||||||||||||
| @PathVariable Long studentId, | ||||||||||||||||||||||||||||||
| HttpServletResponse response, | ||||||||||||||||||||||||||||||
| OutputStream outputStream | ||||||||||||||||||||||||||||||
| ) throws IOException { | ||||||||||||||||||||||||||||||
| log.info("handleRequest"); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| List<StoryBookLearningEvent> storyBookLearningEvents = storyBookLearningEventDao.readAll(); | ||||||||||||||||||||||||||||||
| Student student = studentDao.read(studentId); | ||||||||||||||||||||||||||||||
| log.info("student.getAndroidId(): " + student.getAndroidId()); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| List<StoryBookLearningEvent> storyBookLearningEvents = storyBookLearningEventDao.readAll(student.getAndroidId()); | ||||||||||||||||||||||||||||||
| log.info("storyBookLearningEvents.size(): " + storyBookLearningEvents.size()); | ||||||||||||||||||||||||||||||
|
Comment on lines
+40
to
44
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add null checking for student lookup. The code doesn't handle the case where a student with the given ID doesn't exist, which could result in a Add null checking to handle non-existent students: Student student = studentDao.read(studentId);
+if (student == null) {
+ response.setStatus(HttpServletResponse.SC_NOT_FOUND);
+ return;
+}
log.info("student.getAndroidId(): " + student.getAndroidId());📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
| for (StoryBookLearningEvent storyBookLearningEvent : storyBookLearningEvents) { | ||||||||||||||||||||||||||||||
| storyBookLearningEvent.setAndroidId(AnalyticsHelper.redactAndroidId(storyBookLearningEvent.getAndroidId())); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| CSVFormat csvFormat = CSVFormat.DEFAULT.builder() | ||||||||||||||||||||||||||||||
| .setHeader( | ||||||||||||||||||||||||||||||
| "id", // The Room database ID | ||||||||||||||||||||||||||||||
| "id", | ||||||||||||||||||||||||||||||
| "timestamp", | ||||||||||||||||||||||||||||||
| "android_id", | ||||||||||||||||||||||||||||||
| "package_name", | ||||||||||||||||||||||||||||||
| "storybook_id", | ||||||||||||||||||||||||||||||
| "storybook_title", | ||||||||||||||||||||||||||||||
| "learning_event_type" | ||||||||||||||||||||||||||||||
| "storybook_id", | ||||||||||||||||||||||||||||||
| "learning_event_type", | ||||||||||||||||||||||||||||||
| "additional_data" | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
@@ -58,14 +67,14 @@ public void handleRequest( | |||||||||||||||||||||||||||||
| csvPrinter.printRecord( | ||||||||||||||||||||||||||||||
| storyBookLearningEvent.getId(), | ||||||||||||||||||||||||||||||
| storyBookLearningEvent.getTimestamp().getTimeInMillis(), | ||||||||||||||||||||||||||||||
| storyBookLearningEvent.getAndroidId(), | ||||||||||||||||||||||||||||||
| storyBookLearningEvent.getPackageName(), | ||||||||||||||||||||||||||||||
| (storyBookLearningEvent.getStoryBook() == null) ? null : storyBookLearningEvent.getStoryBook().getId(), | ||||||||||||||||||||||||||||||
| storyBookLearningEvent.getStoryBookTitle(), | ||||||||||||||||||||||||||||||
| storyBookLearningEvent.getLearningEventType() | ||||||||||||||||||||||||||||||
| storyBookLearningEvent.getStoryBookId(), | ||||||||||||||||||||||||||||||
| storyBookLearningEvent.getLearningEventType(), | ||||||||||||||||||||||||||||||
| storyBookLearningEvent.getAdditionalData() | ||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||
| csvPrinter.flush(); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| csvPrinter.flush(); | ||||||||||||||||||||||||||||||
| csvPrinter.close(); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| String csvFileContent = stringWriter.toString(); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
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.
🛠️ Refactor suggestion
Consider the implications of ordering by ID instead of timestamp.
Changing from
ORDER BY event.timestamptoORDER BY event.idmay not preserve chronological ordering. While IDs are typically sequential, they don't guarantee chronological order if:Since this affects how events are displayed in analytics and exported in CSV files, consider whether chronological ordering is important for your use case.
If chronological ordering is required, consider reverting to timestamp ordering:
Or use a compound ordering strategy:
🤖 Prompt for AI Agents