Fix bugs in shot history

[dragm83]

• Add padding to filenames when deleting
• Check for duplicate entries in index when marking as deleted
• Add guards to prevent duplicate entries in index

Summary by CodeRabbit

• Bug Fixes

  • Fixed potential duplicate entries in shot history index
  • Ensured consistent file path references during deletion operations

• Improvements

  • Enhanced error handling and logging for better visibility into shot history operations
  • Improved robustness of index management and file handling

[coderabbitai]

Walkthrough

The ShotHistoryPlugin.cpp file receives robustness improvements including zero-padded ID formatting for file paths, duplicate detection in index append operations, index entry deletion marking with aggregation, and enhanced error handling with logging in the index completion update function.

Changes

Cohort / File(s)

Summary

Index management and error handling src/display/plugins/ShotHistoryPlugin.cpp

History delete path now pads IDs to 6 digits for consistent file path references. appendToIndex detects and prevents duplicate entries by checking existing entries before appending. markIndexDeleted marks all entries with a given shotId as deleted with aggregation and warning logs. updateIndexCompletion closes the index file after successful writes and adds error logging paths for read/write failures.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Duplicate prevention logic: Verify the duplicate detection mechanism correctly identifies all edge cases and doesn't have unintended performance implications on index operations
• ID padding consistency: Confirm the 6-digit padding is applied uniformly across all file path references (delete and read operations)
• Deletion aggregation: Review the aggregation logic in markIndexDeleted to ensure it correctly handles multiple duplicate entries and edge cases with no entries
• Error handling coverage: Check that all new error paths (read/write failures) are properly logged and don't mask underlying issues

Poem

🐰 With padded IDs, we organize with care,
Duplicates detected before they snare,
Entries marked deleted, tracked with precision,
Errors now logged—robustness our mission!
hop hop 📝

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 (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "Fix bugs in shot history" is fully related to the changeset and accurately summarizes the main objective of the pull request. All changes are contained within the ShotHistoryPlugin.cpp file and address specific bugs: ID padding for file deletion, duplicate entry prevention, and enhanced error handling. While the title is somewhat generic rather than specifying the exact nature of each bug, it is sufficiently clear and specific to convey that bug fixes are being applied to the shot history component, which allows teammates scanning the history to understand the primary change.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• 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 2d42b24 and 2d285df.

📒 Files selected for processing (1)

• src/display/plugins/ShotHistoryPlugin.cpp (4 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/display/plugins/ShotHistoryPlugin.cpp (2)

web/src/pages/ShotHistory/parseBinaryIndex.js (4)

• id (61-61)
• i (17-17)
• i (58-58)
• SHOT_FLAG_DELETED (10-10)

src/display/plugins/ShotHistoryPlugin.h (1)

• header (52-52)

⏰ 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). (2)

• GitHub Check: test
• GitHub Check: deploy

🔇 Additional comments (1)

src/display/plugins/ShotHistoryPlugin.cpp (1)

397-404: Good duplicate guard.

Bailing out when a matching ID is already present keeps the index consistent and saves us from hard-to-debug corruption later.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud