refactor: remove audio - frontend code #2049
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2049 +/- ##
============================================
+ Coverage 14.53% 15.29% +0.75%
+ Complexity 418 406 -12
============================================
Files 256 243 -13
Lines 7154 6671 -483
Branches 796 747 -49
============================================
- Hits 1040 1020 -20
+ Misses 6064 5601 -463
Partials 50 50 ☔ View full report in Codecov by Sentry. |
jo-elimu
changed the title
jo-elimu
requested review from
nya-elimu,
vuriaval,
Souvik-Cyclic and
AshishBagdane
and removed request for
a team
|
Warning Rate limit exceeded@jo-elimu has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 17 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughThis pull request removes audio-related functionality from various parts of the application. Changes include deleting documentation instructions for adding audio recordings, removing multiple audio DAOs and their JPA implementations, and eliminating converters and REST endpoints associated with audio entities. In addition, several controllers and JSP templates that handled audio creation, editing, listing, exporting, and downloading have been removed or modified. Localization files and CSS rules specific to audio are also updated. A dependency version was bumped and a new dependency added, but no new public methods or control flows were introduced related to audio. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/main/webapp/WEB-INF/jsp/content/word/edit.jsp (1)
422-423: 💡 Verification agent🧩 Analysis chain
Address the TODO comment for labeled videos
There's a TODO comment for the labeled videos section that should be addressed or removed, particularly since this PR is removing audio functionality.
🏁 Script executed:
#!/bin/bash # Check if there are other instances of labeled videos implementation that could be referenced rg -A 3 "labeledVideos" --type jspLength of output: 62
I'll run an updated shell script that explicitly registers JSP files with ripgrep and searches for any references to
"labeledVideos". This will help us determine if there’s any related implementation that we might need to consider before addressing the TODO.Please execute the following script:
Let me know the output, and then we can finalize the review comment accordingly.
🏁 Script executed:
#!/bin/bash # Add a custom file type for JSP files and search for "labeledVideos" references in the codebase. rg --type-add 'jsp:*.jsp' -l "labeledVideos"Length of output: 262
Action Required: Clean Up Labeled Videos TODO Comment
- In
src/main/webapp/WEB-INF/jsp/content/word/edit.jsp(lines 422–423), the TODO comment for labeled videos remains even though audio functionality is being removed. Please either remove the comment or update it to reflect the current intended behavior.- Additionally, note that a reference to
"labeledVideos"appears insrc/main/webapp/static/css/content/styles.css. Verify whether this CSS reference is still needed or if it should also be updated/removed for consistency.
🧹 Nitpick comments (3)
pom-dependency-tree.txt (1)
93-96: Review of Audio-Related Dependencies
Although this PR is focused on removing audio functionality, libraries such ascom.googlecode.soundlibs:jlayer,tritonus-share, andmp3spistill appear in the dependency tree (lines 93–96). Please confirm that these libraries are still needed, or consider removing them if they are now obsolete. This cleanup can help reduce unnecessary package bloat in future iterations.src/main/webapp/WEB-INF/jsp/content/word/edit.jsp (2)
322-373: Excellent resource replacement for removed audio functionality!The addition of external pronunciation and transcription resources is a good alternative to the removed audio functionality. The implementation is well-organized with language-specific sections and appropriate external links that will help users access pronunciations.
Consider enhancing the user experience by adding a small info message explaining that audio functionality has been removed and these external resources are now recommended instead. This would help users understand the transition:
<content:aside> <c:if test="${applicationScope.configProperties['content.language'] == 'TGL'}"> + <div class="card-panel yellow lighten-4"> + <i class="material-icons left">info</i> + <span>Audio functionality has been removed. Please use the resources below for pronunciation assistance.</span> + </div> <h5 class="center"><fmt:message key="resources" /></h5>
326-337: Consider adding error handling for external resource linksThe external links to services like Forvo and Google Translate might not always handle all words correctly, especially those with special characters or complex orthography.
Consider adding some basic error handling hints for users:
For assistance with pronunciation and IPA transcription of "<c:out value='${word.text}' />", see: <ol style="list-style-type: inherit;"> <li> <a href="https://forvo.com/word/<c:out value='${word.text}' />/#hi" target="_blank">Forvo</a> + <small>(If not found, try searching without diacritical marks)</small> </li> <li> <a href="https://translate.google.com/?sl=hi&tl=en&op=translate&text=<c:out value='${word.text}' />" target="_blank">Google Translate</a> + <small>(Click the speaker icon to hear pronunciation)</small> </li>Also applies to: 347-356
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
src/main/resources/db/content_PROD/eng/audios.csvis excluded by!**/*.csv,!**/*.csvsrc/main/resources/db/content_PROD/hin/audios.csvis excluded by!**/*.csv,!**/*.csvsrc/main/resources/db/content_PROD/tgl/audios.csvis excluded by!**/*.csv,!**/*.csvsrc/main/resources/db/content_PROD/tha/audios.csvis excluded by!**/*.csv,!**/*.csvsrc/main/webapp/WEB-INF/spring/applicationContext-jpa.xmlis excluded by!**/*.xmlsrc/main/webapp/WEB-INF/spring/applicationContext.xmlis excluded by!**/*.xml
📒 Files selected for processing (42)
LOCALIZE.md(0 hunks)pom-dependency-tree.txt(1 hunks)src/main/java/ai/elimu/dao/AudioContributionEventDao.java(0 hunks)src/main/java/ai/elimu/dao/AudioDao.java(0 hunks)src/main/java/ai/elimu/dao/AudioPeerReviewEventDao.java(0 hunks)src/main/java/ai/elimu/dao/ContributorDao.java(0 hunks)src/main/java/ai/elimu/dao/jpa/AudioContributionEventDaoJpa.java(0 hunks)src/main/java/ai/elimu/dao/jpa/AudioDaoJpa.java(0 hunks)src/main/java/ai/elimu/dao/jpa/AudioPeerReviewEventDaoJpa.java(0 hunks)src/main/java/ai/elimu/dao/jpa/ContributorDaoJpa.java(0 hunks)src/main/java/ai/elimu/logic/converters/StringToAudioConverter.java(0 hunks)src/main/java/ai/elimu/rest/v2/JpaToGsonConverter.java(0 hunks)src/main/java/ai/elimu/rest/v2/content/AudiosRestController.java(0 hunks)src/main/java/ai/elimu/web/content/MainContentController.java(0 hunks)src/main/java/ai/elimu/web/content/multimedia/audio/AudioCreateController.java(0 hunks)src/main/java/ai/elimu/web/content/multimedia/audio/AudioCsvExportController.java(0 hunks)src/main/java/ai/elimu/web/content/multimedia/audio/AudioEditController.java(0 hunks)src/main/java/ai/elimu/web/content/multimedia/audio/AudioListController.java(0 hunks)src/main/java/ai/elimu/web/content/multimedia/image/ImageEditController.java(0 hunks)src/main/java/ai/elimu/web/content/peer_review/AudioPeerReviewEventCreateController.java(0 hunks)src/main/java/ai/elimu/web/content/storybook/chapter/StoryBookChapterDeleteController.java(0 hunks)src/main/java/ai/elimu/web/content/storybook/paragraph/StoryBookParagraphCreateController.java(0 hunks)src/main/java/ai/elimu/web/content/storybook/paragraph/StoryBookParagraphDeleteController.java(0 hunks)src/main/java/ai/elimu/web/content/word/WordEditController.java(0 hunks)src/main/java/ai/elimu/web/contributor/ContributorController.java(0 hunks)src/main/java/ai/elimu/web/download/AudioController.java(0 hunks)src/main/webapp/WEB-INF/i18n/errors_en.properties(0 hunks)src/main/webapp/WEB-INF/i18n/messages_en.properties(0 hunks)src/main/webapp/WEB-INF/jsp/content/layout.jsp(0 hunks)src/main/webapp/WEB-INF/jsp/content/main.jsp(0 hunks)src/main/webapp/WEB-INF/jsp/content/multimedia/audio/create.jsp(0 hunks)src/main/webapp/WEB-INF/jsp/content/multimedia/audio/edit.jsp(0 hunks)src/main/webapp/WEB-INF/jsp/content/multimedia/audio/list.jsp(0 hunks)src/main/webapp/WEB-INF/jsp/content/multimedia/image/edit.jsp(0 hunks)src/main/webapp/WEB-INF/jsp/content/sound/edit.jsp(1 hunks)src/main/webapp/WEB-INF/jsp/content/sound/list.jsp(1 hunks)src/main/webapp/WEB-INF/jsp/content/storybook/paragraph/edit.jsp(0 hunks)src/main/webapp/WEB-INF/jsp/content/word/edit.jsp(1 hunks)src/main/webapp/WEB-INF/jsp/contributions/privacy-policy.jsp(1 hunks)src/main/webapp/WEB-INF/jsp/contributor/contributor-summarized.jsp(1 hunks)src/main/webapp/WEB-INF/jsp/contributor/contributor.jsp(0 hunks)src/main/webapp/static/css/content/styles.css(0 hunks)
💤 Files with no reviewable changes (36)
- src/main/java/ai/elimu/dao/jpa/ContributorDaoJpa.java
- src/main/webapp/WEB-INF/jsp/content/main.jsp
- src/main/java/ai/elimu/web/content/MainContentController.java
- src/main/java/ai/elimu/web/content/multimedia/image/ImageEditController.java
- src/main/java/ai/elimu/web/content/word/WordEditController.java
- src/main/java/ai/elimu/web/content/multimedia/audio/AudioCsvExportController.java
- src/main/java/ai/elimu/web/content/multimedia/audio/AudioListController.java
- src/main/webapp/static/css/content/styles.css
- LOCALIZE.md
- src/main/java/ai/elimu/logic/converters/StringToAudioConverter.java
- src/main/webapp/WEB-INF/jsp/content/multimedia/audio/list.jsp
- src/main/java/ai/elimu/dao/ContributorDao.java
- src/main/java/ai/elimu/web/content/multimedia/audio/AudioCreateController.java
- src/main/java/ai/elimu/web/download/AudioController.java
- src/main/webapp/WEB-INF/jsp/content/layout.jsp
- src/main/webapp/WEB-INF/i18n/errors_en.properties
- src/main/webapp/WEB-INF/jsp/content/storybook/paragraph/edit.jsp
- src/main/java/ai/elimu/dao/AudioContributionEventDao.java
- src/main/java/ai/elimu/dao/AudioPeerReviewEventDao.java
- src/main/java/ai/elimu/dao/jpa/AudioContributionEventDaoJpa.java
- src/main/webapp/WEB-INF/jsp/content/multimedia/audio/edit.jsp
- src/main/java/ai/elimu/web/content/storybook/paragraph/StoryBookParagraphCreateController.java
- src/main/java/ai/elimu/dao/jpa/AudioPeerReviewEventDaoJpa.java
- src/main/java/ai/elimu/web/content/peer_review/AudioPeerReviewEventCreateController.java
- src/main/java/ai/elimu/web/content/multimedia/audio/AudioEditController.java
- src/main/webapp/WEB-INF/jsp/content/multimedia/image/edit.jsp
- src/main/webapp/WEB-INF/jsp/content/multimedia/audio/create.jsp
- src/main/java/ai/elimu/web/contributor/ContributorController.java
- src/main/java/ai/elimu/rest/v2/JpaToGsonConverter.java
- src/main/java/ai/elimu/dao/jpa/AudioDaoJpa.java
- src/main/webapp/WEB-INF/i18n/messages_en.properties
- src/main/java/ai/elimu/web/content/storybook/chapter/StoryBookChapterDeleteController.java
- src/main/java/ai/elimu/web/content/storybook/paragraph/StoryBookParagraphDeleteController.java
- src/main/webapp/WEB-INF/jsp/contributor/contributor.jsp
- src/main/java/ai/elimu/rest/v2/content/AudiosRestController.java
- src/main/java/ai/elimu/dao/AudioDao.java
✅ Files skipped from review due to trivial changes (2)
- src/main/webapp/WEB-INF/jsp/content/sound/edit.jsp
- src/main/webapp/WEB-INF/jsp/content/sound/list.jsp
🔇 Additional comments (9)
src/main/webapp/WEB-INF/jsp/contributions/privacy-policy.jsp (1)
25-30: Privacy policy updated to reflect the removal of audio functionality.The privacy policy has been appropriately updated to remove the reference to audio recordings, replacing it with video uploads instead. This change is consistent with the PR objective of removing audio functionality from both the REST API and UI.
I would recommend confirming that video uploads are indeed supported by the application, or will be supported in the future. If video uploads aren't an actual feature of the application, consider using a different example or more generic wording like "uploads content to the website".
src/main/webapp/WEB-INF/jsp/contributor/contributor-summarized.jsp (5)
88-88: Chart labels correctly updated to remove audio.The labels array has been properly updated to remove 'Audios' as part of the audio functionality removal.
91-91: Contributions dataset properly updated.The data array for contributions has been correctly modified to exclude audio contributions, maintaining only storybooks, words, and numbers data.
97-97: Peer-reviews dataset properly updated.The data array for peer-reviews has been correctly modified to exclude audio peer reviews, maintaining only storybooks, words, and numbers data.
86-109: Verify chart rendering with reduced dataset.The radar chart now has three data points instead of four. This change should work correctly, but it's worth confirming that the chart renders properly with the reduced dataset. Make sure to test the visualization in the UI to confirm that the chart appearance is still balanced and informative with one fewer axis.
88-103:✅ Verification successful
Verify chart variable references are defined.
The chart now uses three variables instead of four, but we should ensure that the referenced variables (
storyBookContributionsCount,wordContributionsCount,numberContributionsCount,storyBookPeerReviewsCount,wordPeerReviewsCount,numberPeerReviewsCount) are still properly defined in the controller that populates this JSP.
🏁 Script executed:
#!/bin/bash # Description: Check if all chart variables are defined in the controller that renders this JSP # Find the controller that serves this JSP echo "Looking for controller that serves contributor-summarized.jsp..." rg -A 5 -B 5 "contributor-summarized" --glob "*.java" # Check for variable definitions in potential controller files echo "Looking for chart variable definitions in controllers..." rg "storyBookContributionsCount|wordContributionsCount|numberContributionsCount|storyBookPeerReviewsCount|wordPeerReviewsCount|numberPeerReviewsCount" --glob "*.java"Length of output: 2917
Chart variable declarations verified and correctly defined
The chart data variables—
storyBookContributionsCount,wordContributionsCount,numberContributionsCount,storyBookPeerReviewsCount,wordPeerReviewsCount, andnumberPeerReviewsCount—are all defined in the controller (src/main/java/ai/elimu/web/contributor/ContributorController.java). The shell script output confirms that these variables are set viamodel.addAttributebefore rendering the JSP, so the code changes in this JSP are properly supported.pom-dependency-tree.txt (2)
1-1: Version Bump Verification for Webapp Artifact
The version update from2.5.34-SNAPSHOTto2.5.35-SNAPSHOTon line 1 is clear. Please ensure that this version bump is coordinated with any related components and properly integrated into your release process.
2-4: New Dependency Integration:com.github.elimu-ai:model
A new dependency (com.github.elimu-ai:model:jar:model-2.0.83:compile) has been added along with its transitive dependencies (gson:2.11.0anderror_prone_annotations:2.27.0). Verify that:
- This dependency is required following the refactor (especially after removing audio-related functionality).
- There are no conflicts with existing dependencies.
src/main/webapp/WEB-INF/jsp/content/word/edit.jsp (1)
359-372:❓ Verification inconclusive
Ensure consistent wiki link after audio removal
The link to the wiki (LOCALIZATION.md) may need to be reviewed since, according to the AI summary, the section on adding audio recordings has been deleted from that file.
🏁 Script executed:
#!/bin/bash # Check if the LOCALIZATION.md file was modified in this PR and if it still contains relevant content git diff origin/main...HEAD -- "*LOCALIZATION.md"Length of output: 92
Verify and Update Wiki Link Consistency
The code still references the wiki via the LOCALIZATION.md link. However, our initial check on modifications was inconclusive due to a merge base issue. Please manually verify that the linked LOCALIZATION.md in the external wiki has been updated to remove the audio recording instructions. If those instructions are indeed removed, consider whether the link text "elimu.ai Wiki" remains appropriate or if it needs adjustment to reflect the current content.
- Action Items:
- Manually check the current content of LOCALIZATION.md for any remaining references to audio instructions.
- Update the link text or URL if the removal of audio instructions necessitates a change in messaging.
jo-elimu
changed the title
Removes audio recordings from REST API and UI.
Issue Number
Purpose
Technical Details
Testing Instructions
Screenshots
Format Checks
Note
Files in PRs are automatically checked for format violations with
mvn spotless:check.If this PR contains files with format violations, run
mvn spotless:applyto fix them.