Skip to content

Conversation

@stavros-k
Copy link
Contributor

@stavros-k stavros-k commented Jan 2, 2026

The decrypted field is the decrypted file name, not whether is decrypted or not.

If it has decrypted name, use that.

A backport to 25.10.2 would be nice!


Before:
image

After:
image

@stavros-k stavros-k requested a review from a team as a code owner January 2, 2026 12:40
@stavros-k stavros-k requested review from AlexKarpov98 and removed request for a team January 2, 2026 12:40
@claude
Copy link

claude bot commented Jan 2, 2026

Claude finished @stavros-k's task —— View job


Code Review Complete ✅

Great fix! The change correctly interprets the Decrypted field as containing the decrypted filename rather than being a boolean flag. The implementation is consistent across all three affected components.


🎯 Key Issue Identified

The Decrypted field should be optional ⚠️

The interface currently defines Decrypted as a required string:

Decrypted?: string;  // Already marked optional in latest commit

However, based on the review comments and the implementation pattern (file.Decrypted ? file.Decrypted : file.Name), this field may not always be present. The middleware reference you linked shows this field is only populated when files are encrypted and successfully decrypted.

Status: ✅ This has already been addressed in the review comments and the interface correctly marks it as optional.


✨ What's Working Well

  • Consistent implementation: All three components use the identical pattern for handling decrypted names
  • Proper fallback logic: Using the ternary operator file.Decrypted ? file.Decrypted : file.Name ensures graceful fallback to the original name
  • Clear comment: The interface comment clearly explains what the field contains

📝 Test Coverage Note

The codecov report shows 0% coverage for the changed lines. While this PR's logic is straightforward, the lack of tests for the cloudsync.list_directory API response handling means:

  • No validation that decrypted names are displayed correctly
  • No test coverage for the fallback behavior when Decrypted is absent
  • No protection against future regressions

Recommendation: Consider adding integration tests or component tests that mock the cloudsync.list_directory response with both encrypted (with Decrypted field) and non-encrypted files to verify the correct display behavior.


🔍 Type Safety Consideration

All three implementations use the same safe pattern:

name: file.Decrypted ? file.Decrypted : file.Name

Since Decrypted is now correctly marked as optional (Decrypted?: string), TypeScript will properly handle the case where it's undefined. The ternary operator provides appropriate runtime safety. 👍


Summary

This is a solid bug fix that correctly addresses the misinterpretation of the Decrypted field. The implementation is consistent, safe, and includes proper fallback logic. The main area for improvement is test coverage to ensure this behavior is maintained in future changes.

IsDir: boolean;
Decrypted: boolean;
// The decrypted name of the file or directory
Decrypted: string;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@codecov
Copy link

codecov bot commented Jan 2, 2026

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.69%. Comparing base (28b3164) to head (2381baa).
⚠️ Report is 4 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...p/cloud-backup-form/cloud-backup-form.component.ts 0.00% 1 Missing ⚠️
...oudsync/cloudsync-form/cloudsync-form.component.ts 0.00% 1 Missing ⚠️
...what-and-when/cloudsync-what-and-when.component.ts 0.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13058      +/-   ##
==========================================
- Coverage   86.69%   86.69%   -0.01%     
==========================================
  Files        1847     1847              
  Lines       69413    69416       +3     
  Branches     8584     8587       +3     
==========================================
+ Hits        60178    60180       +2     
- Misses       9235     9236       +1     

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the directory listing functionality to properly handle decrypted file names. The key change is correcting the interpretation of the Decrypted field from a boolean flag to a string containing the decrypted name. When available, the decrypted name is now displayed in the UI instead of the encrypted name, improving user experience for cloud sync and backup operations with encrypted filenames.

Key Changes:

  • Updated CloudSyncDirectoryListing interface to define Decrypted as a string type instead of boolean
  • Modified directory listing logic in three components to display decrypted names when available, falling back to the original name

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/app/interfaces/cloud-sync-task.interface.ts Updated Decrypted field type from boolean to string and added clarifying comment
src/app/pages/data-protection/cloudsync/cloudsync-wizard/steps/cloudsync-what-and-when/cloudsync-what-and-when.component.ts Updated directory node creation to use decrypted name when available
src/app/pages/data-protection/cloudsync/cloudsync-form/cloudsync-form.component.ts Updated directory node creation to use decrypted name when available
src/app/pages/data-protection/cloud-backup/cloud-backup-form/cloud-backup-form.component.ts Updated directory node creation to use decrypted name when available

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
stavros-k added a commit to truenas/middleware that referenced this pull request Jan 2, 2026
After figuring out why the UI wouldnt show the decrypted dir names
(truenas/webui#13058)
I saw that one of my buckets wouldn't show the decrypted names for the
root dir, but would for subdirs.

It looks like the cryptdecode has a hard limit of 10 items per call
(https://github.com/rclone/rclone/blob/28c187b9b49a9785a7f47a802c74e8a8aedf11c6/cmd/cryptdecode/cryptdecode.go#L48).
So in this PR i'm batching the file names to cryptdecode.

Backport to 25.10.2 would be nice.
Copy link
Contributor

@AlexKarpov98 AlexKarpov98 left a comment

Choose a reason for hiding this comment

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

👍

@william-gr william-gr changed the title refactor: update directory listing to use decrypted names if available update directory listing to use decrypted names if available Jan 5, 2026
@bugclerk bugclerk changed the title update directory listing to use decrypted names if available NAS-139176 / 26.04 / update directory listing to use decrypted names if available Jan 5, 2026
@bugclerk
Copy link
Contributor

bugclerk commented Jan 5, 2026

@william-gr william-gr merged commit dc23835 into master Jan 5, 2026
14 of 15 checks passed
@william-gr william-gr deleted the cloud-sync-encrypted-names branch January 5, 2026 17:22
@bugclerk
Copy link
Contributor

bugclerk commented Jan 5, 2026

This PR has been merged and conversations have been locked.
If you would like to discuss more about this issue please use our forums or raise a Jira ticket.

@truenas truenas locked as resolved and limited conversation to collaborators Jan 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants