Skip to content

Comments

Fixed duplicate key issue#3452

Merged
tamaskozmer merged 3 commits intorelease/student-8.4.2-285from
fix-potential-todo-crash
Dec 18, 2025
Merged

Fixed duplicate key issue#3452
tamaskozmer merged 3 commits intorelease/student-8.4.2-285from
fix-potential-todo-crash

Conversation

@tamaskozmer
Copy link
Contributor

Test plan: Use a proxy to generate a To Do list response that contains two items with the same id.

@github-actions
Copy link

github-actions bot commented Dec 17, 2025

🧪 Unit Test Results


📊 Summary

  • Total Tests: 0
  • Failed: 0
  • Skipped: 0
  • Status: ⚠️ No test results found

Last updated: Thu, 18 Dec 2025 09:39:26 GMT

Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Review Summary

This PR modifies the key generation strategy for LazyColumn items in the ToDoList screen. While the intention appears to be solving duplicate key issues, the implementation has concerns that should be addressed.

Issues Found

  • Key uses index which breaks Compose's state preservation (line 375)
    • Using array indices in keys causes state loss and animation issues when list order changes
    • Compose keys should be stable and tied to item identity, not position
    • See inline comment for recommended solutions

What Works Well

✅ Good attempt to ensure key uniqueness by combining multiple properties
✅ Maintains the existing item rendering logic without changes

Recommendations

  1. Investigate the root cause: Why is item.id not sufficient as a unique key? This should be addressed at the data layer.

  2. If items can appear in multiple date groups: Use "${date.time}_${item.id}" without the index.

  3. If item.id truly isn't unique: This indicates a data model problem that should be fixed rather than worked around with indices.

  4. Add a comment: If there's a specific reason for this key structure, document it so future maintainers understand the rationale.

Testing Suggestions

  • Verify list reordering works correctly (if applicable)
  • Test filtering/sorting to ensure items maintain their state
  • Check for any animation glitches when items move positions

Let me know if you need clarification on the Jetpack Compose key semantics or help investigating the duplicate key issue!

Copy link
Contributor

@kristofnemere kristofnemere left a comment

Choose a reason for hiding this comment

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

QA+1

@tamaskozmer tamaskozmer merged commit b21a76a into release/student-8.4.2-285 Dec 18, 2025
24 checks passed
@tamaskozmer tamaskozmer deleted the fix-potential-todo-crash branch December 18, 2025 09:54
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.

2 participants