Skip to content
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

feat(entity): video learning event #1911

Merged
merged 2 commits into from
Oct 9, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public abstract class LearningEvent extends BaseEntity {
private String androidId;

/**
* The package name of the {@link #application}.
* The package name of the {@link #application} where the learning event occurred.
*/
@NotNull
private String packageName;
Expand Down
52 changes: 52 additions & 0 deletions src/main/java/ai/elimu/model/analytics/VideoLearningEvent.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package ai.elimu.model.analytics;

import javax.persistence.Entity;
import javax.persistence.ManyToOne;
import javax.validation.constraints.NotNull;

import ai.elimu.model.content.multimedia.Video;

@Entity
public class VideoLearningEvent extends LearningEvent {

@NotNull
private String videoTitle;

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

/**
* This field will only be populated if a corresponding {@link Video} can be
* found in the database for a given {@link #videoId}.
*/
@ManyToOne
private Video video;
Comment on lines +12 to +27
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

Consider using Optional for videoId.

The videoId field can be null, as indicated by the comment. To make this more explicit in the type system and avoid potential null pointer exceptions, consider using Optional<Long> instead of Long.

Here's a suggested change:

- private Long videoId;
+ private Optional<Long> videoId;

Remember to update the getter and setter methods accordingly if you make this change.

📝 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
@NotNull
private String videoTitle;
/**
* This field might not be included, e.g. if the videos were opened in a 3rd-party
* app that did not load the videos from the elimu.ai Content Provider. In this
* case, the {@link #videoId} will be {@code null}.
*/
private Long videoId;
/**
* This field will only be populated if a corresponding {@link Video} can be
* found in the database for a given {@link #videoId}.
*/
@ManyToOne
private Video video;
@NotNull
private String videoTitle;
/**
* This field might not be included, e.g. if the videos were opened in a 3rd-party
* app that did not load the videos from the elimu.ai Content Provider. In this
* case, the {@link #videoId} will be {@code null}.
*/
private Optional<Long> videoId;
/**
* This field will only be populated if a corresponding {@link Video} can be
* found in the database for a given {@link #videoId}.
*/
@ManyToOne
private Video video;


public String getVideoTitle() {
return videoTitle;
}

public void setVideoTitle(String videoTitle) {
this.videoTitle = videoTitle;
}

public Long getVideoId() {
return videoId;
}

public void setVideoId(Long videoId) {
this.videoId = videoId;
}

public Video getVideo() {
return video;
}

public void setVideo(Video video) {
this.video = video;
}
}
25 changes: 25 additions & 0 deletions src/main/resources/META-INF/jpa-schema-export.sql
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@

drop table if exists Video_Word;

drop table if exists VideoLearningEvent;

drop table if exists Word;

drop table if exists Word_LetterSound;
Expand Down Expand Up @@ -631,6 +633,19 @@
primary key (Video_id, words_id)
) engine=MyISAM;

create table VideoLearningEvent (
id bigint not null auto_increment,
androidId varchar(255),
learningEventType varchar(255),
packageName varchar(255),
timestamp datetime,
videoId bigint,
videoTitle varchar(255),
application_id bigint,
video_id bigint,
primary key (id)
) engine=MyISAM;
Comment on lines +636 to +647
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 9, 2024

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider optimizing the VideoLearningEvent table structure and engine.

The new VideoLearningEvent table looks good overall, but there are a few points to consider:

  1. There seems to be redundancy between videoId and video_id fields. Consider removing one of them to maintain data consistency.
  2. The table uses the MyISAM engine. For web applications, InnoDB is generally preferred due to its support for transactions, row-level locking, and foreign key constraints. Consider changing the engine to InnoDB for better data integrity and performance in concurrent scenarios.

Here's a suggested modification:

create table VideoLearningEvent (
   id bigint not null auto_increment,
    androidId varchar(255),
    learningEventType varchar(255),
    packageName varchar(255),
    timestamp datetime,
    videoTitle varchar(255),
    application_id bigint,
    video_id bigint,
    primary key (id)
) engine=InnoDB;

This change removes the redundant videoId field and changes the engine to InnoDB.

Copy link
Member Author

Choose a reason for hiding this comment

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

@coderabbitai How can I ensure that not null gets correctly included in the database schema?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jo-elimu, to ensure that the NOT NULL constraints are correctly included in your database schema, you need to specify nullable = false in the @Column annotations of your entity fields in the VideoLearningEvent class.

Here's how you can update your VideoLearningEvent entity:

@Entity
public class VideoLearningEvent extends LearningEvent {

    @Column(nullable = false)
    private String videoTitle;

    @Column(nullable = false)
    private Long videoId;

    @ManyToOne
    private Video video;

    // Getters and setters...
}

