Skip to content

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.

Sign up for GitHub

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

Merged
jo-elimu merged 2 commits into from
Jun 1, 2025
Merged

feat: export csv - storybook learning events, video learning events #2205

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d5c2b1e
feat: export csv - storybook learning events
jo-elimu Jun 1, 2025
c0105f6
feat: export csv - video learning events
jo-elimu Jun 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom-dependency-tree.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
ai.elimu:webapp:war:2.6.22-SNAPSHOT
ai.elimu:webapp:war:2.6.23-SNAPSHOT
+- ai.elimu:model:jar:model-2.0.97:compile
| \- com.google.code.gson:gson:jar:2.13.0:compile
| \- com.google.errorprone:error_prone_annotations:jar:2.37.0:compile
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/ai/elimu/dao/jpa/LetterSoundAssessmentEventDaoJpa.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public List<LetterSoundAssessmentEvent> readAll(String androidId) throws DataAcc
"SELECT event " +
"FROM LetterSoundAssessmentEvent event " +
"WHERE event.androidId = :androidId " +
"ORDER BY event.timestamp")
"ORDER BY event.id")
.setParameter("androidId", androidId)
.getResultList();
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/ai/elimu/dao/jpa/LetterSoundLearningEventDaoJpa.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public List<LetterSoundLearningEvent> readAll(String androidId) throws DataAcces
"SELECT event " +
"FROM LetterSoundLearningEvent event " +
"WHERE event.androidId = :androidId " +
"ORDER BY event.timestamp")
"ORDER BY event.id")
.setParameter("androidId", androidId)
.getResultList();
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/ai/elimu/dao/jpa/StoryBookLearningEventDaoJpa.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public List<StoryBookLearningEvent> readAll(String androidId) throws DataAccessE
"SELECT event " +
"FROM StoryBookLearningEvent event " +
"WHERE event.androidId = :androidId " +
"ORDER BY event.timestamp")
"ORDER BY event.id")
.setParameter("androidId", androidId)
.getResultList();
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/ai/elimu/dao/jpa/VideoLearningEventDaoJpa.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public List<VideoLearningEvent> readAll(String androidId) throws DataAccessExcep
"SELECT event " +
"FROM VideoLearningEvent event " +
"WHERE event.androidId = :androidId " +
"ORDER BY event.timestamp")
"ORDER BY event.id")
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 1, 2025

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.timestamp to ORDER BY event.id may not preserve chronological ordering. While IDs are typically sequential, they don't guarantee chronological order if:

  • Events are inserted out of sequence
  • There are concurrent insertions
  • Database transactions complete in different orders

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:

-            "ORDER BY event.id")
+            "ORDER BY event.timestamp ASC")

Or use a compound ordering strategy:

-            "ORDER BY event.id")
+            "ORDER BY event.timestamp ASC, event.id ASC")
🤖 Prompt for AI Agents 
In src/main/java/ai/elimu/dao/jpa/VideoLearningEventDaoJpa.java at line 51, the
query orders events by event.id which may not reflect the true chronological
order due to possible out-of-sequence inserts or concurrent transactions. To
ensure events are displayed and exported in the correct chronological order,
change the ORDER BY clause to use event.timestamp instead of event.id, or use a
compound ordering such as ORDER BY event.timestamp, event.id to maintain
consistent and accurate event sequencing.

.setParameter("androidId", androidId)
.getResultList();
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/ai/elimu/dao/jpa/WordLearningEventDaoJpa.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 1, 2025

Choose a reason for hiding this comment

The 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 event.id instead of event.timestamp could break chronological ordering of learning events. This is particularly important for analytics and learning progression tracking.

Consider maintaining chronological ordering:

-            "ORDER BY event.id")
+            "ORDER BY event.timestamp ASC")
📝 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
"ORDER BY event.id")
- "ORDER BY event.id")
+ "ORDER BY event.timestamp ASC")
🤖 Prompt for AI Agents 
In src/main/java/ai/elimu/dao/jpa/WordLearningEventDaoJpa.java at line 43, the
query orders results by event.id, which may disrupt chronological order. To fix
this, change the ORDER BY clause to use event.timestamp instead of event.id to
ensure learning events are returned in the correct chronological sequence.

.setParameter("androidId", androidId)
.getResultList();
}
Expand Down
70 changes: 0 additions & 70 deletions src/main/java/ai/elimu/web/analytics/StoryBookLearningEventListController.java
View file
Open in desktop

This file was deleted.

31 changes: 20 additions & 11 deletions ...BookLearningEventCsvExportController.java → ...ookLearningEventsCsvExportController.java
View file
Open in desktop
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;
Expand All @@ -14,24 +16,31 @@
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
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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 NullPointerException.

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

‼️ 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
Student student = studentDao.read(studentId);
log.info("student.getAndroidId(): " + student.getAndroidId());
List<StoryBookLearningEvent> storyBookLearningEvents = storyBookLearningEventDao.readAll(student.getAndroidId());
log.info("storyBookLearningEvents.size(): " + storyBookLearningEvents.size());
Student student = studentDao.read(studentId);
if (student == null) {
response.setStatus(HttpServletResponse.SC_NOT_FOUND);
return;
}
log.info("student.getAndroidId(): " + student.getAndroidId());
List<StoryBookLearningEvent> storyBookLearningEvents = storyBookLearningEventDao.readAll(student.getAndroidId());
log.info("storyBookLearningEvents.size(): " + storyBookLearningEvents.size());
🤖 Prompt for AI Agents 
In
src/main/java/ai/elimu/web/analytics/students/StoryBookLearningEventsCsvExportController.java
around lines 40 to 44, add a null check after retrieving the student with
studentDao.read(studentId) to handle the case where the student is not found. If
the student is null, handle this scenario appropriately (e.g., log a warning and
return or throw an exception) before accessing student.getAndroidId() to prevent
a NullPointerException.

for (StoryBookLearningEvent storyBookLearningEvent : storyBookLearningEvents) {
storyBookLearningEvent.setAndroidId(AnalyticsHelper.redactAndroidId(storyBookLearningEvent.getAndroidId()));
Expand All @@ -41,11 +50,11 @@ public void handleRequest(
.setHeader(
"id", // The Room database ID
"timestamp",
"android_id",
"package_name",
"storybook_id",
"storybook_title",
"learning_event_type"
"storybook_id",
"learning_event_type",
"additional_data"
)
.build();

Expand All @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/ai/elimu/web/analytics/students/WordLearningEventsCsvExportController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void handleRequest(
wordLearningEvent.getTimestamp().getTimeInMillis(),
wordLearningEvent.getPackageName(),
wordLearningEvent.getWordText(),
(wordLearningEvent.getWord() != null) ? wordLearningEvent.getWord().getId() : 0,
(wordLearningEvent.getWord() == null) ? null : wordLearningEvent.getWord().getId(),
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Improved data integrity for null word references.

This change correctly represents missing word references as null instead of 0 in the CSV export, which improves data integrity and semantic accuracy.

Please verify that downstream systems consuming this CSV data can handle null values in the word_id column, as this changes the data format from the previous implementation.

🏁 Script executed:

#!/bin/bash
# Description: Check for other CSV export controllers to ensure consistent null handling
# Expected: Find similar patterns in other CSV export controllers

rg -A 5 -B 5 "== null.*\?.*:" --type java src/main/java/ai/elimu/web/analytics/

Length of output: 1578

Ensure downstream compatibility and consistent null handling

The change to output null instead of 0 for missing word_id improves data integrity and semantic correctness. A quick search across other CSV export controllers in src/main/java/ai/elimu/web/analytics/ found no similar null-handling logic:

• Only WordLearningEventsCsvExportController.java uses

(wordLearningEvent.getWord() == null) ? null : wordLearningEvent.getWord().getId()

in its CSV export.

Recommendations:

  • Verify that all downstream systems consuming the CSV can handle null values in the word_id column without errors.
  • If other controllers or consumers expect 0 for missing IDs, consider updating them to accept null or standardize on a single representation across all exports.
🤖 Prompt for AI Agents 
In
src/main/java/ai/elimu/web/analytics/students/WordLearningEventsCsvExportController.java
at line 68, the CSV export now outputs null instead of 0 for missing word IDs to
improve data integrity. Verify that all downstream systems consuming this CSV
can handle null values in the word_id column without errors. If other CSV export
controllers or consumers expect 0 for missing IDs, update them to accept null or
standardize the representation across all exports for consistency.

// wordLearningEvent.getWordId(), https://github.com/elimu-ai/webapp/issues/2113
wordLearningEvent.getLearningEventType(),
wordLearningEvent.getAdditionalData()
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/ai/elimu/web/servlet/CustomDispatcherServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ private void populateDatabase(WebApplicationContext webApplicationContext) {
while (!week.after(calendarNow)) {
List<Student> students = studentDao.readAll();
for (Student student : students) {
int randomNumberOfStoryBookLearningEvents = (int) (Math.random() * 8);
int randomNumberOfStoryBookLearningEvents = (int) (Math.random() * 10);
for (int i = 0; i < randomNumberOfStoryBookLearningEvents; i++) {
StoryBookLearningEvent storyBookLearningEvent = new StoryBookLearningEvent();
storyBookLearningEvent.setTimestamp(week);
Expand All @@ -341,7 +341,7 @@ private void populateDatabase(WebApplicationContext webApplicationContext) {
storyBookLearningEventDao.create(storyBookLearningEvent);
}

int randomNumberOfVideoLearningEvents = (int) (Math.random() * 8);
int randomNumberOfVideoLearningEvents = (int) (Math.random() * 10);
for (int i = 0; i < randomNumberOfVideoLearningEvents; i++) {
VideoLearningEvent videoLearningEvent = new VideoLearningEvent();
videoLearningEvent.setTimestamp(week);
Expand Down
69 changes: 69 additions & 0 deletions src/main/webapp/WEB-INF/jsp/analytics/students/id.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,19 @@

<h5 style="margin-top: 1em;">📚 Storybooks</h5>
<div class="card-panel">
<a id="exportStoryBookLearningEventsToCsvButton" class="right btn waves-effect waves-light grey-text white"
href="<spring:url value='/analytics/students/${student.id}/storybook-learning-events.csv' />">
Export to CSV<i class="material-icons right">vertical_align_bottom</i>
</a>
<script>
$(function() {
$('#exportStoryBookLearningEventsToCsvButton').click(function() {
console.info('#exportStoryBookLearningEventsToCsvButton click');
Materialize.toast('Preparing CSV file. Please wait...', 4000, 'rounded');
});
});
</script>

<h5>Storybook learning events (${fn:length(storyBookLearningEvents)})</h5>
<canvas id="storyBookChart"></canvas>
<script>
Expand All @@ -248,6 +261,34 @@
var storyBookCtx = document.getElementById('storyBookChart');
new Chart(storyBookCtx, storyBookConfig);
</script>

<table class="bordered highlight">
<thead>
<th>id</th>
<th>timestamp</th>
<th>package_name</th>
<th>storybook_title</th>
</thead>
<tbody>
<c:forEach var="i" begin="0" end="4">
<c:set var="storyBookLearningEvent" value="${storyBookLearningEvents[fn:length(storyBookLearningEvents) - 1 - i]}" />
<tr>
<td>
${storyBookLearningEvent.id}
</td>
<td>
${storyBookLearningEvent.timestamp.time}
</td>
<td>
<code>${storyBookLearningEvent.packageName}</code>
</td>
<td>
"${storyBookLearningEvent.storyBookTitle}"
</td>
</tr>
</c:forEach>
Comment on lines +273 to +289
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix potential index out of bounds in table display.

The table logic could fail if there are fewer than 5 storybook learning events. When i=4 and there are only 2 events, the expression storyBookLearningEvents[fn:length(storyBookLearningEvents) - 1 - i] would try to access index -3.

Add a condition to check if the event exists:

 <c:forEach var="i" begin="0" end="4">
-    <c:set var="storyBookLearningEvent" value="${storyBookLearningEvents[fn:length(storyBookLearningEvents) - 1 - i]}" />
-    <tr>
+    <c:if test="${(fn:length(storyBookLearningEvents) - 1 - i) >= 0}">
+        <c:set var="storyBookLearningEvent" value="${storyBookLearningEvents[fn:length(storyBookLearningEvents) - 1 - i]}" />
+        <tr>
         <td>
             ${storyBookLearningEvent.id}
         </td>
         ...
-    </tr>
+        </tr>
+    </c:if>
 </c:forEach>
📝 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
<c:forEach var="i" begin="0" end="4">
<c:set var="storyBookLearningEvent" value="${storyBookLearningEvents[fn:length(storyBookLearningEvents) - 1 - i]}" />
<tr>
<td>
${storyBookLearningEvent.id}
</td>
<td>
${storyBookLearningEvent.timestamp.time}
</td>
<td>
<code>${storyBookLearningEvent.packageName}</code>
</td>
<td>
"${storyBookLearningEvent.storyBookTitle}"
</td>
</tr>
</c:forEach>
<c:forEach var="i" begin="0" end="4">
<c:if test="${(fn:length(storyBookLearningEvents) - 1 - i) >= 0}">
<c:set var="storyBookLearningEvent"
value="${storyBookLearningEvents[fn:length(storyBookLearningEvents) - 1 - i]}" />
<tr>
<td>
${storyBookLearningEvent.id}
</td>
<td>
${storyBookLearningEvent.timestamp.time}
</td>
<td>
<code>${storyBookLearningEvent.packageName}</code>
</td>
<td>
"${storyBookLearningEvent.storyBookTitle}"
</td>
</tr>
</c:if>
</c:forEach>
🤖 Prompt for AI Agents 
In src/main/webapp/WEB-INF/jsp/analytics/students/id.jsp around lines 273 to
289, the loop assumes there are always at least 5 storyBookLearningEvents, which
can cause an index out of bounds error when there are fewer events. Modify the
<c:forEach> loop to iterate only up to the smaller of 4 or the last index of
storyBookLearningEvents, and add a condition inside the loop to check if the
event at the calculated index exists before rendering the table row.

</tbody>
</table>
</div>
<div style="clear: both;"></div>

Expand Down Expand Up @@ -278,5 +319,33 @@
var videoCtx = document.getElementById('videoChart');
new Chart(videoCtx, videoConfig);
</script>

<table class="bordered highlight">
<thead>
<th>id</th>
<th>timestamp</th>
<th>package_name</th>
<th>video_title</th>
</thead>
<tbody>
<c:forEach var="i" begin="0" end="4">
<c:set var="videoLearningEvent" value="${videoLearningEvents[fn:length(videoLearningEvents) - 1 - i]}" />
<tr>
<td>
${videoLearningEvent.id}
</td>
<td>
${videoLearningEvent.timestamp.time}
</td>
<td>
<code>${videoLearningEvent.packageName}</code>
</td>
<td>
"${videoLearningEvent.videoTitle}"
</td>
</tr>
</c:forEach>
Comment on lines +344 to +360
Copy link
Contributor

@coderabbitai coderabbitai bot Jun 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix potential index out of bounds in video events table.

Same issue as the storybook events table - this could fail if there are fewer than 5 video learning events.

Apply the same fix pattern:

 <c:forEach var="i" begin="0" end="4">
-    <c:set var="videoLearningEvent" value="${videoLearningEvents[fn:length(videoLearningEvents) - 1 - i]}" />
-    <tr>
+    <c:if test="${(fn:length(videoLearningEvents) - 1 - i) >= 0}">
+        <c:set var="videoLearningEvent" value="${videoLearningEvents[fn:length(videoLearningEvents) - 1 - i]}" />
+        <tr>
         <td>
             ${videoLearningEvent.id}
         </td>
         ...
-    </tr>
+        </tr>
+    </c:if>
 </c:forEach>
🤖 Prompt for AI Agents 
In src/main/webapp/WEB-INF/jsp/analytics/students/id.jsp around lines 331 to
347, the forEach loop assumes there are always at least 5 videoLearningEvents,
which can cause an index out of bounds error if there are fewer. Fix this by
adjusting the loop to iterate only up to the smaller of 4 or the last index of
videoLearningEvents, ensuring the index used to access videoLearningEvents is
always valid.

</tbody>
</table>
</div>
</content:section>
5 changes: 0 additions & 5 deletions src/test/java/selenium/analytics/MainAnalyticsPage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ public MainAnalyticsPage(WebDriver driver) {
ErrorHelper.verifyNoScriptOrMarkupError(driver);
}

public void pressStoryBookLearningEventsLink() {
WebElement link = driver.findElement(By.id("storyBookLearningEventsLink"));
link.click();
}

public void pressVideoLearningEventsLink() {
WebElement link = driver.findElement(By.id("videoLearningEventsLink"));
link.click();
Expand Down
19 changes: 0 additions & 19 deletions src/test/java/selenium/analytics/StoryBookLearningEventsPage.java
View file
Open in desktop

This file was deleted.

Loading