Skip to content

Conversation

@odanko01
Copy link

@odanko01 odanko01 commented Oct 2, 2025

### Issue Number

### Purpose

  • When an emoji is edited, existing word labels are removed.

Technical Details

  • Issue: The Emoji object passed to handleSubmit contained null in the words field. When calling emojiDao.update(emoji), this overwrote the existing words in the database with null, causing them to be removed.
  • Fix: Instead of updating directly with the detached emoji object, the persistent existingEmoji is loaded from the database. Its scalar fields (glyph, unicodeVersion, unicodeEmojiVersion, revisionNumber) are updated, and then emojiDao.update(existingEmoji) is called. This preserves the associated words.

Testing Instructions

  • Open any existing emoji or create a new one
  • Add any words to it
  • Update any value (Glyph, Unicode version or Enicode Emoji version) and click Edit

Screenshots

  • Before:
image * After updating Unicode Emoji version to 2: image

@odanko01 odanko01 requested a review from a team as a code owner October 2, 2025 15:21
@odanko01 odanko01 requested review from alexander-kuruvilla, eymaal and vrudas and removed request for a team October 2, 2025 15:21
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

Walkthrough

Switches Emoji update to read-modify-write: loads persistent Emoji, updates fields, increments revision, saves persistent instance, and updates subsequent references (event association, Discord message, redirect) to use the persistent entity.

Changes

Cohort / File(s) Summary
Controller read-modify-write update
src/main/java/ai/elimu/web/content/emoji/EmojiEditController.java
Replace direct update with DAO fetch, field updates on persistent entity (glyph, unicodeVersion, unicodeEmojiVersion), increment revision on persistent object, save persistent instance; update event association, Discord message, and redirect to use persistent emoji and its revision/id.

Sequence Diagram(s)

sequenceDiagram
  participant U as User
  participant C as EmojiEditController
  participant D as EmojiDao
  participant E as EmojiContributionEventService
  participant N as DiscordNotifier

  U->>C: POST /content/emoji/edit (emoji form data)
  C->>D: findById(emoji.id)
  D-->>C: persistentEmoji
  C->>C: Update fields on persistentEmoji<br/>Increment revision
  C->>D: save(persistentEmoji)
  D-->>C: saved persistentEmoji
  C->>E: createEvent(persistentEmoji, revision)
  E-->>C: event created
  C->>N: sendEditNotification(persistentEmoji.glyph, revision)
  C-->>U: Redirect to /content/emoji/edit/{persistentEmoji.id}
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed The title directly references the linked issue and concisely describes the bug fix for preserving word labels when editing an emoji, clearly summarizing the primary change.
Linked Issues Check ✅ Passed The modifications accurately implement the linked issue’s requirements by loading and updating the persistent Emoji entity to prevent overwriting its associated word labels, fully addressing issue #2327.
Out of Scope Changes Check ✅ Passed All changes are localized to the EmojiEditController logic for emoji updates and directly relate to preserving word labels, with no unrelated or extraneous modifications introduced.
Description Check ✅ Passed The pull request description follows the repository’s template by including Issue Number, Purpose, Technical Details, Testing Instructions, and Screenshots sections with all required content and examples.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c1848f and 6077865.

📒 Files selected for processing (1)
  • src/main/java/ai/elimu/web/content/emoji/EmojiEditController.java (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: test_rest
  • GitHub Check: build (ubuntu-latest, 21)
  • GitHub Check: test_rest
  • GitHub Check: test_rest
  • GitHub Check: test_rest
  • GitHub Check: build (windows-latest, 21)
  • GitHub Check: build (windows-latest, 17)
  • GitHub Check: build (ubuntu-latest, 17)
  • GitHub Check: build (macos-latest, 17)
  • GitHub Check: test_rest
  • GitHub Check: build (macos-latest, 21)
  • GitHub Check: test_rest
🔇 Additional comments (4)
src/main/java/ai/elimu/web/content/emoji/EmojiEditController.java (4)

95-101: LGTM! Correctly implements read-modify-write to preserve associations.

The fix properly addresses the root cause by loading the persistent emoji, updating only the scalar fields (glyph, unicodeVersion, unicodeEmojiVersion), incrementing the revision number, and saving the persistent instance. This prevents the null words collection in the incoming emoji parameter from overwriting the database associations.


106-107: LGTM! Contribution event correctly references the persistent emoji.

The contribution event now uses persistentEmoji and its updated revision number, ensuring accurate tracking of the entity state after the update.


110-110: LGTM! Discord notification uses the persistent emoji reference.

Using persistentEmoji for the notification is semantically correct and ensures the message reflects the actual persisted state.


112-112: LGTM! Redirect URL uses the persistent emoji ID.

Using persistentEmoji.getId() maintains consistency with the rest of the method, even though the ID value should be identical to the incoming emoji.getId().


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Oct 2, 2025

Codecov Report

❌ Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.83%. Comparing base (2aa995d) to head (6077865).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
...i/elimu/web/content/emoji/EmojiEditController.java 0.00% 10 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2352      +/-   ##
============================================
- Coverage     16.84%   16.83%   -0.01%     
  Complexity      465      465              
============================================
  Files           264      264              
  Lines          7848     7852       +4     
  Branches        903      903              
============================================
  Hits           1322     1322              
- Misses         6450     6454       +4     
  Partials         76       76              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@jo-elimu jo-elimu left a comment

Choose a reason for hiding this comment

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

Thank you, @odanko01

Another solution would be to include the IDs of the word labels as a hidden input field inside the

<form:form modelAttribute="emoji">

element in the edit.jsp file.

But your solution also works, so approving your PR for merging 👍

@jo-elimu jo-elimu merged commit 26805f9 into elimu-ai:main Oct 2, 2025
17 of 32 checks passed
@nya-elimu
Copy link
Member

@odanko01 Congrats on your first pull request contribution! 🎉

If you would like to be included in our monthly token allocations, please add your Ethereum address to contributor-addresses.json.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Word labels incorrectly removed when editing emoji

3 participants