By setting nullable = false, you're indicating that these fields cannot be null, and the schema generation tool will include the NOT NULL constraint in the SQL schema.

After making these changes, regenerate the SQL schema using your build tool or ORM framework (e.g., running mvn clean compile if you're using Maven with Hibernate). The jpa-schema-export.sql file should then reflect the NOT NULL constraints accordingly.

Let me know if you need further assistance!


create table Word (
id bigint not null auto_increment,
contentStatus varchar(255),
Expand Down Expand Up @@ -1072,6 +1087,16 @@
foreign key (Video_id)
references Video (id);

alter table VideoLearningEvent
add constraint FKoqqhe1r2epyv55g6jo79t251h
foreign key (application_id)
references Application (id);

alter table VideoLearningEvent
add constraint FK38rllate5mtlhi6fdiudffm4c
foreign key (video_id)
references Video (id);

alter table Word
add constraint FKd1ussioi3bpu2tmxm0cim5s5a
foreign key (rootWord_id)
Expand Down
4 changes: 2 additions & 2 deletions src/main/webapp/WEB-INF/jsp/admin/layout.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@
</div>
</c:if>

<div id="${cssId}" class="container <c:if test="${cssClass != null}">${cssClass}</c:if>">
<main id="${cssId}" class="container <c:if test="${cssClass != null}">${cssClass}</c:if>">
<div class="section row">
<c:choose>
<c:when test="${!hasAside}">
Expand All @@ -204,6 +204,6 @@
</c:otherwise>
</c:choose>
</div>
</div>
</main>
</body>
</html>
4 changes: 2 additions & 2 deletions src/main/webapp/WEB-INF/jsp/analytics/layout.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
</div>
</c:if>

<div id="${cssId}" class="container <c:if test="${cssClass != null}">${cssClass}</c:if>">
<main id="${cssId}" class="container <c:if test="${cssClass != null}">${cssClass}</c:if>">
<div class="section row">
<c:choose>
<c:when test="${!hasAside}">
Expand All @@ -91,6 +91,6 @@
</c:otherwise>
</c:choose>
</div>
</div>
</main>
</body>
</html>
4 changes: 2 additions & 2 deletions src/main/webapp/WEB-INF/jsp/content/layout.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@
</div>
</c:if>

<div id="${cssId}" class="container <c:if test="${cssClass != null}">${cssClass}</c:if>">
<main id="${cssId}" class="container <c:if test="${cssClass != null}">${cssClass}</c:if>">
<div class="section row">
<c:choose>
<c:when test="${!hasAside}">
Expand All @@ -310,6 +310,6 @@
</c:otherwise>
</c:choose>
</div>
</div>
</main>
</body>
</html>
4 changes: 2 additions & 2 deletions src/main/webapp/WEB-INF/jsp/layout.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@
</div>
</c:if>

<div id="${cssId}" class="container <c:if test="${cssClass != null}">${cssClass}</c:if>">
<main id="${cssId}" class="container <c:if test="${cssClass != null}">${cssClass}</c:if>">
<div class="section row">
<c:choose>
<c:when test="${!hasAside}">
Expand All @@ -253,7 +253,7 @@
</c:otherwise>
</c:choose>
</div>
</div>
</main>

<c:if test="${!fn:contains(pageContext.request.requestURI, '/jsp/content/')}">
<footer class="page-footer deep-purple lighten-1">
Expand Down
15 changes: 10 additions & 5 deletions src/main/webapp/WEB-INF/jsp/welcome.jsp
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,8 @@
</p>
</div>
<div class="col s12 m5">
<img style="border-radius: 8px; outline: 8px solid #d1c4e9;" src="https://github.blog/wp-content/uploads/2019/05/xprize-no-text.png?w=1200" style="border-radius: 8px;" />
<img src="https://github.blog/wp-content/uploads/2019/05/xprize-no-text.png?w=1200"
style="border: 2px solid darkgoldenrod; border-right-width: 4px; border-bottom-width: 4px;" />
</div>
</div>

Expand All @@ -97,7 +98,8 @@
</p>
</div>
<div class="col s12 m5">
<img style="border-radius: 8px; outline: 8px solid #d1c4e9;" src="https://user-images.githubusercontent.com/15718174/26873794-641bfbfc-4b7b-11e7-95ee-82b19b661ad5.png" style="border-radius: 8px;" />
<img src="https://user-images.githubusercontent.com/15718174/26873794-641bfbfc-4b7b-11e7-95ee-82b19b661ad5.png"
style="border: 2px solid green; border-right-width: 4px; border-bottom-width: 4px;" />
</div>
</div>

Expand All @@ -116,7 +118,8 @@
</p>
</div>
<div class="col s12 m5">
<img style="border-radius: 8px; outline: 8px solid #d1c4e9;" src="https://user-images.githubusercontent.com/15718174/27299402-95bea44c-552c-11e7-84ab-217cdca758e4.gif" style="border-radius: 8px;" />
<img src="https://user-images.githubusercontent.com/15718174/27299402-95bea44c-552c-11e7-84ab-217cdca758e4.gif"
style="border: 2px solid rgba(103,58,183, 0.8); border-right-width: 4px; border-bottom-width: 4px;" />
</div>
</div>

Expand All @@ -131,7 +134,8 @@
</p>
</div>
<div class="col s12 m5">
<img style="border-radius: 8px; outline: 8px solid #d1c4e9;" src="https://user-images.githubusercontent.com/15718174/27515871-41872c60-59ae-11e7-9b2d-3ca886d0d7f2.png" style="border-radius: 8px;" />
<img src="https://user-images.githubusercontent.com/15718174/27515871-41872c60-59ae-11e7-9b2d-3ca886d0d7f2.png"
style="border: 2px solid rgba(103,58,183, 0.9); border-right-width: 4px; border-bottom-width: 4px;" />
</div>
</div>
</div>
Expand Down Expand Up @@ -193,7 +197,8 @@
<h2>🧑🏽‍🚀🧑🏽‍⚕️🧑🏽‍🚒🧑🏽‍🏫</h2>
</div>
<div class="row">
<img style="border-radius: 8px; opacity: 80%; outline: 8px solid #d1c4e9" src="https://i.pinimg.com/736x/e8/7c/56/e87c5635fa622c2da8c6dddd893113b9--open-source-children.jpg" />
<img src="https://i.pinimg.com/736x/e8/7c/56/e87c5635fa622c2da8c6dddd893113b9--open-source-children.jpg"
style="border: 2px solid darkgoldenrod; border-right-width: 4px; border-bottom-width: 4px;" />
</div>
</div>
</div>
Expand Down
16 changes: 3 additions & 13 deletions src/main/webapp/static/css/content/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ nav {
transition: all .5s;
}

#letterSoundsContainer .chip {
main #letterSoundsContainer .chip {
text-align: center;
font-size: 2rem;
padding: 1rem;
height: auto;
background-color: #EEE;
}

#letterCreatePage input#text,
Expand All @@ -34,19 +33,10 @@ nav {
height: 1.2em;
}

.card-panel img {
border-radius: 8px;
background-color: rgba(255,255,255, 0.5);
box-shadow: 4px 4px rgba(103,58,183, 0.2); /* deep-purple */
}
#labeledAudios img,
#labeledImages img,
#labeledVideos img {
width: 47%;
margin-right: 2%;
border-radius: 8px;
background-color: rgba(255,255,255, 0.5);
box-shadow: 4px 4px rgba(103,58,183, 0.2); /* deep-purple */
margin-top: 0.5em;
}
#imageEditPage #image img {
display: block;
Expand Down Expand Up @@ -82,7 +72,7 @@ html[data-content-language="hin"] #storyBookEditPage .storyBookChapter .storyBoo
blockquote,
.comment {
white-space: pre-line;
overflow: scroll;
overflow: auto;
}

