Skip to content

Bulk invite collaborators#9987

Open
BacLuc wants to merge 3 commits into
ecamp:develfrom
BacLuc:bulk-invite-collaborators
Open

Bulk invite collaborators#9987
BacLuc wants to merge 3 commits into
ecamp:develfrom
BacLuc:bulk-invite-collaborators

Conversation

@BacLuc

@BacLuc BacLuc commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

feat: add bulk camp invitation endpoint and frontend component

Adds POST /camp_collaborations/bulk_invite to invite multiple people
to a camp at once by providing a list of email addresses.

  • BulkInvite DTO entity with camp, inviteEmails, role fields
  • BulkInviteProcessor handles creating CampCollaborations for each email,
    skipping emails that are already collaborators
  • CollaboratorBulkCreate.vue frontend component with textarea for emails
  • Translations for de, en, fr, it, rm locales

Fixes #8045

BacLuc and others added 3 commits June 9, 2026 21:29
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: BacLuc <lucius.bachmann@clubpage.ch>
As it was done in other components.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: BacLuc <lucius.bachmann@clubpage.ch>
Adds POST /camp_collaborations/bulk_invite to invite multiple people
to a camp at once by providing a list of email addresses.

- BulkInvite DTO entity with camp, inviteEmails, role fields
- BulkInviteProcessor handles creating CampCollaborations for each email,
skipping emails that are already collaborators
- CollaboratorBulkCreate.vue frontend component with textarea for emails
- Translations for de, en, fr, it, rm locales

Fixes ecamp#8045

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: BacLuc <lucius.bachmann@clubpage.ch>
@BacLuc BacLuc requested a review from a team June 9, 2026 20:26
@BacLuc

BacLuc commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review this pr

@pmattmann pmattmann added the deploy! Creates a feature branch deployment for this PR label Jun 10, 2026
@pmattmann pmattmann temporarily deployed to feature-branch June 10, 2026 04:47 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Jun 10, 2026

Copy link
Copy Markdown

Feature branch deployment ready!

Name Link
😎 Deployment https://pr9987.ecamp3.ch/
🔑 Login test@example.com / test
🕒 Last deployed at Wed Jun 10 2026 06:48:40 GMT+0200
🔨 Latest commit a937281997bb0c00e69dda591a7323fd8b830941
🔍 Latest deploy log https://github.com/ecamp/ecamp3/actions/runs/27253764924/job/80483870667
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

@BacLuc Sure, I'll review the PR right away!

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

Release Notes

  • New Features

    • Added bulk camp collaborator invitations—invite multiple people at once via email addresses.
    • New invite dialog with email list input and role selection.
    • Validates email format, list size (1–100), and detects already-invited addresses.
  • Tests

    • Added API and end-to-end tests for bulk invite workflows.
  • Localization

    • Bulk invite interface now available in English, German, French, Italian, and Rumantsch.

Walkthrough

This PR delivers end-to-end bulk camp collaboration invitations. The backend introduces a validated DTO and processor to handle multiple email invitations via /camp_collaborations/bulk_invite, with authorization checks and deduplication. The frontend provides a dialog UI for email entry and role selection, refactors role selection into a reusable component, and integrates the feature into the admin collaborators view. All changes are tested (API, E2E) and localized in five languages.

Changes

Bulk Invite Camp Collaborators

Layer / File(s) Summary
API DTO contract and validation
api/src/DTO/BulkInvite.php
BulkInvite DTO defines the request shape (camp IRI, inviteEmails array with 1–100 count, role with ROLE_MEMBER default) and response shape (createdCampCollaborations and alreadyInCamp arrays) with full validation groups.
API processor bulk invite logic
api/src/State/BulkInviteProcessor.php
BulkInviteProcessor resolves the camp, enforces CAMP_MEMBER/CAMP_MANAGER authorization, iterates emails to deduplicate (by user or by invite email), creates CampCollaboration in INVITED status with hashed invite key, associates MaterialList, flushes, and triggers invite email.
API tests and OpenAPI spec
api/tests/Api/CampCollaborations/BulkInviteTest.php, api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testOpenApiSpecMatchesSnapshot__1.yml
Tests cover access control (401/403 for anon/guest/unrelated), role-based access (201 for member/manager), behavior (multiple creates, skip duplicates by user and by email), and validation (required camp, email format, role choice, non-empty inviteEmails). OpenAPI snapshot documents the new schemas, POST endpoint, and content-type definitions.
Frontend role-select component extraction
frontend/src/components/collaborator/CollaboratorRoleSelect.vue, frontend/src/components/collaborator/CollaboratorForm.vue
New CollaboratorRoleSelect component encapsulates role picker with translated labels/icons and two-way binding. CollaboratorForm refactored to use it, removing inline e-select and items() computed.
Frontend bulk-invite dialog component
frontend/src/components/collaborator/CollaboratorBulkCreate.vue
CollaboratorBulkCreate dialog parses email input (split on newlines/commas/semicolons), maintains state for loading/error/result, calls bulk invite API with camp, parsed emails, and role, reloads camp collaborations on success, and displays created and already-in-camp counts.
Frontend admin view and form integration
frontend/src/views/camp/admin/Collaborators.vue, frontend/src/components/form/base/ETextarea.vue
Adds manager-conditional CollaboratorBulkCreate button in title-actions. Fixes ETextarea validation name fallback to include validationLabel.
Localization for bulk-invite UI
frontend/src/locales/de.json, frontend/src/locales/en.json, frontend/src/locales/fr.json, frontend/src/locales/it.json, frontend/src/locales/rm.json
Adds collaboratorBulkCreate translation blocks in all five locales with strings for title, email input guidance, invite CTA, success message with {count}, and already-in-camp notice.
End-to-end behavior tests
e2e/tests/9-behavior-tests/bulkInvite.spec.ts
Playwright tests verify dialog open/close and reset, email parsing and submission, success alert with counts, already-invited detection, error handling with form state preservation, and cancel via Escape.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • simfeld
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title 'Bulk invite collaborators' is concise and directly describes the primary change introduced in the PR.
Description check ✅ Passed Description clearly outlines the bulk invitation feature with API endpoint, DTO, processor, frontend component, and translations.
Linked Issues check ✅ Passed The PR fully implements the requirement from #8045: ability to invite multiple email addresses to a camp at once.
Out of Scope Changes check ✅ Passed All changes relate to the bulk invitation feature and supporting refactoring. CollaboratorRoleSelect extraction and ETextarea fix are reasonable supporting changes.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (3)
frontend/src/components/collaborator/CollaboratorBulkCreate.vue (1)

29-35: ⚖️ Poor tradeoff

Consider client-side email validation for better UX.

The textarea accepts freeform text with no format validation. Invalid email addresses will only be rejected by the API, requiring a round-trip. Consider adding vee-validate rules or inline hints to catch format errors earlier.

For example, you could add a computed property that validates parsed emails and displays warnings before submission.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/collaborator/CollaboratorBulkCreate.vue` around lines
29 - 35, The textarea bound to emailsText in CollaboratorBulkCreate.vue lacks
client-side validation; add a computed property (e.g., validatedEmails and
invalidEmails) that parses emailsText into individual addresses, validates each
with a simple email regex or vee-validate rule, and expose invalidEmails to the
template as inline warnings (VAlert, helper text or per-line error) and to
disable the submit action; update the component's submit handler (e.g.,
onBulkCreate/createCollaborators) to check validatedEmails before calling the
API so only well-formed addresses are sent.
e2e/tests/9-behavior-tests/bulkInvite.spec.ts (1)

15-16: ⚡ Quick win

Use stable selectors instead of localized button/field text in E2E.

These assertions are tightly coupled to German labels, so locale/default-language changes can break tests without behavioral regressions. Prefer data-testid (or similarly stable selectors) for dialog open/submit/textbox/role controls.

Also applies to: 20-20, 26-27, 40-40, 83-83, 89-90, 95-96, 104-105

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e/tests/9-behavior-tests/bulkInvite.spec.ts` around lines 15 - 16, The test
uses localized visible text (e.g., the string 'Mehrere Personen einladen' used
in page.getByRole(...)) which is brittle; update the selectors across the spec
(all places using page.getByRole or page.getByLabelText with localized German
strings) to use stable attributes like data-testid or aria-testids instead
(e.g., replace page.getByRole('button', { name: 'Mehrere Personen einladen' })
and similar selectors for dialog open/submit buttons, textboxes and role
controls with page.locator('[data-testid="bulk-invite-button"]') /
page.getByTestId('...') equivalents); ensure the app adds matching data-testid
attributes for the dialog open, submit, textbox and role controls so the E2E
references (open, submit, input, role selectors) are stable across locales.
api/tests/Api/CampCollaborations/BulkInviteTest.php (1)

81-127: ⚡ Quick win

Add a same-request duplicate-email regression test.

Nothing here exercises a payload like ['dup@example.com', ' DUP@example.com ']. That case is easy to mis-handle, and a focused test asserting dedup-before-persist would lock down the API contract.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@api/tests/Api/CampCollaborations/BulkInviteTest.php` around lines 81 - 127,
Add a new test method (e.g.,
testBulkInviteDedupsSameRequestDifferentCaseAndWhitespace) that posts to
/camp_collaborations/bulk_invite via createClientWithCredentials() with JSON
inviteEmails like ['dup@example.com', ' DUP@example.com '] and the same camp IRI
from getIriFor('camp1'), then assert the response status is 201,
response->toArray() shows only one item in createdCampCollaborations and zero
items in alreadyInCamp (or that alreadyInCamp does not contain the normalized
email); this ensures dedup-before-persist behavior for case/whitespace variants.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@api/src/State/BulkInviteProcessor.php`:
- Around line 63-64: The code currently flushes each created collaboration and
then calls sendInviteEmail, which allows partial commits if a later email send
fails; wrap the whole batch in a single DB transaction in BulkInviteProcessor
(the loop that calls createCollaboration and sendInviteEmail), stop calling
flush() per-invite and instead persist entities only, collect created
collaborations, commit (flush) once at the end, then (after successful commit)
resolve IRIs with iriConverter->getIriFromResource and call sendInviteEmail (or
enqueue emails asynchronously); ensure rollback on failure so no partial commits
occur.
- Around line 54-58: The loop over $data->inviteEmails normalizes each address
and then checks isAlreadyInCamp($camp, $email), which causes intra-request
duplicates to be treated as "already in camp"; to fix, deduplicate the
normalized invite list before any persistence or isAlreadyInCamp checks by
building a normalized set (e.g., lowercased/trimmed values) and iterating only
unique addresses, or track a local $seen array inside the processing logic to
skip duplicates; update references to $data->inviteEmails processing (the
foreach block that normalizes values and populates $data->alreadyInCamp) so only
unique normalized emails are checked and added.

In
`@api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testOpenApiSpecMatchesSnapshot__1.yml`:
- Around line 3940-3951: The snapshot shows inviteEmails array items as "null |
string" without an email format; update the DTO/property metadata for
inviteEmails so its elements are non-null email strings (e.g., replace nullable
item typing with a plain string type and add an email format
annotation/decorator that your OpenAPI generator recognizes), ensure the
property still enforces minItems/maxItems, then regenerate the OpenAPI
spec/snapshots so items appear as type: string with format: email instead of
allowing nulls.

In `@frontend/src/components/collaborator/CollaboratorBulkCreate.vue`:
- Line 10: The submit-enable check uses emailsText.length which counts
whitespace; change the condition to use the parsed/validated list instead—use
parsedEmails.length > 0 (or compute a trimmed non-empty check like
emailsText.trim() and/or reuse the existing parsedEmails computed property) so
the button only enables when there is at least one non-empty parsed email;
update the binding that references :submit-enabled to use parsedEmails (or a new
computed like hasValidEmails) and ensure parsedEmails is the same computed that
trims/splits/filters empty strings.
- Line 122: The debug console.log(err) call should be removed from the
bulk-create error handling; locate the console.log inside the
CollaboratorBulkCreate.vue method handling the bulk create (the catch/error
branch where console.log(err) is present) and either delete it or replace it
with the component's production logging/error-handling mechanism (e.g.,
this.$emit('error', err) or this.$logger.error(...)/processLogger.error(...)) so
errors are recorded appropriately instead of printed to the console.

In `@frontend/src/components/collaborator/CollaboratorRoleSelect.vue`:
- Around line 7-9: The select is using wrong prop names—change item-title="role"
to item-title="text" and item-value="key" to item-value="value" in the
CollaboratorRoleSelect.vue template (the component that renders the
select/dropdown) so it matches the items array shape (items use text and value),
and leave the existing vee-rules attribute as-is; update those attributes on the
select component element to restore correct display and stored values.
- Line 43: The prop definition for modelValue in CollaboratorRoleSelect.vue uses
an empty-string default which is invalid per backend role contract (allowed:
'guest'|'member'|'manager'); update the prop to either set a valid default
(e.g., 'member' to align with DEFAULT_BULK_INVITE_ROLE used in
CollaboratorBulkCreate) or make modelValue required so callers must supply one;
change the modelValue prop declaration accordingly and ensure any parent
components that rely on the previous empty default are updated to pass a valid
role.

---

