fix(mail-sidebar): restore body-mount + attachment drag-drop + header email typo#1285
fix(mail-sidebar): restore body-mount + attachment drag-drop + header email typo#1285rubenvdlinde wants to merge 14 commits intobetafrom
Conversation
- Convert Dutch hardcoded strings to English t() keys in OrganisationsIndex, UploadFiles - Wrap bare attributes and template strings across 20+ Vue files - Wrap showSuccess/showError notifications in settings store - Update l10n files with new keys and Dutch translations
Standardize on #actions across all components per the updated slot naming convention in @conduction/nextcloud-vue.
Update all translation keys to use sentence case (only first letter capitalized) instead of title case. Keys changed: - "Add Groups" → "Add groups" - "Active Collections" → "Active collections" - "API Key" → "API key" - And 400+ similar keys across all apps Sentence case for keys improves consistency and readability while preserving proper English grammar in translated values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Wrap bare strings in 20+ Vue files (modals, views, settings, workflow) - Convert Dutch strings in OrganisationsIndex and UploadFiles to English - Fix template attributes (label=, placeholder=, title=, aria-label=) - Wrap showSuccess/showError notifications in t() - Regenerate l10n files with 1,863 keys (sentence case)
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 147/147 | |||
| npm | ✅ | ✅ 599/599 | |||
| PHPUnit | ❌ | ||||
| Newman | ❌ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-04-20 07:07 UTC
Download the full PDF report from the workflow artifacts.
The two NcTextArea placeholders in EditWebhook.vue were written as inline
template expressions containing a `\n` escape:
:placeholder="t('openregister', 'X-Custom-Header: value\nAuthorization: Bearer token')"
:placeholder="t('openregister', 'objectType: object\naction: created')"
Vue's template compiler inlines that expression into the generated render
function, where the `\n` is turned into a real newline character inside a
single-quoted JS string. Webpack then emits that literal newline into
openregister-main.js, producing an unterminated string constant. V8 throws
"Invalid or unexpected token" on parse and the entire app bundle never
executes, so /apps/openregister/ renders a blank page under the Nextcloud
shell with no further console info.
Move both strings to `headersPlaceholder` / `filtersPlaceholder` computed
properties (where `\n` is a normal JS escape) and bind them by name. Added
a comment on the computed block so the next person who's tempted to inline
a multi-line placeholder knows why not to.
Add bind-mount for ../decidesk so the Decidesk app is available in the dev nextcloud container alongside the other custom_apps.
…ent class name Previously the listener decided whether to inject its script by string- matching "OCA\\Mail\\" against the fully qualified event class name. That was fragile: the listener is registered against the core BeforeTemplateRenderedEvent, and Mail's own class may not appear in the FQCN depending on how the event is dispatched. Instead, verify the event is a BeforeTemplateRenderedEvent, cast the response to TemplateResponse, and check $response->getApp() === 'mail'. This is the supported way to target rendering from a specific app and avoids the coincidental-string-match hazard.
Adds docker/mail/seed-cases.sh which creates two caseType objects (Omgevingsvergunning, Kapvergunning) and two case objects (ZK-2026-0142, ZK-2026-0034) in the Procest register (id=92) via the OpenRegister objects API. The cases are the counterparts of the emails seeded by seed-mail.sh, so the OpenRegister mail sidebar has real link targets to demo the email-to-case linking flow end-to-end. Default target is http://localhost:8080 with admin:admin; override via positional args. Script header documents prerequisites and the follow-up TODOs (Decidesk meetings, Pipelinq leads) that are blocked on upstream register initialisation.
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 147/147 | |||
| npm | ✅ | ✅ 599/599 | |||
| PHPUnit | ❌ | ||||
| Newman | ❌ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-04-22 01:57 UTC
Download the full PDF report from the workflow artifacts.
…ns' into fix/header-info-email # Conflicts: # lib/Db/MagicMapper/MagicSearchHandler.php # lib/Listener/MailAppScriptListener.php # src/modals/webhook/EditWebhook.vue
The body-mount fix from 04948cc was accidentally reverted during a merge conflict resolution, causing the sidebar to disappear whenever Mail's Vue root re-renders. Restored by mounting the Vue app via $mount() (no el:) and appending to document.body. Gate on the presence of #initial-state-mail-accounts so the sidebar only injects on Mail app pages without depending on transient DOM that Mail's Vue owns. Added fixed-position CSS on .or-mail-sidebar (higher specificity than NcAppSidebar's .app-sidebar default so the panel floats on the right with rounded corners, 8px margins and the classic NC chrome. Also wires up useAttachmentDrag composable: patches Mail's MessageAttachment DOM elements with draggable=true and a dragstart listener that writes attachment metadata (messageId, attachmentId, fileName, mime, size, downloadUrl) into dataTransfer under the application/x-nc-mail-attachment MIME type. ObjectsTab already consumes this payload and uploads the file to the dropped object via /filesMultipart. Upstream PR to make Mail attachments natively draggable tracked separately. Seed script gained an extra email with three real attachments (2 PDFs + 1 PNG) matching ZK-2026-0142 so the drag-drop flow has demo data out of the box.
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ❌ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 147/147 | |||
| npm | ✅ | ✅ 599/599 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-04-22 12:16 UTC
Download the full PDF report from the workflow artifacts.
rubenvdlinde
changed the title
Specter-generated spec directories that were sitting untracked. Each contains the standard openspec scaffold (proposal.md, design.md, tasks.md, specs/, hydra.json) for a future implementation cycle. The 21 specs include: - 18 integration-* specs (calendar, contacts, deck, email, photos, etc.) - 3 standalone: cleanup-linked-entity-type-map, fix-date-histogram-mariadb, seed-related-items Several of these will need re-splitting after market-intelligence#30 landed (MAX_FEATURES_PER_SPEC: 100→15). The companion preflight in hydra#196 will block oversized ones at builder dispatch with a clear "spec too large" comment.
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ❌ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ✅ | ||||
| composer | ✅ | ✅ 147/147 | |||
| npm | ✅ | ✅ 599/599 | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-04-25 15:06 UTC
Download the full PDF report from the workflow artifacts.
# Conflicts: # src/mail-sidebar.js # src/mail-sidebar/MailSidebar.vue
🔴 Blocker — Base branch is
|
|
🟡 Concern — Email typo count in PR description is wrong — diff shows 243, PR claims 232 The PR description states '232 occurrences'. Grepping the diff shows 243 removed lines with Suggested fix: Update the PR description to state 243 occurrences. The fix itself is complete and correct — no old typos remain, no new code reintroduces the typo. |
|
🟡 Concern — Email typo replacement is complete and correct 243 lines removing |
WilcoLouwerse
left a comment
WilcoLouwerse
left a comment
There was a problem hiding this comment.
Strict-mode review — scope-limited (PR is too large for exhaustive review). Focused on mail-sidebar surfaces, MailAppScriptListener, routes, CSS, seed scripts, and email-typo sweep. 237 of 448 files are single-line email-typo changes. 106 added files are openspec/ spec docs or docker/ seeds and are skipped in detail.
Top blockers:
- 🔴 Base branch is
beta, notdevelopment - 🔴 Vue 2 template has two root elements — will fail t
- 🔴 PR description claims ObjectsTab accepts attachmen
- 🔴 Mount guard breaks when sidebar starts collapsed —
Strict mode requires REQUEST_CHANGES on any 🔴 or 🟡. Please address blockers and concerns before merge.
Resolve the requested blockers by restoring a Vue 2 single-root template, hardening sidebar mount deduplication, and implementing object-card drop uploads for mail attachments. Also make seed-cases idempotent and environment-resilient with API-based ID discovery plus overrides, and add coverage for attachment drag behavior. Co-authored-by: Cursor <cursoragent@cursor.com>
| @@ -42,6 +96,7 @@ export default { | |||
| return { | |||
| objects: [], | |||
| loading: false, | |||
| uploadingObjectUuid: null, | |||
There was a problem hiding this comment.
🟢 Minor — uploadingObjectUuid data property is dead state
The new uploadingObjectUuid is set to obj.uuid during onAttachmentDrop and cleared in finally (lines 175, 182), but it is never referenced in the template — no :class, :disabled, or v-if binding consumes it. Either wire it to a per-card 'Uploading…' affordance (e.g. :class="{ 'or-mail-object-card--uploading': uploadingObjectUuid === obj.uuid }" plus a spinner) or drop the property — right now it adds reactivity overhead with no UX payoff. The showSuccess / showError toasts already cover post-upload feedback.
WilcoLouwerse
left a comment
WilcoLouwerse
left a comment
There was a problem hiding this comment.
All 3 prior 🔴 blockers verified fixed in 2c5b838 (Vue 2 single-root wrapper, ObjectsTab drop handlers, mount guard with stable ID). 8 of 11 🟡 concerns properly addressed; 1 reasonable scope deferral on the !important CSS. Two AuditTrailMapper concerns remain open with invalid deferral reasoning — getProcessingActivities() and findByIdentifier() are net-new methods added in this PR (+90 lines), not pre-existing code, so the perf risks (unbounded SELECT * and unindexed LIKE '%…%') are introduced here and worth tracking. One minor follow-up on dead state. Note: required CI check branch-protection / check-branch is failing — the source fix/header-info-email may need to retarget development instead of beta per the standard ConductionNL flow.
2 participants
Summary
Branch started life as a one-line docblock email typo fix but grew to restore the mail sidebar after a merge regression. All changes are additive — no existing feature removed.
04948ccc2(silently reverted in an earlier merge) is back; sidebar now survives Mail's Vue re-renders. Fixed-position CSS with higher specificity than.app-sidebarso the NcAppSidebar floats on the right with the classic NC chrome (rounded corners, 8px margins, box-shadow).useAttachmentDragcomposable patches Mail'sMessageAttachmentDOM at runtime (native Mail attachments aren't draggable) and writes{messageId, attachmentId, fileName, mime, size, downloadUrl}into dataTransfer underapplication/x-nc-mail-attachment. ObjectsTab already accepts the drop and uploads via/api/objects/.../filesMultipart.$response->getApp() === 'mail') instead of matchingOCA\\Mail\\in the event class name (the core event class never contains that string, so the sidebar script was never actually injected before).docker/mail/seed-cases.shcreates 2 caseTypes + 2 cases that match the mails fromseed-mail.sh, plus an extra attachment-bearing email so the drag-drop flow has demo data out of the box.dev@conductio.nl → info@conduction.nl(root-cause fix in fix(headers): correct email + add SPDX + @spec example nextcloud-app-template#19).decideskinto the nextcloud container.Test plan
bash ../openregister/docker/mail/seed-mail.sh && bash ../openregister/docker/mail/seed-cases.shon a fresh env — produces 12 emails + 2 casescomposer check:strict+npm run lintpass