Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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.15-SNAPSHOT
ai.elimu:webapp:war:2.6.16-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
@@ -0,0 +1,8 @@
package ai.elimu.dao;

import ai.elimu.entity.analytics.LetterSoundAssessmentEvent;

public interface LetterSoundAssessmentEventDao extends GenericDao<LetterSoundAssessmentEvent> {


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package ai.elimu.dao.jpa;

import ai.elimu.dao.LetterSoundAssessmentEventDao;
import ai.elimu.entity.analytics.LetterSoundAssessmentEvent;

public class LetterSoundAssessmentEventDaoJpa extends GenericDaoJpa<LetterSoundAssessmentEvent> implements LetterSoundAssessmentEventDao {


}
Comment on lines +6 to +22
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add @repository annotation for Spring dependency injection.

The DAO implementation class is missing the @Repository annotation, which is required for Spring to properly manage this bean and enable dependency injection.

Apply this diff to add the necessary annotation:

+import org.springframework.stereotype.Repository;
+
+@Repository
 public class LetterSoundAssessmentEventDaoJpa extends GenericDaoJpa<LetterSoundAssessmentEvent> implements LetterSoundAssessmentEventDao {
 
-    
 }
📝 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 class LetterSoundAssessmentEventDaoJpa extends GenericDaoJpa<LetterSoundAssessmentEvent> implements LetterSoundAssessmentEventDao {
}
import org.springframework.stereotype.Repository;
@Repository
public class LetterSoundAssessmentEventDaoJpa extends GenericDaoJpa<LetterSoundAssessmentEvent> implements LetterSoundAssessmentEventDao {
}
🤖 Prompt for AI Agents
In src/main/java/ai/elimu/dao/jpa/LetterSoundAssessmentEventDaoJpa.java on lines
6 to 9, the class lacks the @Repository annotation, which is necessary for
Spring to recognize it as a bean for dependency injection. Add the @Repository
annotation above the class declaration to enable proper Spring management and
injection of this DAO component.

54 changes: 54 additions & 0 deletions src/main/java/ai/elimu/entity/analytics/AssessmentEvent.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package ai.elimu.entity.analytics;

import ai.elimu.entity.BaseEntity;
import ai.elimu.entity.application.Application;
import jakarta.persistence.Column;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.MappedSuperclass;
import jakarta.persistence.Temporal;
import jakarta.persistence.TemporalType;
import jakarta.validation.constraints.NotNull;
import java.util.Calendar;
import lombok.Getter;
import lombok.Setter;

@Getter
@Setter
@MappedSuperclass
public abstract class AssessmentEvent extends BaseEntity {

@NotNull
@Temporal(TemporalType.TIMESTAMP)
private Calendar timestamp;

/**
* See https://developer.android.com/reference/android/provider/Settings.Secure#ANDROID_ID
*/
@NotNull
private String androidId;

/**
* The package name of the {@link #application} where the assessment event occurred.
* E.g. <code>ai.elimu.soundcards</code>.
*/
@NotNull
private String packageName;

/**
* This field will only be populated if a corresponding {@link Application} can be
* found in the database for the {@link #packageName}.
*/
@ManyToOne
private Application application;

/**
* Any additional data should be stored in the format of a JSON object.
*
* Example:
* <pre>
* {'word_ids_presented': [1,2,3], 'word_id_selected': 2}
* </pre>
*/
@Column(length = 1024)
private String additionalData;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package ai.elimu.entity.analytics;

import jakarta.persistence.Entity;
import lombok.Getter;
import lombok.Setter;

@Getter
@Setter
@Entity
public class LetterSoundAssessmentEvent extends AssessmentEvent {

/**
* The sequence of letters. E.g. <code>["s", "h"]</code>.
*/
private String[] letterSoundLetters;

/**
* The sequence of sounds (IPA values). E.g. <code>["ʃ"]</code>.
*/
private String [] letterSoundSounds;

/**
* This field might not be included, e.g. if the assessment task was done in a
* 3rd-party app that did not load the content from the elimu.ai Content Provider.
* In this case, the {@link #letterSoundId} will be {@code null}.
*/
private Long letterSoundId;

/**
* A value in the range [0.0, 1.0].
*/
private Float masteryScore;
Comment on lines +29 to +32
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation constraint for mastery score range.

The JavaDoc specifies a range [0.0, 1.0] but there's no validation to enforce this constraint.

+import jakarta.validation.constraints.DecimalMax;
+import jakarta.validation.constraints.DecimalMin;