Nitpick comments:
In `@api/tests/Api/CampCollaborations/BulkInviteTest.php`:
- Around line 81-127: Add a new test method (e.g.,
testBulkInviteDedupsSameRequestDifferentCaseAndWhitespace) that posts to
/camp_collaborations/bulk_invite via createClientWithCredentials() with JSON
inviteEmails like ['dup@example.com', ' DUP@example.com '] and the same camp IRI
from getIriFor('camp1'), then assert the response status is 201,
response->toArray() shows only one item in createdCampCollaborations and zero
items in alreadyInCamp (or that alreadyInCamp does not contain the normalized
email); this ensures dedup-before-persist behavior for case/whitespace variants.

In `@e2e/tests/9-behavior-tests/bulkInvite.spec.ts`:
- Around line 15-16: The test uses localized visible text (e.g., the string
'Mehrere Personen einladen' used in page.getByRole(...)) which is brittle;
update the selectors across the spec (all places using page.getByRole or
page.getByLabelText with localized German strings) to use stable attributes like
data-testid or aria-testids instead (e.g., replace page.getByRole('button', {
name: 'Mehrere Personen einladen' }) and similar selectors for dialog
open/submit buttons, textboxes and role controls with
page.locator('[data-testid="bulk-invite-button"]') / page.getByTestId('...')
equivalents); ensure the app adds matching data-testid attributes for the dialog
open, submit, textbox and role controls so the E2E references (open, submit,
input, role selectors) are stable across locales.

In `@frontend/src/components/collaborator/CollaboratorBulkCreate.vue`:
- Around line 29-35: The textarea bound to emailsText in
CollaboratorBulkCreate.vue lacks client-side validation; add a computed property
(e.g., validatedEmails and invalidEmails) that parses emailsText into individual
addresses, validates each with a simple email regex or vee-validate rule, and
expose invalidEmails to the template as inline warnings (VAlert, helper text or
per-line error) and to disable the submit action; update the component's submit
handler (e.g., onBulkCreate/createCollaborators) to check validatedEmails before
calling the API so only well-formed addresses are sent.
🪄 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: cc1725f0-429a-4c0c-8bed-567ca25bdeb6

📥 Commits

Reviewing files that changed from the base of the PR and between aab09ff and a937281.

📒 Files selected for processing (15)
  • api/src/DTO/BulkInvite.php
  • api/src/State/BulkInviteProcessor.php
  • api/tests/Api/CampCollaborations/BulkInviteTest.php
  • api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testOpenApiSpecMatchesSnapshot__1.yml
  • e2e/tests/9-behavior-tests/bulkInvite.spec.ts
  • frontend/src/components/collaborator/CollaboratorBulkCreate.vue
  • frontend/src/components/collaborator/CollaboratorForm.vue
  • frontend/src/components/collaborator/CollaboratorRoleSelect.vue
  • frontend/src/components/form/base/ETextarea.vue
  • frontend/src/locales/de.json
  • frontend/src/locales/en.json
  • frontend/src/locales/fr.json
  • frontend/src/locales/it.json
  • frontend/src/locales/rm.json
  • frontend/src/views/camp/admin/Collaborators.vue

