Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
ai.elimu:webapp:war:2.6.17-SNAPSHOT
ai.elimu:webapp:war:2.6.20-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
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package ai.elimu.web.analytics;

import ai.elimu.dao.LetterSoundAssessmentEventDao;
import ai.elimu.dao.LetterSoundLearningEventDao;
import ai.elimu.dao.StoryBookLearningEventDao;
import ai.elimu.dao.StudentDao;
import ai.elimu.dao.VideoLearningEventDao;
import ai.elimu.dao.WordLearningEventDao;
import ai.elimu.entity.analytics.LetterSoundLearningEvent;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Controller;
Expand All @@ -17,10 +20,16 @@
@Slf4j
public class MainAnalyticsController {

private final StudentDao studentDao;

private final LetterSoundAssessmentEventDao letterSoundAssessmentEventDao;
private final LetterSoundLearningEventDao letterSoundLearningEventDao;

// private final WordAssessmentEventDao wordAssessmentEventDao;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical issue: Uncomment WordAssessmentEventDao or remove JSP reference.

The WordAssessmentEventDao is commented out, but the JSP file expects ${wordAssessmentEventCount}. This inconsistency will cause the JSP to display an empty value or error.

Either uncomment and properly inject the DAO:

-// private final WordAssessmentEventDao wordAssessmentEventDao;
+private final WordAssessmentEventDao wordAssessmentEventDao;

Or remove the corresponding row from the JSP table.

📝 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
// private final WordAssessmentEventDao wordAssessmentEventDao;
private final WordAssessmentEventDao wordAssessmentEventDao;
🤖 Prompt for AI Agents
In src/main/java/ai/elimu/web/analytics/MainAnalyticsController.java at line 28,
the WordAssessmentEventDao declaration is commented out, but the JSP expects the
variable wordAssessmentEventCount which depends on this DAO. To fix this,
uncomment the WordAssessmentEventDao declaration and ensure it is properly
injected and used to set the wordAssessmentEventCount attribute for the JSP.
Alternatively, if this data is no longer needed, remove the corresponding
reference from the JSP file to avoid errors.

private final WordLearningEventDao wordLearningEventDao;

// TODO: Numbers

private final StoryBookLearningEventDao storyBookLearningEventDao;

private final VideoLearningEventDao videoLearningEventDao;
Expand All @@ -29,9 +38,16 @@ public class MainAnalyticsController {
public String handleRequest(Model model) {
log.info("handleRequest");

model.addAttribute("studentCount", studentDao.readCount());

model.addAttribute("letterSoundAssessmentEventCount", letterSoundAssessmentEventDao.readCount());
model.addAttribute("letterSoundLearningEventCount", letterSoundLearningEventDao.readCount());

// model.addAttribute("wordAssessmentEventCount", wordAssessmentEventDao.readCount());
model.addAttribute("wordLearningEventCount", wordLearningEventDao.readCount());

model.addAttribute("storyBookLearningEventCount", storyBookLearningEventDao.readCount());

model.addAttribute("videoLearningEventCount", videoLearningEventDao.readCount());

return "analytics/main";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
@RequestMapping("/analytics/students/{studentId}/letter-sound-assessment-events.csv")
@RequiredArgsConstructor
@Slf4j
public class LetterSoundAssessmentEventCsvExportController {
public class LetterSoundAssessmentEventsCsvExportController {

private final StudentDao studentDao;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package ai.elimu.web.analytics;
package ai.elimu.web.analytics.students;

import ai.elimu.dao.LetterSoundLearningEventDao;
import ai.elimu.dao.StudentDao;
import ai.elimu.entity.analytics.LetterSoundLearningEvent;
import ai.elimu.util.AnalyticsHelper;
import ai.elimu.entity.analytics.students.Student;
import jakarta.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.io.OutputStream;
Expand All @@ -14,39 +15,41 @@
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/letter-sound-learning-event/list/letter-sound-learning-events.csv")
@RequestMapping("/analytics/students/{studentId}/letter-sound-learning-events.csv")
@RequiredArgsConstructor
@Slf4j
public class LetterSoundLearningEventCsvExportController {
public class LetterSoundLearningEventsCsvExportController {

private final StudentDao studentDao;

private final LetterSoundLearningEventDao letterSoundLearningEventDao;

@GetMapping
public void handleRequest(
@PathVariable Long studentId,
HttpServletResponse response,
OutputStream outputStream
) throws IOException {
log.info("handleRequest");

List<LetterSoundLearningEvent> letterSoundLearningEvents = letterSoundLearningEventDao.readAll();
Student student = studentDao.read(studentId);
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add null check for student entity.

The controller should handle the case where a student with the given ID doesn't exist.

     Student student = studentDao.read(studentId);
+    if (student == null) {
+        response.setStatus(HttpServletResponse.SC_NOT_FOUND);
+        return;
+    }
📝 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);
Student student = studentDao.read(studentId);
if (student == null) {
response.setStatus(HttpServletResponse.SC_NOT_FOUND);
return;
}
🤖 Prompt for AI Agents
In
src/main/java/ai/elimu/web/analytics/students/LetterSoundLearningEventsCsvExportController.java
at line 39, add a null check after retrieving the student entity with
studentDao.read(studentId). If the student is null, handle this case
appropriately by either returning an error response or throwing a suitable
exception to prevent further processing with a null student.

🛠️ Refactor suggestion

Add null check for student.

The code doesn't handle the case where studentDao.read(studentId) returns null. This could lead to a NullPointerException if the student ID doesn't exist.

 Student student = studentDao.read(studentId);
+if (student == null) {
+  log.warn("Student not found with ID: " + studentId);
+  response.sendError(HttpServletResponse.SC_NOT_FOUND, "Student 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);
Student student = studentDao.read(studentId);
if (student == null) {
log.warn("Student not found with ID: " + studentId);
response.sendError(HttpServletResponse.SC_NOT_FOUND, "Student not found");
return;
}
log.info("student.getAndroidId(): " + student.getAndroidId());
🤖 Prompt for AI Agents
In
src/main/java/ai/elimu/web/analytics/students/LetterSoundLearningEventsCsvExportController.java
at line 39, add a null check after calling studentDao.read(studentId) to handle
the case when it returns null. If the student is null, handle it appropriately
by either returning an error response or throwing a meaningful exception to
prevent a NullPointerException later in the code.

log.info("student.getAndroidId(): " + student.getAndroidId());
Comment on lines +39 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix privacy issue: Android ID logged without redaction.

The Android ID is being logged directly without redaction, which could compromise user privacy.

-    log.info("student.getAndroidId(): " + student.getAndroidId());
+    log.info("student.getAndroidId(): " + AnalyticsHelper.redactAndroidId(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());
Student student = studentDao.read(studentId);
log.info("student.getAndroidId(): " + AnalyticsHelper.redactAndroidId(student.getAndroidId()));
🤖 Prompt for AI Agents
In
src/main/java/ai/elimu/web/analytics/students/LetterSoundLearningEventsCsvExportController.java
around lines 39 to 40, the Android ID of the student is logged directly, which
poses a privacy risk. Modify the log statement to redact or mask the Android ID
before logging it, for example by replacing part of it with asterisks or only
logging a hash or partial value, to prevent exposing sensitive user information.

⚠️ Potential issue

Security concern: Avoid logging unredacted Android IDs.

The code logs the actual Android ID without redaction, which could expose sensitive user data in log files. Consider redacting the Android ID before logging or removing this log statement entirely.

-log.info("student.getAndroidId(): " + student.getAndroidId());
+log.info("student.getAndroidId(): " + AnalyticsHelper.redactAndroidId(student.getAndroidId()));

Or simply remove the log statement if it's not essential for debugging.

🤖 Prompt for AI Agents
In
src/main/java/ai/elimu/web/analytics/students/LetterSoundLearningEventsCsvExportController.java
at lines 39-40, the code logs the full Android ID, which is sensitive
information. To fix this, either remove the log statement entirely if it is not
necessary, or redact the Android ID by masking part of it before logging to
avoid exposing sensitive user data.


List<LetterSoundLearningEvent> letterSoundLearningEvents = letterSoundLearningEventDao.readAll(student.getAndroidId());
Comment on lines +39 to +42
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add null check 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.

 Student student = studentDao.read(studentId);
+if (student == null) {
+  log.warn("Student not found with ID: " + studentId);
+  response.sendError(HttpServletResponse.SC_NOT_FOUND, "Student 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<LetterSoundLearningEvent> letterSoundLearningEvents = letterSoundLearningEventDao.readAll(student.getAndroidId());
Student student = studentDao.read(studentId);
if (student == null) {
log.warn("Student not found with ID: " + studentId);
response.sendError(HttpServletResponse.SC_NOT_FOUND, "Student not found");
return;
}
log.info("student.getAndroidId(): " + student.getAndroidId());
List<LetterSoundLearningEvent> letterSoundLearningEvents =
letterSoundLearningEventDao.readAll(student.getAndroidId());
🤖 Prompt for AI Agents
In
src/main/java/ai/elimu/web/analytics/students/LetterSoundLearningEventsCsvExportController.java
around lines 39 to 42, add a null check after retrieving the student with
studentDao.read(studentId). If the student is null, handle this case
appropriately, such as logging a warning and returning early or throwing a
controlled exception, to prevent a NullPointerException when accessing
student.getAndroidId().

log.info("letterSoundLearningEvents.size(): " + letterSoundLearningEvents.size());
for (LetterSoundLearningEvent letterSoundLearningEvent : letterSoundLearningEvents) {
letterSoundLearningEvent.setAndroidId(AnalyticsHelper.redactAndroidId(letterSoundLearningEvent.getAndroidId()));
}

CSVFormat csvFormat = CSVFormat.DEFAULT.builder()
.setHeader(
"id", // The Room database ID
"id",
"timestamp",
"android_id",
"package_name",
// "letter_sound_letters",
// "letter_sound_sounds",
"letter_sound_id",
"letter_sound_letters",
"letter_sound_sounds",
"learning_event_type",
"additional_data"
)
.build();
Expand All @@ -60,12 +63,10 @@ public void handleRequest(
csvPrinter.printRecord(
letterSoundLearningEvent.getId(),
letterSoundLearningEvent.getTimestamp().getTimeInMillis(),
letterSoundLearningEvent.getAndroidId(),
letterSoundLearningEvent.getPackageName(),
// letterSoundLearningEvent.getLetterSoundLetters(),
// letterSoundLearningEvent.getLetterSoundSounds(),
letterSoundLearningEvent.getLetterSoundId(),
new String[] {}, // TODO
new String[] {}, // TODO
letterSoundLearningEvent.getLearningEventType(),
letterSoundLearningEvent.getAdditionalData()
);
}
Expand Down
105 changes: 45 additions & 60 deletions src/main/java/ai/elimu/web/analytics/students/StudentController.java
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,21 @@ public String handleRequest(@PathVariable Long studentId, Model model) {
model.addAttribute("numeracySkills", NumeracySkill.values());


// Generate a list of weeks from 6 months ago until now
List<String> weekList = new ArrayList<>();
SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyy-ww");
Calendar calendar6MonthsAgo = Calendar.getInstance();
calendar6MonthsAgo.add(Calendar.MONTH, -6);
Calendar calendarNow = Calendar.getInstance();
Calendar week = calendar6MonthsAgo;
while (!week.after(calendarNow)) {
String weekAsString = simpleDateFormat.format(week.getTime());
weekList.add(weekAsString);
week.add(Calendar.WEEK_OF_YEAR, 1);
}
model.addAttribute("weekList", weekList);

Comment on lines +63 to +76
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Address SimpleDateFormat pattern and time zone issues.

The yyyy-ww pattern with SimpleDateFormat can be problematic for week-based analytics:

  1. Week numbering inconsistency: The "ww" pattern has locale-dependent behavior and can cause issues across year boundaries
  2. Time zone dependency: No explicit time zone is set, which may cause inconsistent behavior across different server environments

Consider using DateTimeFormatter with explicit time zone handling:

-    SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyy-ww");
+    DateTimeFormatter weekFormatter = DateTimeFormatter.ofPattern("yyyy-ww", Locale.US);
+    ZoneId serverTimeZone = ZoneId.systemDefault();

Or better yet, use java.time API for more reliable week calculations:

-    List<String> weekList = new ArrayList<>();
-    SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyy-ww");
-    Calendar calendar6MonthsAgo = Calendar.getInstance();
-    calendar6MonthsAgo.add(Calendar.MONTH, -6);
-    Calendar calendarNow = Calendar.getInstance();
-    Calendar week = calendar6MonthsAgo;
-    while (!week.after(calendarNow)) {
-      String weekAsString = simpleDateFormat.format(week.getTime());
-      weekList.add(weekAsString);
-      week.add(Calendar.WEEK_OF_YEAR, 1);
-    }
+    List<String> weekList = new ArrayList<>();
+    LocalDate sixMonthsAgo = LocalDate.now().minusMonths(6);
+    LocalDate now = LocalDate.now();
+    LocalDate current = sixMonthsAgo.with(DayOfWeek.MONDAY); // Start from Monday of the week
+    
+    while (!current.isAfter(now)) {
+      String weekAsString = current.format(DateTimeFormatter.ofPattern("yyyy-ww"));
+      weekList.add(weekAsString);
+      current = current.plusWeeks(1);
+    }
📝 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
// Generate a list of weeks from 6 months ago until now
List<String> weekList = new ArrayList<>();
SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyy-ww");
Calendar calendar6MonthsAgo = Calendar.getInstance();
calendar6MonthsAgo.add(Calendar.MONTH, -6);
Calendar calendarNow = Calendar.getInstance();
Calendar week = calendar6MonthsAgo;
while (!week.after(calendarNow)) {
String weekAsString = simpleDateFormat.format(week.getTime());
weekList.add(weekAsString);
week.add(Calendar.WEEK_OF_YEAR, 1);
}
model.addAttribute("weekList", weekList);
// Generate a list of weeks from 6 months ago until now
List<String> weekList = new ArrayList<>();
LocalDate sixMonthsAgo = LocalDate.now().minusMonths(6);
LocalDate now = LocalDate.now();
LocalDate current = sixMonthsAgo.with(DayOfWeek.MONDAY); // Start from Monday of the week
while (!current.isAfter(now)) {
String weekAsString = current.format(DateTimeFormatter.ofPattern("yyyy-ww"));
weekList.add(weekAsString);
current = current.plusWeeks(1);
}
model.addAttribute("weekList", weekList);
🤖 Prompt for AI Agents
In src/main/java/ai/elimu/web/analytics/students/StudentController.java around
lines 63 to 76, the use of SimpleDateFormat with the pattern "yyyy-ww" and no
explicit time zone causes potential inconsistencies in week numbering and time
zone handling. Replace SimpleDateFormat with java.time API classes such as
ZonedDateTime or LocalDate combined with WeekFields to generate the list of
weeks reliably. Use DateTimeFormatter with an explicit time zone (e.g., UTC) and
locale to ensure consistent week calculations across year boundaries and
different server environments.


List<LetterSoundAssessmentEvent> letterSoundAssessmentEvents = letterSoundAssessmentEventDao.readAll(student.getAndroidId());
model.addAttribute("letterSoundAssessmentEvents", letterSoundAssessmentEvents);

Expand All @@ -69,99 +84,69 @@ public String handleRequest(@PathVariable Long studentId, Model model) {

// Prepare chart data - WordLearningEvents
List<WordLearningEvent> wordLearningEvents = wordLearningEventDao.readAll(student.getAndroidId());
List<String> wordMonthList = new ArrayList<>();
List<Integer> wordEventCountList = new ArrayList<>();
if (!wordLearningEvents.isEmpty()) {
// Group event count by month (e.g. "Aug-2024", "Sep-2024")
Map<String, Integer> eventCountByMonthMap = new HashMap<>();
SimpleDateFormat simpleDateFormat = new SimpleDateFormat("MMM-yyyy");
// Group event count by week (e.g. "2024-09", "2024-26")
Map<String, Integer> eventCountByWeekMap = new HashMap<>();
for (WordLearningEvent event : wordLearningEvents) {
String eventMonth = simpleDateFormat.format(event.getTimestamp().getTime());
eventCountByMonthMap.put(eventMonth, eventCountByMonthMap.getOrDefault(eventMonth, 0) + 1);
String eventWeek = simpleDateFormat.format(event.getTimestamp().getTime());
eventCountByWeekMap.put(eventWeek, eventCountByWeekMap.getOrDefault(eventWeek, 0) + 1);
}

// Iterate each month from 4 years ago until now
Calendar calendar4YearsAgo = Calendar.getInstance();
calendar4YearsAgo.add(Calendar.YEAR, -4);
Calendar calendarNow = Calendar.getInstance();
Calendar month = calendar4YearsAgo;
while (!month.after(calendarNow)) {
String monthAsString = simpleDateFormat.format(month.getTime());
wordMonthList.add(monthAsString);

wordEventCountList.add(eventCountByMonthMap.getOrDefault(monthAsString, 0));

// Increase the date by 1 month
month.add(Calendar.MONTH, 1);
// Iterate each week from 6 months ago until now
week = calendar6MonthsAgo;
while (!week.after(calendarNow)) {
String weekAsString = simpleDateFormat.format(week.getTime());
wordEventCountList.add(eventCountByWeekMap.getOrDefault(weekAsString, 0));
week.add(Calendar.WEEK_OF_YEAR, 1);
}
Comment on lines +97 to 102
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix potential Calendar mutation issue.

The code reuses the same Calendar instances (calendar6MonthsAgo, calendarNow) across multiple iterations, but modifies the week variable directly from calendar6MonthsAgo. This can lead to unexpected behavior as Calendar objects are mutable.

Apply this fix to ensure Calendar independence:

-      // Iterate each week from 6 months ago until now
-      week = calendar6MonthsAgo;
-      while (!week.after(calendarNow)) {
-        String weekAsString = simpleDateFormat.format(week.getTime());
-        wordEventCountList.add(eventCountByWeekMap.getOrDefault(weekAsString, 0));
-        week.add(Calendar.WEEK_OF_YEAR, 1);
-      }
+      // Iterate each week from 6 months ago until now
+      week = (Calendar) calendar6MonthsAgo.clone();
+      while (!week.after(calendarNow)) {
+        String weekAsString = simpleDateFormat.format(week.getTime());
+        wordEventCountList.add(eventCountByWeekMap.getOrDefault(weekAsString, 0));
+        week.add(Calendar.WEEK_OF_YEAR, 1);
+      }

The same fix should be applied to lines 120-125 and 143-148 for the other event types.

📝 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
week = calendar6MonthsAgo;
while (!week.after(calendarNow)) {
String weekAsString = simpleDateFormat.format(week.getTime());
wordEventCountList.add(eventCountByWeekMap.getOrDefault(weekAsString, 0));
week.add(Calendar.WEEK_OF_YEAR, 1);
}
// Iterate each week from 6 months ago until now
week = (Calendar) calendar6MonthsAgo.clone();
while (!week.after(calendarNow)) {
String weekAsString = simpleDateFormat.format(week.getTime());
wordEventCountList.add(eventCountByWeekMap.getOrDefault(weekAsString, 0));
week.add(Calendar.WEEK_OF_YEAR, 1);
}
🤖 Prompt for AI Agents
In src/main/java/ai/elimu/web/analytics/students/StudentController.java around
lines 97 to 102, the code assigns the mutable Calendar instance
calendar6MonthsAgo directly to the variable week and then modifies week, which
unintentionally mutates calendar6MonthsAgo. To fix this, create a new Calendar
instance by cloning calendar6MonthsAgo before the loop to ensure week is
independent and modifications do not affect the original calendar. Apply the
same cloning approach to the similar loops at lines 120-125 and 143-148 for
other event types.

}
model.addAttribute("wordMonthList", wordMonthList);
model.addAttribute("wordEventCountList", wordEventCountList);
model.addAttribute("wordLearningEvents", wordLearningEvents);


// Prepare chart data - StoryBookLearningEvents
List<StoryBookLearningEvent> storyBookLearningEvents = storyBookLearningEventDao.readAll(student.getAndroidId());
List<String> storyBookMonthList = new ArrayList<>();
List<Integer> storyBookEventCountList = new ArrayList<>();
if (!storyBookLearningEvents.isEmpty()) {
// Group event count by month (e.g. "Aug-2024", "Sep-2024")
Map<String, Integer> eventCountByMonthMap = new HashMap<>();
SimpleDateFormat simpleDateFormat = new SimpleDateFormat("MMM-yyyy");
// Group event count by week (e.g. "2024-09", "2024-26")
Map<String, Integer> eventCountByWeekMap = new HashMap<>();
for (StoryBookLearningEvent event : storyBookLearningEvents) {
String eventMonth = simpleDateFormat.format(event.getTimestamp().getTime());
eventCountByMonthMap.put(eventMonth, eventCountByMonthMap.getOrDefault(eventMonth, 0) + 1);
String eventWeek = simpleDateFormat.format(event.getTimestamp().getTime());
eventCountByWeekMap.put(eventWeek, eventCountByWeekMap.getOrDefault(eventWeek, 0) + 1);
}

// Iterate each month from 4 years ago until now
Calendar calendar4YearsAgo = Calendar.getInstance();
calendar4YearsAgo.add(Calendar.YEAR, -4);
Calendar calendarNow = Calendar.getInstance();
Calendar month = calendar4YearsAgo;
while (!month.after(calendarNow)) {
String monthAsString = simpleDateFormat.format(month.getTime());
storyBookMonthList.add(monthAsString);

storyBookEventCountList.add(eventCountByMonthMap.getOrDefault(monthAsString, 0));

// Increase the date by 1 month
month.add(Calendar.MONTH, 1);
// Iterate each week from 6 months ago until now
week = calendar6MonthsAgo;
while (!week.after(calendarNow)) {
String weekAsString = simpleDateFormat.format(week.getTime());
storyBookEventCountList.add(eventCountByWeekMap.getOrDefault(weekAsString, 0));
week.add(Calendar.WEEK_OF_YEAR, 1);
}
}
model.addAttribute("storyBookMonthList", storyBookMonthList);
model.addAttribute("storyBookEventCountList", storyBookEventCountList);
model.addAttribute("storyBookLearningEvents", storyBookLearningEvents);


// Prepare chart data - VideoLearningEvents
List<VideoLearningEvent> videoLearningEvents = videoLearningEventDao.readAll(student.getAndroidId());
List<String> videoMonthList = new ArrayList<>();
List<Integer> videoEventCountList = new ArrayList<>();
if (!videoLearningEvents.isEmpty()) {
// Group event count by month (e.g. "Aug-2024", "Sep-2024")
Map<String, Integer> eventCountByMonthMap = new HashMap<>();
SimpleDateFormat simpleDateFormat = new SimpleDateFormat("MMM-yyyy");
// Group event count by week (e.g. "2024-09", "2024-26")
Map<String, Integer> eventCountByWeekMap = new HashMap<>();
for (VideoLearningEvent event : videoLearningEvents) {
String eventMonth = simpleDateFormat.format(event.getTimestamp().getTime());
eventCountByMonthMap.put(eventMonth, eventCountByMonthMap.getOrDefault(eventMonth, 0) + 1);
String eventWeek = simpleDateFormat.format(event.getTimestamp().getTime());
eventCountByWeekMap.put(eventWeek, eventCountByWeekMap.getOrDefault(eventWeek, 0) + 1);
}

// Iterate each month from 4 years ago until now
Calendar calendar4YearsAgo = Calendar.getInstance();
calendar4YearsAgo.add(Calendar.YEAR, -4);
Calendar calendarNow = Calendar.getInstance();
Calendar month = calendar4YearsAgo;
while (!month.after(calendarNow)) {
String monthAsString = simpleDateFormat.format(month.getTime());
videoMonthList.add(monthAsString);

videoEventCountList.add(eventCountByMonthMap.getOrDefault(monthAsString, 0));

// Increase the date by 1 month
month.add(Calendar.MONTH, 1);
// Iterate each week from 6 months ago until now
week = calendar6MonthsAgo;
while (!week.after(calendarNow)) {
String weekAsString = simpleDateFormat.format(week.getTime());
videoEventCountList.add(eventCountByWeekMap.getOrDefault(weekAsString, 0));
week.add(Calendar.WEEK_OF_YEAR, 1);
}
Comment on lines +89 to 148
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Eliminate code duplication and improve maintainability.

The same event processing logic is repeated three times for different event types, violating the DRY principle. This makes the code harder to maintain and increases the risk of inconsistencies.

Extract the common logic into a reusable method:

+  private <T> List<Integer> processLearningEvents(List<T> events, 
+                                                   Function<T, Calendar> timestampExtractor,
+                                                   SimpleDateFormat dateFormat,
+                                                   Calendar startCalendar,
+                                                   Calendar endCalendar) {
+    List<Integer> eventCountList = new ArrayList<>();
+    if (!events.isEmpty()) {
+      Map<String, Integer> eventCountByWeekMap = new HashMap<>();
+      for (T event : events) {
+        String eventWeek = dateFormat.format(timestampExtractor.apply(event).getTime());
+        eventCountByWeekMap.put(eventWeek, eventCountByWeekMap.getOrDefault(eventWeek, 0) + 1);
+      }
+      
+      Calendar week = (Calendar) startCalendar.clone();
+      while (!week.after(endCalendar)) {
+        String weekAsString = dateFormat.format(week.getTime());
+        eventCountList.add(eventCountByWeekMap.getOrDefault(weekAsString, 0));
+        week.add(Calendar.WEEK_OF_YEAR, 1);
+      }
+    }
+    return eventCountList;
+  }

Then replace the repeated code blocks:

-    // Prepare chart data - WordLearningEvents
-    List<WordLearningEvent> wordLearningEvents = wordLearningEventDao.readAll(student.getAndroidId());
-    List<Integer> wordEventCountList = new ArrayList<>();
-    if (!wordLearningEvents.isEmpty()) {
-      // Group event count by week (e.g. "2024-09", "2024-26")
-      Map<String, Integer> eventCountByWeekMap = new HashMap<>();
-      for (WordLearningEvent event : wordLearningEvents) {
-        String eventWeek = simpleDateFormat.format(event.getTimestamp().getTime());
-        eventCountByWeekMap.put(eventWeek, eventCountByWeekMap.getOrDefault(eventWeek, 0) + 1);
-      }
-
-      // Iterate each week from 6 months ago until now
-      week = calendar6MonthsAgo;
-      while (!week.after(calendarNow)) {
-        String weekAsString = simpleDateFormat.format(week.getTime());
-        wordEventCountList.add(eventCountByWeekMap.getOrDefault(weekAsString, 0));
-        week.add(Calendar.WEEK_OF_YEAR, 1);
-      }
-    }
+    List<WordLearningEvent> wordLearningEvents = wordLearningEventDao.readAll(student.getAndroidId());
+    List<Integer> wordEventCountList = processLearningEvents(
+      wordLearningEvents, 
+      WordLearningEvent::getTimestamp,
+      simpleDateFormat,
+      calendar6MonthsAgo,
+      calendarNow
+    );
🤖 Prompt for AI Agents
In src/main/java/ai/elimu/web/analytics/students/StudentController.java between
lines 89 and 148, the code for grouping event counts by week and iterating over
weeks is duplicated for WordLearningEvent, StoryBookLearningEvent, and
VideoLearningEvent. To fix this, extract the common logic into a reusable method
that accepts a list of events and returns the list of weekly event counts.
Replace the repeated code blocks by calling this new method with the respective
event lists, thereby eliminating duplication and improving maintainability.

}
model.addAttribute("videoMonthList", videoMonthList);
model.addAttribute("videoEventCountList", videoEventCountList);
model.addAttribute("videoLearningEvents", videoLearningEvents);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package ai.elimu.web.analytics.students;

import ai.elimu.dao.StoryBookLearningEventDao;
import ai.elimu.dao.StudentDao;
import ai.elimu.dao.WordLearningEventDao;
import ai.elimu.entity.analytics.students.Student;
import ai.elimu.util.AnalyticsHelper;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;

import java.util.List;

import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;

@Controller
@RequestMapping("/analytics/students")
@RequiredArgsConstructor
@Slf4j
public class StudentListController {

private final StudentDao studentDao;

@GetMapping
public String handleRequest(Model model) {
log.info("handleRequest");

List<Student> students = studentDao.readAll();
log.info("students.size(): " + students.size());
for (Student student : students) {
student.setAndroidId(AnalyticsHelper.redactAndroidId(student.getAndroidId()));
}
model.addAttribute("students", students);

return "analytics/students/list";
}
}
Loading
Loading