Skip to content

Add legacy device deprecation banner for users and admins#420

Open
mindmonk wants to merge 1 commit intodevelopfrom
feature/legacy-device-banner
Open

Add legacy device deprecation banner for users and admins#420
mindmonk wants to merge 1 commit intodevelopfrom
feature/legacy-device-banner

Conversation

@mindmonk
Copy link
Contributor

To inform users ahead of time, a warning banner is shown on the profile page and vault list whenever the logged-in user still has legacy devices.

Admins additionally see a variant of this banner on the vault list and admin settings page when any user in the system has legacy devices.

Screenshots:
User – Profile Page
4

User – Vault List
3

Admin – Vault List
1

Admin – Admin Settings
2

@mindmonk mindmonk requested a review from overheadhunter March 11, 2026 13:27
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 11, 2026

Walkthrough

This PR adds support for detecting and displaying legacy device warnings across the application. On the backend, a new endpoint hasAnyLegacyDevices() is exposed at GET /devices/has-legacy-devices that returns a boolean indicating whether any legacy devices exist. The frontend adds a corresponding service method and integrates warning banners in three components (AdminSettings, LegacyDeviceList, VaultList) that display contextual messages to both regular users and administrators. New i18n entries are added for banner titles and descriptions in both user and admin contexts.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • overheadhunter
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding deprecation banners for legacy devices visible to both users and admins across multiple pages.
Description check ✅ Passed The description clearly relates to the changeset, explaining where banners appear, who sees them, and providing visual context through screenshots.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/legacy-device-banner

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
frontend/src/components/VaultList.vue (1)

219-220: Optional chaining on non-optional field.

The devices field on UserDto is typed as DeviceDto[] (non-optional). The optional chaining meWithLegacy.devices?.length is defensive but technically unnecessary. This is a minor nitpick and safe to keep as-is.

📝 Optional simplification
-    hasLegacyDevices.value = (meWithLegacy.devices?.length ?? 0) > 0;
+    hasLegacyDevices.value = meWithLegacy.devices.length > 0;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/VaultList.vue` around lines 219 - 220, The optional
chaining on a non-optional field is unnecessary; in the block that awaits
userdata.meWithLegacyDevicesAndLastAccess, replace the defensive access
meWithLegacy.devices?.length with a direct access meWithLegacy.devices.length
(keeping the surrounding assignment to hasLegacyDevices.value) so the code uses
the declared DeviceDto[] type correctly; update the expression where
meWithLegacy and hasLegacyDevices are referenced to remove the "?".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@frontend/src/components/VaultList.vue`:
- Around line 219-220: The optional chaining on a non-optional field is
unnecessary; in the block that awaits userdata.meWithLegacyDevicesAndLastAccess,
replace the defensive access meWithLegacy.devices?.length with a direct access
meWithLegacy.devices.length (keeping the surrounding assignment to
hasLegacyDevices.value) so the code uses the declared DeviceDto[] type
correctly; update the expression where meWithLegacy and hasLegacyDevices are
referenced to remove the "?".

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 10fdecee-193f-4700-8683-2e922ec07cc0

📥 Commits

Reviewing files that changed from the base of the PR and between 5614653 and 8bba159.

📒 Files selected for processing (7)
  • backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java
  • backend/src/main/java/org/cryptomator/hub/entities/LegacyDevice.java
  • frontend/src/common/backend.ts
  • frontend/src/components/AdminSettings.vue
  • frontend/src/components/LegacyDeviceList.vue
  • frontend/src/components/VaultList.vue
  • frontend/src/i18n/en-US.json

Copy link
Member

@overheadhunter overheadhunter left a comment

Choose a reason for hiding this comment

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

I have slight concerns regarding fetch efficiency of userdata.meWithLegacyDevicesAndLastAccess but since this is just a temporary banner, this should work.

@SailReal do you agree?

@overheadhunter overheadhunter added this to the 1.5.0 milestone Mar 12, 2026
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