Improve Emergency Access view loading performance #440
Improve Emergency Access view loading performance #440
Conversation
WalkthroughThis change implements a performance optimization for emergency access recovery processes by adding a batch-retrieval mechanism. A new backend endpoint Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue (1)
435-437:⚠️ Potential issue | 🔴 CriticalDon't pass
includesunbound toevery().Line 437 will throw a
TypeErrorwhen thestartablefilter is selected. Whenevery()invokesprocessTypes.includesas a plain callback, thethiscontext is lost (set toundefined), andArray.prototype.includesrequires a validthisvalue. Wrap it in a lambda instead.Suggested fix
- return !SUPPORTED_PROCESS_TYPES.every(processTypes.includes); + return !SUPPORTED_PROCESS_TYPES.every(type => processTypes.includes(type));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue` around lines 435 - 437, The 'startable' case uses SUPPORTED_PROCESS_TYPES.every(processTypes.includes) which passes includes unbound and will throw a TypeError; change the call to provide a bound callback such as using a lambda that calls processTypes.includes for each type (e.g., SUPPORTED_PROCESS_TYPES.every(t => processTypes.includes(t))) so that includes runs with the correct receiver; update the logic in the case labeled 'startable' referencing processTypes and SUPPORTED_PROCESS_TYPES accordingly.
🧹 Nitpick comments (1)
frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue (1)
144-148: Drop the redundantpictureUrlguard.
loadMembersForVault()callsbackend.vaults.getMembers(vaultId), which already fills missing avatars in the service layer, so this conditional is dead weight here. Render the image unconditionally and rely on the shared contract instead.Based on learnings, in the frontend codebase, ensure that authority DTOs for users and groups always include a pictureUrl by having AuthorityService.fillInMissingPicture() run at the service layer, and UI components should not implement their own fallback logic for missing images.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue` around lines 144 - 148, Remove the redundant guard around the avatar image in EmergencyAccessVaultList.vue: because loadMembersForVault() uses backend.vaults.getMembers(vaultId) which relies on AuthorityService.fillInMissingPicture() to populate pictureUrl, delete the v-if="member.pictureUrl" condition and render the <img> unconditionally (keep the :src="member.pictureUrl" and class attributes) so the UI relies on the service-layer contract rather than duplicating fallback logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue`:
- Line 188: The hard-coded heading "Vault Access" in
EmergencyAccessVaultList.vue should be localized; replace the literal string
inside the <h4> element with a call to the localization function used in this
component (e.g., t('...') or $t('...')) so the heading uses the same i18n key
naming convention as other strings in this file (update the key to something
like 'emergencyAccess.vaultAccess' to match existing keys and add the key/value
to the locale files).
---
Outside diff comments:
In `@frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue`:
- Around line 435-437: The 'startable' case uses
SUPPORTED_PROCESS_TYPES.every(processTypes.includes) which passes includes
unbound and will throw a TypeError; change the call to provide a bound callback
such as using a lambda that calls processTypes.includes for each type (e.g.,
SUPPORTED_PROCESS_TYPES.every(t => processTypes.includes(t))) so that includes
runs with the correct receiver; update the logic in the case labeled 'startable'
referencing processTypes and SUPPORTED_PROCESS_TYPES accordingly.
---
Nitpick comments:
In `@frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue`:
- Around line 144-148: Remove the redundant guard around the avatar image in
EmergencyAccessVaultList.vue: because loadMembersForVault() uses
backend.vaults.getMembers(vaultId) which relies on
AuthorityService.fillInMissingPicture() to populate pictureUrl, delete the
v-if="member.pictureUrl" condition and render the <img> unconditionally (keep
the :src="member.pictureUrl" and class attributes) so the UI relies on the
service-layer contract rather than duplicating fallback logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d9790049-7901-4c5c-9532-d080036eda93
📒 Files selected for processing (4)
backend/src/main/java/org/cryptomator/hub/api/EmergencyAccessResource.javabackend/src/main/java/org/cryptomator/hub/entities/EmergencyRecoveryProcess.javafrontend/src/common/backend.tsfrontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue
|
|
||
| <!-- VAULT MEMBERS --> | ||
| <section class="flex flex-1 min-w-0 flex-col"> | ||
| <h4 class="mb-2 font-semibold text-gray-700">Vault Access</h4> |
There was a problem hiding this comment.
Localize this heading.
Vault Access is hard-coded, so it will stay English while the rest of the screen uses t(...).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue` at line
188, The hard-coded heading "Vault Access" in EmergencyAccessVaultList.vue
should be localized; replace the literal string inside the <h4> element with a
call to the localization function used in this component (e.g., t('...') or
$t('...')) so the heading uses the same i18n key naming convention as other
strings in this file (update the key to something like
'emergencyAccess.vaultAccess' to match existing keys and add the key/value to
the locale files).
1 participant
Problem
When opening the Emergency Access view, the frontend made one sequential HTTP request per vault to load its recovery processes (N+1 requests total). This caused unnecessary load time, especially with many vaults.
Additionally, the template structure was missing a proper v-if/v-else-if chain, causing other content (license alerts, banners, etc.) to render alongside the loading indicator instead of being hidden until data is ready.
Solution
A new bulk endpoint loads all recovery processes for the current user's recoverable vaults in a single backend query. On the frontend, this request runs in parallel with the vault list fetch, replacing the sequential loop.
The template was also restructured so that loading, error, and content states are mutually exclusive.