 /**
  * A value in the range [0.0, 1.0].
  */
+@DecimalMin(value = "0.0", message = "Mastery score must be between 0.0 and 1.0")
+@DecimalMax(value = "1.0", message = "Mastery score must be between 0.0 and 1.0")
 private Float masteryScore;
🤖 Prompt for AI Agents
In src/main/java/ai/elimu/entity/analytics/LetterSoundAssessmentEvent.java
around lines 29 to 32, the masteryScore field has a JavaDoc specifying it should
be in the range [0.0, 1.0], but no validation enforces this. Add a validation
annotation such as @DecimalMin("0.0") and @DecimalMax("1.0") to the masteryScore
field to ensure the value stays within the specified range.


/**
* The number of milliseconds passed between the student opening the assessment task
* and submitting a response. E.g. <code>15000</code>.
*/
private Long timeSpentMs;
Comment on lines +34 to +38
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for time measurement.

Consider adding validation to ensure timeSpentMs is non-negative, as negative time values wouldn't make sense for assessment duration.

+import jakarta.validation.constraints.Min;

 /**
  * The number of milliseconds passed between the student opening the assessment task 
  * and submitting a response. E.g. <code>15000</code>.
  */
+@Min(value = 0, message = "Time spent must be non-negative")
 private Long timeSpentMs;

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/main/java/ai/elimu/entity/analytics/LetterSoundAssessmentEvent.java
around lines 34 to 38, add validation to ensure the timeSpentMs field is never
set to a negative value. Implement a check in the setter method or wherever
timeSpentMs is assigned to throw an exception or reject values less than zero,
enforcing that the assessment duration is always non-negative.

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

import ai.elimu.dao.LetterSoundAssessmentEventDao;
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.LetterSoundAssessmentEvent;
import ai.elimu.entity.analytics.StoryBookLearningEvent;
import ai.elimu.entity.analytics.VideoLearningEvent;
import ai.elimu.entity.analytics.WordLearningEvent;
Expand Down Expand Up @@ -33,6 +35,8 @@ public class StudentController {

private final StudentDao studentDao;

private final LetterSoundAssessmentEventDao letterSoundAssessmentEventDao;

private final WordLearningEventDao wordLearningEventDao;

private final StoryBookLearningEventDao storyBookLearningEventDao;
Expand All @@ -46,6 +50,10 @@ public String handleRequest(@PathVariable Long studentId, Model model) {
Student student = studentDao.read(studentId);
log.info("student.getAndroidId(): " + student.getAndroidId());


List<LetterSoundAssessmentEvent> letterSoundAssessmentEvents = letterSoundAssessmentEventDao.readAll();
model.addAttribute("letterSoundAssessmentEvents", letterSoundAssessmentEvents);
Comment on lines +63 to +64
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Filter events by student to maintain consistency with controller purpose.

The current implementation fetches all letter sound assessment events without filtering by the specific student. This is inconsistent with the controller's purpose (handling requests for a specific student) and differs from how the student data is retrieved.

Apply this diff to filter events by the student's Android ID:

-List<LetterSoundAssessmentEvent> letterSoundAssessmentEvents = letterSoundAssessmentEventDao.readAll();
+List<LetterSoundAssessmentEvent> letterSoundAssessmentEvents = letterSoundAssessmentEventDao.readAllByAndroidId(student.getAndroidId());

Note: This assumes the DAO has or will implement a readAllByAndroidId method. If not available, you'll need to add this method to the DAO interface and implementation.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/main/java/ai/elimu/web/analytics/students/StudentController.java around
lines 54-55, the code retrieves all LetterSoundAssessmentEvent objects without
filtering by student, which is inconsistent with the controller's purpose. To
fix this, modify the code to fetch only events associated with the specific
student's Android ID by calling a method like readAllByAndroidId, assuming it
exists or add this method to the DAO. This ensures the data displayed is
relevant to the current student context.


Comment on lines +62 to +65
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add chart data preparation for consistency with other event types.

The letter sound assessment events are added to the model without any chart data preparation, unlike the other event types (Word, StoryBook, Video) which all have extensive chart preparation logic. This inconsistency suggests the implementation is incomplete.

Consider adding chart data preparation similar to other event types:

+// Prepare chart data - LetterSoundAssessmentEvents
 List<LetterSoundAssessmentEvent> letterSoundAssessmentEvents = letterSoundAssessmentEventDao.readAll();
+List<String> letterSoundMonthList = new ArrayList<>();
+List<Integer> letterSoundEventCountList = new ArrayList<>();
+if (!letterSoundAssessmentEvents.isEmpty()) {
+    // Group event count by month (e.g. "Aug-2024", "Sep-2024")
+    Map<String, Integer> eventCountByMonthMap = new HashMap<>();
+    SimpleDateFormat simpleDateFormat = new SimpleDateFormat("MMM-yyyy");
+    for (LetterSoundAssessmentEvent event : letterSoundAssessmentEvents) {
+        String eventMonth = simpleDateFormat.format(event.getTimestamp().getTime());
+        eventCountByMonthMap.put(eventMonth, eventCountByMonthMap.getOrDefault(eventMonth, 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());
+        letterSoundMonthList.add(monthAsString);
+
+        letterSoundEventCountList.add(eventCountByMonthMap.getOrDefault(monthAsString, 0));
+
+        // Increase the date by 1 month
+        month.add(Calendar.MONTH, 1);
+    }
+}
+model.addAttribute("letterSoundMonthList", letterSoundMonthList);
+model.addAttribute("letterSoundEventCountList", letterSoundEventCountList);
 model.addAttribute("letterSoundAssessmentEvents", letterSoundAssessmentEvents);
📝 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
List<LetterSoundAssessmentEvent> letterSoundAssessmentEvents = letterSoundAssessmentEventDao.readAll();
model.addAttribute("letterSoundAssessmentEvents", letterSoundAssessmentEvents);
// Prepare chart data - LetterSoundAssessmentEvents
List<LetterSoundAssessmentEvent> letterSoundAssessmentEvents = letterSoundAssessmentEventDao.readAll();
List<String> letterSoundMonthList = new ArrayList<>();
List<Integer> letterSoundEventCountList = new ArrayList<>();
if (!letterSoundAssessmentEvents.isEmpty()) {
// Group event count by month (e.g. "Aug-2024", "Sep-2024")
Map<String, Integer> eventCountByMonthMap = new HashMap<>();
SimpleDateFormat simpleDateFormat = new SimpleDateFormat("MMM-yyyy");
for (LetterSoundAssessmentEvent event : letterSoundAssessmentEvents) {
String eventMonth = simpleDateFormat.format(event.getTimestamp().getTime());
eventCountByMonthMap.put(eventMonth, eventCountByMonthMap.getOrDefault(eventMonth, 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());
letterSoundMonthList.add(monthAsString);
letterSoundEventCountList.add(eventCountByMonthMap.getOrDefault(monthAsString, 0));
month.add(Calendar.MONTH, 1);
}
}
model.addAttribute("letterSoundMonthList", letterSoundMonthList);
model.addAttribute("letterSoundEventCountList", letterSoundEventCountList);
model.addAttribute("letterSoundAssessmentEvents", letterSoundAssessmentEvents);
🤖 Prompt for AI Agents
In src/main/java/ai/elimu/web/analytics/students/StudentController.java around
lines 53 to 56, the code retrieves all LetterSoundAssessmentEvent objects and
adds them to the model without generating chart data, unlike other event types.
To fix this, implement chart data preparation for letter sound assessment events
similar to other event types, ensuring consistency across the model attributes
and complete analytics visualization.


// Prepare chart data - WordLearningEvents
List<WordLearningEvent> wordLearningEvents = wordLearningEventDao.readAll();
Expand Down
22 changes: 22 additions & 0 deletions src/main/resources/META-INF/jpa-schema-export.sql
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@

drop table if exists LetterSound_Sound;

drop table if exists LetterSoundAssessmentEvent;

drop table if exists LetterSoundContributionEvent;

drop table if exists LetterSoundLearningEvent;
Expand Down Expand Up @@ -304,6 +306,21 @@
primary key (LetterSound_id, sounds_ORDER)
) type=MyISAM;

create table LetterSoundAssessmentEvent (
id bigint not null auto_increment,
additionalData text,
androidId varchar(255),
packageName varchar(255),
timestamp datetime,
letterSoundId bigint,
letterSoundLetters varbinary(255),
letterSoundSounds varbinary(255),
masteryScore float(23),
timeSpentMs bigint,
application_id bigint,
primary key (id)
) type=MyISAM;
Comment thread
coderabbitai[bot] marked this conversation as resolved.

create table LetterSoundContributionEvent (
id bigint not null auto_increment,
comment text,
Expand Down Expand Up @@ -755,6 +772,11 @@
foreign key (LetterSound_id)
references LetterSound (id);

alter table LetterSoundAssessmentEvent
add constraint FKehf1rjbixnmdjol91didaj4b5
foreign key (application_id)
references Application (id);

alter table LetterSoundContributionEvent
add constraint FK5uk320agfa13pvh52v6n6ncbs
foreign key (contributor_id)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
<bean id="imageContributionEventDao" class="ai.elimu.dao.jpa.ImageContributionEventDaoJpa" />
<bean id="letterDao" class="ai.elimu.dao.jpa.LetterDaoJpa" />
<bean id="letterContributionEventDao" class="ai.elimu.dao.jpa.LetterContributionEventDaoJpa" />
<bean id="letterSoundAssessmentEventDao" class="ai.elimu.dao.jpa.LetterSoundAssessmentEventDaoJpa" />
<bean id="letterSoundContributionEventDao" class="ai.elimu.dao.jpa.LetterSoundContributionEventDaoJpa" />
<bean id="letterSoundDao" class="ai.elimu.dao.jpa.LetterSoundDaoJpa" />
<bean id="letterSoundLearningEventDao" class="ai.elimu.dao.jpa.LetterSoundLearningEventDaoJpa" />
Expand Down