.row.peerReviewEvent {
Expand Down
36 changes: 32 additions & 4 deletions src/main/webapp/static/css/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,38 @@ video {
max-width: 100%;
}

main img {
background-color: rgba(255,255,255, 0.5);
border-radius: 8px;
border: 2px solid rgba(103,58,183, 0.3); /* deep-purple */
box-shadow: 4px 4px rgba(103,58,183, 0.15); /* deep-purple */
}
main .card-panel img {
border-width: 1px;
box-shadow: 2px 2px rgba(103,58,183, 0.15); /* deep-purple */
}
main .card .card-image img {
border-radius: 8px 8px 0 0;
border-width: 0;
}
main .card .card-image {
border-radius: 8px 8px 0 0;
}

main .chip {
background-color: rgba(103,58,183, 0.05); /* deep-purple */
border: 1px solid rgba(103,58,183, 0.15); /* deep-purple */
box-shadow: none;

overflow: visible;
text-overflow: ellipsis;
white-space: nowrap;
}
main .chip img {
border: 0;
box-shadow: none;
}

#welcomePage .card-title {
text-shadow: 1px 1px rgba(0,0,0, .8),
0 -1rem 3rem rgba(0,0,0, .8),
Expand Down Expand Up @@ -98,10 +130,6 @@ footer.page-footer .footer-copyright {
.card-panel {
border-radius: 8px;
}
.card .card-image,
.card .card-image img {
border-radius: 8px 8px 0 0;
}
.card .card-action:last-child {
border-radius: 0 0 8px 8px;
}
Expand Down