Comment on lines +54 to +58
foreach ($data->inviteEmails as $email) {
$email = strtolower(trim($email));

if ($this->isAlreadyInCamp($camp, $email)) {
$data->alreadyInCamp[] = $email;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Deduplicate the normalized email list before hitting persistence.

A duplicate address in the same payload is currently pushed into alreadyInCamp on its second occurrence, because the first iteration has already created it. That makes the response say “already in camp” for addresses that only duplicated within this request.

♻️ Proposed fix
-        foreach ($data->inviteEmails as $email) {
-            $email = strtolower(trim($email));
+        $inviteEmails = array_values(array_unique(array_map(
+            static fn (string $email): string => strtolower(trim($email)),
+            $data->inviteEmails,
+        )));
+
+        foreach ($inviteEmails as $email) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
foreach ($data->inviteEmails as $email) {
$email = strtolower(trim($email));
if ($this->isAlreadyInCamp($camp, $email)) {
$data->alreadyInCamp[] = $email;
$inviteEmails = array_values(array_unique(array_map(
static fn (string $email): string => strtolower(trim($email)),
$data->inviteEmails,
)));
foreach ($inviteEmails as $email) {
if ($this->isAlreadyInCamp($camp, $email)) {
$data->alreadyInCamp[] = $email;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@api/src/State/BulkInviteProcessor.php` around lines 54 - 58, The loop over
$data->inviteEmails normalizes each address and then checks
isAlreadyInCamp($camp, $email), which causes intra-request duplicates to be
treated as "already in camp"; to fix, deduplicate the normalized invite list
before any persistence or isAlreadyInCamp checks by building a normalized set
(e.g., lowercased/trimmed values) and iterating only unique addresses, or track
a local $seen array inside the processing logic to skip duplicates; update
references to $data->inviteEmails processing (the foreach block that normalizes
values and populates $data->alreadyInCamp) so only unique normalized emails are
checked and added.

Comment on lines +63 to +64
$collaboration = $this->createCollaboration($camp, $email, $data->role);
$data->createdCampCollaborations[] = $this->iriConverter->getIriFromResource($collaboration);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

This bulk endpoint can commit a partial batch and still fail.

Each invite is flush()ed before sendInviteEmail() runs. If one mail send throws, earlier collaborations stay committed and the request returns 500, so a retry turns a failed bulk invite into a partial success. Persist the whole batch in one transaction and send mail after commit or asynchronously.

Also applies to: 106-115

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@api/src/State/BulkInviteProcessor.php` around lines 63 - 64, The code
currently flushes each created collaboration and then calls sendInviteEmail,
which allows partial commits if a later email send fails; wrap the whole batch
in a single DB transaction in BulkInviteProcessor (the loop that calls
createCollaboration and sendInviteEmail), stop calling flush() per-invite and
instead persist entities only, collect created collaborations, commit (flush)
once at the end, then (after successful commit) resolve IRIs with
iriConverter->getIriFromResource and call sendInviteEmail (or enqueue emails
asynchronously); ensure rollback on failure so no partial commits occur.

Comment on lines +3940 to +3951
inviteEmails:
description: 'List of email addresses to invite.'
example:
- another@example.com
- invited@example.com
items:
type:
- 'null'
- string
maxItems: 100
minItems: 1
type: array

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Document inviteEmails as non-null email strings.

Line 3946 currently declares each entry as string | null and never marks it as an email, so generated clients will treat inputs like [null, "not-an-email"] as schema-valid. That weakens the published API contract for a field described as a list of email addresses and can drift from the server-side validation/tests. Please tighten the DTO metadata so the regenerated write schema emits non-null type: string items with format: email.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@api/tests/Api/SnapshotTests/__snapshots__/ResponseSnapshotTest__testOpenApiSpecMatchesSnapshot__1.yml`
around lines 3940 - 3951, The snapshot shows inviteEmails array items as "null |
string" without an email format; update the DTO/property metadata for
inviteEmails so its elements are non-null email strings (e.g., replace nullable
item typing with a plain string type and add an email format
annotation/decorator that your OpenAPI generator recognizes), ensure the
property still enforces minItems/maxItems, then regenerate the OpenAPI
spec/snapshots so items appear as type: string with format: email instead of
allowing nulls.

:title="$t('components.collaborator.collaboratorBulkCreate.title')"
:submit-action="bulkInvite"
:submit-label="$t('components.collaborator.collaboratorBulkCreate.invite')"
:submit-enabled="emailsText.length > 0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Submit button enables on whitespace-only input.

Line 10 checks emailsText.length > 0, so entering only whitespace/newlines will enable the button. But parsedEmails (lines 83-88) filters empty strings, so the submission would send an empty array and fail backend validation (min: 1).

🐛 Proposed fix
     :submit-action="bulkInvite"
     :submit-label="$t('components.collaborator.collaboratorBulkCreate.invite')"
-    :submit-enabled="emailsText.length > 0"
+    :submit-enabled="parsedEmails.length > 0"
     submit-icon="mdi-email-fast"
     submit-color="success"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
:submit-enabled="emailsText.length > 0"
:submit-action="bulkInvite"
:submit-label="$t('components.collaborator.collaboratorBulkCreate.invite')"
:submit-enabled="parsedEmails.length > 0"
submit-icon="mdi-email-fast"
submit-color="success"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/collaborator/CollaboratorBulkCreate.vue` at line 10,
The submit-enable check uses emailsText.length which counts whitespace; change
the condition to use the parsed/validated list instead—use parsedEmails.length >
0 (or compute a trimmed non-empty check like emailsText.trim() and/or reuse the
existing parsedEmails computed property) so the button only enables when there
is at least one non-empty parsed email; update the binding that references
:submit-enabled to use parsedEmails (or a new computed like hasValidEmails) and
ensure parsedEmails is the same computed that trims/splits/filters empty
strings.

this.emailsText = ''
await this.api.reload(this.camp.campCollaborations())
} catch (err) {
console.log(err)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove debug console.log before production.

The console.log(err) on line 122 is a debug artifact. Remove it or replace with proper error logging if needed for production diagnostics.

🧹 Proposed fix
       await this.api.reload(this.camp.campCollaborations())
     } catch (err) {
-      console.log(err)
       this.error = err
     } finally {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log(err)
await this.api.reload(this.camp.campCollaborations())
} catch (err) {
this.error = err
} finally {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/collaborator/CollaboratorBulkCreate.vue` at line 122,
The debug console.log(err) call should be removed from the bulk-create error
handling; locate the console.log inside the CollaboratorBulkCreate.vue method
handling the bulk create (the catch/error branch where console.log(err) is
present) and either delete it or replace it with the component's production
logging/error-handling mechanism (e.g., this.$emit('error', err) or
this.$logger.error(...)/processLogger.error(...)) so errors are recorded
appropriately instead of printed to the console.

Comment on lines +7 to +9
item-title="role"
item-value="key"
vee-rules="required"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Critical: Incorrect property names break role selection.

Lines 7-8 reference properties that don't exist in the items array:

  • Line 7: item-title="role" but items use text property (lines 59, 65, 71)
  • Line 8: item-value="key" but items use value property (lines 58, 64, 70)

This will cause the select to display and store incorrect values.

🐛 Proposed fix
     path="role"
     :items="items"
     persistent-hint
-    item-title="role"
-    item-value="key"
+    item-title="text"
+    item-value="value"
     vee-rules="required"
     v-bind="$attrs"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
item-title="role"
item-value="key"
vee-rules="required"
item-title="text"
item-value="value"
vee-rules="required"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/collaborator/CollaboratorRoleSelect.vue` around lines
7 - 9, The select is using wrong prop names—change item-title="role" to
item-title="text" and item-value="key" to item-value="value" in the
CollaboratorRoleSelect.vue template (the component that renders the
select/dropdown) so it matches the items array shape (items use text and value),
and leave the existing vee-rules attribute as-is; update those attributes on the
select component element to restore correct display and stored values.

export default {
name: 'CollaboratorRoleSelect',
props: {
modelValue: { type: String, required: false, default: '' },

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Invalid default role value.

The modelValue default is an empty string, but the backend contract restricts role to guest, member, or manager. An empty string will fail API validation if submitted.

Consider defaulting to a valid role (e.g., 'member' to match DEFAULT_BULK_INVITE_ROLE in CollaboratorBulkCreate) or making the prop explicitly required.

🛡️ Proposed fix (option 1: sensible default)
   props: {
-    modelValue: { type: String, required: false, default: '' },
+    modelValue: { type: String, required: false, default: 'member' },
   },
🛡️ Proposed fix (option 2: require explicit value)
   props: {
-    modelValue: { type: String, required: false, default: '' },
+    modelValue: { type: String, required: true },
   },
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
modelValue: { type: String, required: false, default: '' },
modelValue: { type: String, required: false, default: 'member' },
Suggested change
modelValue: { type: String, required: false, default: '' },
modelValue: { type: String, required: true },
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@frontend/src/components/collaborator/CollaboratorRoleSelect.vue` at line 43,
The prop definition for modelValue in CollaboratorRoleSelect.vue uses an
empty-string default which is invalid per backend role contract (allowed:
'guest'|'member'|'manager'); update the prop to either set a valid default
(e.g., 'member' to align with DEFAULT_BULK_INVITE_ROLE used in
CollaboratorBulkCreate) or make modelValue required so callers must supply one;
change the modelValue prop declaration accordingly and ensure any parent
components that rely on the previous empty default are updated to pass a valid
role.

@carlobeltrame carlobeltrame left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer this to be implemented in the frontend.

  • Sending emails can take some time, and this could require a job queue if done on the server side.
  • Almost all of our endpoints are pure REST; this one is another RPC-style endpoint (evident by the necessity of a DTO).
  • The whole operation feels like it is necessitated by a UI requirement, not a data requirement. We have previously rejected storing frontend configuration in the backend, in order to keep frontend and backend clearly separated.
  • Doing it in the frontend, error reporting and handling of individual failures is easier.
  • Doing it with AI, this shouldn't be too much effort to redo I hope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deploy! Creates a feature branch deployment for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invite multiple people to a Camp at once

3 participants