-
-
Notifications
You must be signed in to change notification settings - Fork 70
feat: letter-sound assessment event #2195
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 6 commits
93f04d2
6a53986
27de084
bfd07fa
19e2d46
7a9c508
c30663a
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 |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| package ai.elimu.dao; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import org.springframework.dao.DataAccessException; | ||
|
|
||
| import ai.elimu.entity.analytics.LetterSoundAssessmentEvent; | ||
|
|
||
| public interface LetterSoundAssessmentEventDao extends GenericDao<LetterSoundAssessmentEvent> { | ||
|
|
||
| List<LetterSoundAssessmentEvent> readAll(String androidId) throws DataAccessException; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package ai.elimu.dao.jpa; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import org.springframework.dao.DataAccessException; | ||
|
|
||
| import ai.elimu.dao.LetterSoundAssessmentEventDao; | ||
| import ai.elimu.entity.analytics.LetterSoundAssessmentEvent; | ||
|
|
||
| public class LetterSoundAssessmentEventDaoJpa extends GenericDaoJpa<LetterSoundAssessmentEvent> implements LetterSoundAssessmentEventDao { | ||
|
|
||
| @Override | ||
| public List<LetterSoundAssessmentEvent> readAll(String androidId) throws DataAccessException { | ||
| return em.createQuery( | ||
| "SELECT event " + | ||
| "FROM LetterSoundAssessmentEvent event " + | ||
| "WHERE event.androidId = :androidId " + | ||
| "ORDER BY event.timestamp") | ||
| .setParameter("androidId", androidId) | ||
| .getResultList(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| 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; | ||
|
|
||
| /** | ||
| * A 64-bit number (expressed as a hexadecimal string), unique to each combination of | ||
| * app-signing key, user, and device. | ||
| * | ||
| * 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>"sh"</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; | ||
|
|
||
| /** | ||
| * 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
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 Add validation for time measurement. Consider adding validation to ensure +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;
🤖 Prompt for AI Agents |
||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,93 @@ | ||||||||||||||||||||||||||||||||||||
| package ai.elimu.web.analytics.students; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| import ai.elimu.dao.LetterSoundAssessmentEventDao; | ||||||||||||||||||||||||||||||||||||
| import ai.elimu.dao.StudentDao; | ||||||||||||||||||||||||||||||||||||
| import ai.elimu.entity.analytics.LetterSoundAssessmentEvent; | ||||||||||||||||||||||||||||||||||||
| import ai.elimu.entity.analytics.students.Student; | ||||||||||||||||||||||||||||||||||||
| import jakarta.servlet.http.HttpServletResponse; | ||||||||||||||||||||||||||||||||||||
| import java.io.IOException; | ||||||||||||||||||||||||||||||||||||
| import java.io.OutputStream; | ||||||||||||||||||||||||||||||||||||
| import java.io.StringWriter; | ||||||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||||
| import lombok.extern.slf4j.Slf4j; | ||||||||||||||||||||||||||||||||||||
| import org.apache.commons.csv.CSVFormat; | ||||||||||||||||||||||||||||||||||||
| 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/students/{studentId}/letter-sound-assessment-events.csv") | ||||||||||||||||||||||||||||||||||||
| @RequiredArgsConstructor | ||||||||||||||||||||||||||||||||||||
| @Slf4j | ||||||||||||||||||||||||||||||||||||
| public class LetterSoundAssessmentEventCsvExportController { | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| private final StudentDao studentDao; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| private final LetterSoundAssessmentEventDao letterSoundAssessmentEventDao; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @GetMapping | ||||||||||||||||||||||||||||||||||||
| public void handleRequest( | ||||||||||||||||||||||||||||||||||||
| @PathVariable Long studentId, | ||||||||||||||||||||||||||||||||||||
| HttpServletResponse response, | ||||||||||||||||||||||||||||||||||||
| OutputStream outputStream | ||||||||||||||||||||||||||||||||||||
| ) throws IOException { | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+32
to
+36
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 Remove redundant OutputStream parameter. The @GetMapping
public void handleRequest(
@PathVariable Long studentId,
- HttpServletResponse response,
- OutputStream outputStream
+ HttpServletResponse response
) throws IOException {Then update the output writing section to use 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| log.info("handleRequest"); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Student student = studentDao.read(studentId); | ||||||||||||||||||||||||||||||||||||
| log.info("student.getAndroidId(): " + student.getAndroidId()); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| List<LetterSoundAssessmentEvent> letterSoundAssessmentEvents = letterSoundAssessmentEventDao.readAll(student.getAndroidId()); | ||||||||||||||||||||||||||||||||||||
| log.info("letterSoundAssessmentEvents.size(): " + letterSoundAssessmentEvents.size()); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+39
to
+43
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 safety checks for student retrieval. The code doesn't handle the case where the student might not exist, which could result in a Student student = studentDao.read(studentId);
+ if (student == null) {
+ response.sendError(HttpServletResponse.SC_NOT_FOUND, "Student not found");
+ return;
+ }
log.info("student.getAndroidId(): " + student.getAndroidId());📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| CSVFormat csvFormat = CSVFormat.DEFAULT.builder() | ||||||||||||||||||||||||||||||||||||
| .setHeader( | ||||||||||||||||||||||||||||||||||||
| "id", | ||||||||||||||||||||||||||||||||||||
| "timestamp", | ||||||||||||||||||||||||||||||||||||
| "package_name", | ||||||||||||||||||||||||||||||||||||
| "letter_sound_letters", | ||||||||||||||||||||||||||||||||||||
| "letter_sound_sounds", | ||||||||||||||||||||||||||||||||||||
| "letter_sound_id", | ||||||||||||||||||||||||||||||||||||
| "mastery_score", | ||||||||||||||||||||||||||||||||||||
| "time_spent_ms", | ||||||||||||||||||||||||||||||||||||
| "additional_data" | ||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||
| .build(); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| StringWriter stringWriter = new StringWriter(); | ||||||||||||||||||||||||||||||||||||
| CSVPrinter csvPrinter = new CSVPrinter(stringWriter, csvFormat); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| for (LetterSoundAssessmentEvent letterSoundAssessmentEvent : letterSoundAssessmentEvents) { | ||||||||||||||||||||||||||||||||||||
| log.info("letterSoundAssessmentEvent.getId(): " + letterSoundAssessmentEvent.getId()); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| csvPrinter.printRecord( | ||||||||||||||||||||||||||||||||||||
| letterSoundAssessmentEvent.getId(), | ||||||||||||||||||||||||||||||||||||
| letterSoundAssessmentEvent.getTimestamp().getTimeInMillis(), | ||||||||||||||||||||||||||||||||||||
| letterSoundAssessmentEvent.getPackageName(), | ||||||||||||||||||||||||||||||||||||
| letterSoundAssessmentEvent.getLetterSoundLetters(), | ||||||||||||||||||||||||||||||||||||
| letterSoundAssessmentEvent.getLetterSoundSounds(), | ||||||||||||||||||||||||||||||||||||
| letterSoundAssessmentEvent.getLetterSoundId(), | ||||||||||||||||||||||||||||||||||||
| letterSoundAssessmentEvent.getMasteryScore(), | ||||||||||||||||||||||||||||||||||||
| letterSoundAssessmentEvent.getTimeSpentMs(), | ||||||||||||||||||||||||||||||||||||
| letterSoundAssessmentEvent.getAdditionalData() | ||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| csvPrinter.flush(); | ||||||||||||||||||||||||||||||||||||
| csvPrinter.close(); | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+59
to
+78
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 Implement proper resource management using try-with-resources. The current implementation manually closes the - StringWriter stringWriter = new StringWriter();
- CSVPrinter csvPrinter = new CSVPrinter(stringWriter, csvFormat);
-
- for (LetterSoundAssessmentEvent letterSoundAssessmentEvent : letterSoundAssessmentEvents) {
- log.info("letterSoundAssessmentEvent.getId(): " + letterSoundAssessmentEvent.getId());
-
- csvPrinter.printRecord(
- letterSoundAssessmentEvent.getId(),
- letterSoundAssessmentEvent.getTimestamp().getTimeInMillis(),
- letterSoundAssessmentEvent.getPackageName(),
- letterSoundAssessmentEvent.getLetterSoundLetters(),
- letterSoundAssessmentEvent.getLetterSoundSounds(),
- letterSoundAssessmentEvent.getLetterSoundId(),
- letterSoundAssessmentEvent.getMasteryScore(),
- letterSoundAssessmentEvent.getTimeSpentMs(),
- letterSoundAssessmentEvent.getAdditionalData()
- );
- }
- csvPrinter.flush();
- csvPrinter.close();
+ try (StringWriter stringWriter = new StringWriter();
+ CSVPrinter csvPrinter = new CSVPrinter(stringWriter, csvFormat)) {
+
+ for (LetterSoundAssessmentEvent letterSoundAssessmentEvent : letterSoundAssessmentEvents) {
+ log.info("letterSoundAssessmentEvent.getId(): " + letterSoundAssessmentEvent.getId());
+
+ csvPrinter.printRecord(
+ letterSoundAssessmentEvent.getId(),
+ letterSoundAssessmentEvent.getTimestamp().getTimeInMillis(),
+ letterSoundAssessmentEvent.getPackageName(),
+ letterSoundAssessmentEvent.getLetterSoundLetters(),
+ letterSoundAssessmentEvent.getLetterSoundSounds(),
+ letterSoundAssessmentEvent.getLetterSoundId(),
+ letterSoundAssessmentEvent.getMasteryScore(),
+ letterSoundAssessmentEvent.getTimeSpentMs(),
+ letterSoundAssessmentEvent.getAdditionalData()
+ );
+ }
+ csvPrinter.flush();
+
+ String csvFileContent = stringWriter.toString();
+ // Move response writing logic here
+ }🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| String csvFileContent = stringWriter.toString(); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| response.setContentType("text/csv"); | ||||||||||||||||||||||||||||||||||||
| byte[] bytes = csvFileContent.getBytes(); | ||||||||||||||||||||||||||||||||||||
| response.setContentLength(bytes.length); | ||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||
| outputStream.write(bytes); | ||||||||||||||||||||||||||||||||||||
| outputStream.flush(); | ||||||||||||||||||||||||||||||||||||
| outputStream.close(); | ||||||||||||||||||||||||||||||||||||
| } catch (IOException ex) { | ||||||||||||||||||||||||||||||||||||
| log.error(ex.getMessage()); | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+85
to
+91
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 Improve error handling and use response.getOutputStream(). The current error handling catches response.setContentType("text/csv");
byte[] bytes = csvFileContent.getBytes();
response.setContentLength(bytes.length);
- try {
- outputStream.write(bytes);
- outputStream.flush();
- outputStream.close();
- } catch (IOException ex) {
- log.error(ex.getMessage());
- }
+ try (OutputStream outputStream = response.getOutputStream()) {
+ outputStream.write(bytes);
+ outputStream.flush();
+ } catch (IOException ex) {
+ log.error("Failed to write CSV data to response", ex);
+ response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "Failed to export CSV");
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,13 +1,19 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package ai.elimu.web.analytics.students; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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.LetterSoundAssessmentEvent; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ai.elimu.entity.analytics.LetterSoundLearningEvent; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ai.elimu.entity.analytics.StoryBookLearningEvent; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ai.elimu.entity.analytics.VideoLearningEvent; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ai.elimu.entity.analytics.WordLearningEvent; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ai.elimu.entity.analytics.students.Student; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ai.elimu.model.v2.enums.content.LiteracySkill; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ai.elimu.model.v2.enums.content.NumeracySkill; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import ai.elimu.util.AnalyticsHelper; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import lombok.RequiredArgsConstructor; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import lombok.extern.slf4j.Slf4j; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -33,6 +39,9 @@ public class StudentController { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final StudentDao studentDao; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final LetterSoundAssessmentEventDao letterSoundAssessmentEventDao; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final LetterSoundLearningEventDao letterSoundLearningEventDao; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final WordLearningEventDao wordLearningEventDao; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private final StoryBookLearningEventDao storyBookLearningEventDao; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -46,6 +55,17 @@ public String handleRequest(@PathVariable Long studentId, Model model) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Student student = studentDao.read(studentId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.info("student.getAndroidId(): " + student.getAndroidId()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| model.addAttribute("literacySkills", LiteracySkill.values()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| model.addAttribute("numeracySkills", NumeracySkill.values()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<LetterSoundAssessmentEvent> letterSoundAssessmentEvents = letterSoundAssessmentEventDao.readAll(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| model.addAttribute("letterSoundAssessmentEvents", letterSoundAssessmentEvents); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+63
to
+64
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. 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
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+62
to
+65
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 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<LetterSoundLearningEvent> letterSoundLearningEvents = letterSoundLearningEventDao.readAll(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| model.addAttribute("letterSoundLearningEvents", letterSoundLearningEvents); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Prepare chart data - WordLearningEvents | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<WordLearningEvent> wordLearningEvents = wordLearningEventDao.readAll(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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
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.
🤖 Prompt for AI Agents