ECHO-43 Project report notification feature#90
Conversation
- Updated the Docker Compose configuration to include email transport settings. - Enhanced the ProjectPortalEditor to allow enabling/disabling project notification subscriptions. - Implemented email subscription management in the ParticipantPostConversation component, allowing users to add and validate email addresses for notifications. - Added new hooks in the query library for checking subscription status and updating participant email preferences. - Updated the ProjectReportRoute to confirm email notifications before publishing reports. - Introduced new types for project report notification participants in TypeScript definitions. - Adjusted the backend to handle email notification status during report creation. This feature allows participants to subscribe to notifications for project report updates, improving user engagement and communication.
- Wrapped the request for fetching participant IDs in a try-catch block to handle potential errors during the unsubscribe process. - Added console error logging for better debugging and user feedback when the unsubscribe operation fails. - Ensured that a clear error message is thrown if the subscription status update fails, improving overall reliability of the feature.
- Implemented a new API endpoint to handle participant unsubscription, allowing updates to email preferences securely. - Created a new function in the frontend to manage the unsubscribe process, integrating it with the existing query hooks. - Removed the deprecated useUpdateProjectContacts hook to streamline the unsubscribe functionality. - Enhanced the ProjectUnsubscribe component to utilize the new API for managing participant email preferences, improving user experience and engagement.
…ation - Updated the unsubscribe button to utilize the Trans component for improved internationalization support. - Added a loading prop to the button for better user feedback during the unsubscribe process.
…scribe features - Added new translations for various UI elements related to participant interactions, including buttons and notifications in German, English, Spanish, French, and Dutch. - Enhanced user feedback messages for actions such as submitting responses and unsubscribing from notifications. - Updated existing translations to ensure consistency and clarity across all supported languages.
- Introduced new email notification flows for sending report updates, including "Send Email" and "Send Report Emails" operations. - Implemented various conditions and triggers for email notifications based on project report status and language preferences. - Added permissions for managing project report notifications, allowing for create, read, and update actions. - Updated the schema for the 'quote' collection to change the embedding field type to 'text' and adjusted its display properties. - Removed the deprecated 'centroid_embedding' field from the 'aspect' collection, streamlining the data structure.
|
""" WalkthroughThe changes enhance the email notification capabilities across the system. SMTP settings are added to the Directus service configuration, and new JSON configurations establish flows, operations, permissions, and snapshot schemas for email notifications and project report participant management. Frontend components are updated with routes, forms, and hooks to manage email subscriptions and unsubscriptions, including modal confirmations during report publishing. A new API endpoint and associated server logic support unsubscribing participants, while localization files are updated across multiple languages. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant C as ProjectUnsubscribe Component
participant Q as useCheckUnsubscribeStatus Hook
participant M as useMutation (unsubscribe)
participant A as API (unsubscribeParticipant)
participant S as Server Endpoint
participant DB as Database
U->>C: Access unsubscribe URL with token & project_id
C->>Q: Validate unsubscription token and project_id
Q-->>C: Return validation result
U->>C: Clicks "Unsubscribe" button
C->>M: Trigger unsubscribe mutation
M->>A: Call API with payload (project_id, token, email_opt_in)
A->>S: Forward PATCH /projects/{project_id}/contacts/unsubscribe
S->>DB: Query and update participant records
DB-->>S: Return update result
S-->>A: Success response
A-->>M: Success response
M-->>C: Mutation resolved (success)
C->>U: Display success message ("You have successfully unsubscribed.")
sequenceDiagram
participant U as User
participant R as ProjectReportRoute Component
participant P as useGetProjectParticipants Hook
participant M as Modal Confirmation
participant H as handleConfirmPublish function
participant API as Reporting API
U->>R: Toggle Publish Switch
R->>P: Fetch count of project participants
P-->>R: Return participant count
alt Participants Exist
R->>M: Open confirmation modal with count
U->>M: Click "Proceed"
M->>H: Confirm publish action
else No Participants
R->>H: Publish report directly
end
H->>API: Execute report update with new status ("archived")
API-->>H: Return update result
H-->>R: Update UI state and close modal
Assessment against linked issues
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
echo/directus/sync/snapshot/fields/quote/embedding.json (1)
28-43: 💡 Verification agent🧩 Analysis chain
Data type change aligns with field type
Updating the data_type from "vector" to "text" ensures consistency between the field type and underlying storage.
🌐 Web query:
What is the appropriate data type for storing vector embeddings in SQL databases?💡 Result:
The appropriate data type for storing vector embeddings in SQL databases is the
VECTORdata type, a purpose-built feature now widely supported across major SQL platforms. This specialized type optimizes storage, indexing, and vector operations while maintaining compatibility with standard database workflows.Key features of
VECTORimplementations:
- Dimension specification:
Vectors require fixed dimensions (e.g.,VECTOR(768)for 768-dimensional embeddings), enforced during schema definition to ensure data integrity[7][12].- Storage optimization:
Uses efficient binary formats while exposing values as JSON arrays for readability[5][7][8].- Native operations:
Supports vector-specific functions like similarity calculations (VECTOR_DISTANCE) and casting utilities (string_to_vector)[4][5][10].Platform-specific implementations:
Database Syntax Example Key Features Cloud SQL/MySQL VECTOR(3) USING VARBINARYVertex AI/OpenAI integration, UPSERT support[1] SQL Server VECTOR(3)Cosine/Euclidean distance functions, JSON casting[5][7] PostgreSQL vector(768)(via pgvector extension)Hybrid search with metadata columns[2][9] Oracle VECTORwith DBMS_VECTOR_CHAINAutomatic embedding generation pipelines[4] Snowflake VECTOR(FLOAT, 768)Cortex embedding functions, ANN indexes[14] Advantages over alternatives like BLOB/text:
- Performance:
Native vector indexes (HNSW, IVFFlat) accelerate similarity searches by 10-100x compared to full scans[4][8][12].- Function support:
Enables direct use of vector math in SQL queries without data conversion[5][10][14].- Type safety:
Enforces dimension and data type constraints during insertion/updates[7][12].For legacy systems without native
VECTORsupport, common workarounds include:
- Storing embeddings as
VARBINARYwith schema-enforced dimensions[1]- Using JSON arrays in
TEXTcolumns with validation constraints[5][11]- Implementing custom similarity functions via extensions (e.g., pgvector)[2]
Modern implementations like SQL Server 2025 and Cloud SQL now combine native vector storage with integrated ML pipelines, enabling direct generation and querying of embeddings without ETL[4][7][14].
Citations:
- 1: https://cloud.google.com/sql/docs/mysql/generate-manage-vector-embeddings
- 2: https://dev.to/stephenc222/how-to-use-postgresql-to-store-and-query-vector-embeddings-h4b
- 3: https://www.pinecone.io/learn/vector-database/
- 4: https://docs.oracle.com/en/database/oracle/oracle-database/23/vecse/sql-quick-start-using-vector-embedding-model-uploaded-database.html
- 5: https://learn.microsoft.com/en-us/sql/relational-databases/vectors/vectors-sql-server?view=azuresqldb-current
- 6: https://aws.amazon.com/blogs/database/a-generative-ai-use-case-using-amazon-rds-for-sql-server-as-a-vector-data-store/
- 7: https://learn.microsoft.com/en-us/sql/t-sql/data-types/vector-data-type?view=azuresqldb-current
- 8: https://devblogs.microsoft.com/azure-sql/storing-querying-and-keeping-embeddings-updated-options-and-best-practices/
- 9: https://weaviate.io/blog/what-is-a-vector-database
- 10: https://www.red-gate.com/simple-talk/databases/sql-server/t-sql-programming-sql-server/ai-in-sql-server-2025-embeddings/
- 11: https://community.openai.com/t/which-database-tools-suit-for-storing-embeddings-generated-by-the-embedding-endpoint/23337
- 12: https://docs.singlestore.com/cloud/reference/sql-reference/data-types/vector-type/
- 13: https://community.openai.com/t/storing-embeddings-in-sql-server-latency-between-redis-pinecone-vector-db-recommendations/55134
- 14: https://docs.snowflake.com/en/user-guide/snowflake-cortex/vector-embeddings
- 15: https://docs.snowflake.com/en/sql-reference/data-types-vector
- 16: https://learn.microsoft.com/en-us/shows/data-exposed/azure-sql-db-vector-functions-private-preview-data-exposed
Attention: Update Data Type for Embeddings
The current change in
echo/directus/sync/snapshot/fields/quote/embedding.json—switching"data_type"from"vector"to"text"—does not align with best practices for storing embeddings. Recent research confirms that for vector embeddings, a dedicated SQLVECTORtype (with proper dimension specifications, e.g.,VECTOR(768)) is optimal. This data type offers better performance, native vector operations, and enhanced storage efficiency compared to using plain text.
- Action Required: Update the
"data_type"field to use the nativeVECTORtype (with dimensions defined if applicable).- Review Considerations: Verify compatibility with your SQL platform and adjust any associated schema constraints accordingly.
🧹 Nitpick comments (8)
echo/directus/sync/snapshot/fields/project_report_notification_participants/email.json (1)
1-43: This email field config is fire! 🔥Clean JSON schema for the email field in the notification participants collection. Based on the structure, this is a VARCHAR(255) field that's made nullable. The field is rendered as a standard input with half-width UI.
One small thing to improve: you might want to consider adding email validation in the
validationfield (currently null at line 21) to ensure proper email format, and possibly setis_indexed: truesince this field will likely be queried against frequently.echo/directus/sync/snapshot/fields/project_report_notification_participants/project_id.json (1)
1-44: Project ID Field is Solid!
Theproject_idfield is laid out clearly with a well-defined schema. One minor note: if every participant is meant to be tied to a specific project, you might eventually consider marking this field as required. If the intentional flexibility is desired, then it's all good. LGTM!echo/frontend/src/routes/project/unsubscribe/ProjectUnsubscribe.tsx (1)
12-46: DRY violation in mutation success callbackYou're defining the same success callback in two places - once in useMutation config and again in the mutate call. This redundancy isn't very 100x of you.
const { mutate, isPending } = useMutation({ mutationFn: ({ project_id, token, email_opt_in, }: { project_id: string; token: string; email_opt_in: boolean; }) => unsubscribeParticipant(project_id, token, email_opt_in), onSuccess: () => setSuccess(true), }); const handleUnsubscribe = () => { mutate( { token, project_id, email_opt_in: false }, - { - onSuccess: () => setSuccess(true), - }, ); };echo/frontend/src/routes/participant/ParticipantPostConversation.tsx (2)
76-80: Email validation regex could be more bulletproofYour regex will handle 99% of cases, but the email validation universe is more complex than most realize. For production environments, consider a more robust regex or a dedicated validation library.
Some edge cases your current regex might miss:
- IP addresses in domain part [127.0.0.1]
- International domains with IDNs
- TLDs longer than 4 chars
- const validateEmail = (email: string) => { - const emailRegex = - /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9-]+(?:\.[a-zA-Z0-9-]+)*\.[a-zA-Z]{2,}$/; - return emailRegex.test(email); - }; + const validateEmail = (email: string) => { + // RFC 5322 compliant regex + const emailRegex = /^(([^<>()\[\]\\.,;:\s@"]+(\.[^<>()\[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/; + return emailRegex.test(email); + };
183-309: Consider adding ARIA labels for better accessibilityThe UI implementation is solid but missing some accessibility enhancements. Screen reader users may have difficulty understanding the purpose of some interactive elements.
Add ARIA labels to improve accessibility:
<TextInput ref={inputRef} placeholder={t`email@work.com`} + aria-label={t`Email subscription input`} value={email} size="md" // ... other props /> <Button size="sm" variant="light" + aria-label={t`Add email to subscription list`} onClick={() => addEmail(inputRef.current)} // ... other props >echo/frontend/src/locales/fr-FR.po (2)
1485-1487: Good validation feedback translation!"This email is already in the list." → "Cette e-mail est déjà dans la liste." Note: There's a minor gender disagreement - "e-mail" is masculine in French, so it should be "Cet e-mail" instead of "Cette e-mail".
-msgstr "Cette e-mail est déjà dans la liste." +msgstr "Cet e-mail est déjà dans la liste."
1489-1491: Same gender issue in another message!"This email is already subscribed to notifications." → "Cette e-mail est déjà abonnée aux notifications." The gender issue appears again.
-msgstr "Cette e-mail est déjà abonnée aux notifications." +msgstr "Cet e-mail est déjà abonné aux notifications."echo/frontend/src/locales/en-US.po (1)
801-803: "loading..." indicator string. LGTM!Simple loading text for async operations. Consider standardizing loading states across the app - some use "Loading..." (line 807) while this uses lowercase "loading...".
-msgid "loading..." -msgstr "loading..." +msgid "Loading..." +msgstr "Loading..."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
echo/.devcontainer/docker-compose.yml(1 hunks)echo/directus/sync/collections/flows.json(1 hunks)echo/directus/sync/collections/operations.json(1 hunks)echo/directus/sync/collections/permissions.json(4 hunks)echo/directus/sync/snapshot/collections/project_report_notification_participants.json(1 hunks)echo/directus/sync/snapshot/fields/project/is_project_notification_subscription_allowed.json(1 hunks)echo/directus/sync/snapshot/fields/project_report_notification_participants/date_submitted.json(1 hunks)echo/directus/sync/snapshot/fields/project_report_notification_participants/date_updated.json(1 hunks)echo/directus/sync/snapshot/fields/project_report_notification_participants/email.json(1 hunks)echo/directus/sync/snapshot/fields/project_report_notification_participants/email_opt_in.json(1 hunks)echo/directus/sync/snapshot/fields/project_report_notification_participants/email_opt_out_token.json(1 hunks)echo/directus/sync/snapshot/fields/project_report_notification_participants/id.json(1 hunks)echo/directus/sync/snapshot/fields/project_report_notification_participants/project_id.json(1 hunks)echo/directus/sync/snapshot/fields/project_report_notification_participants/sort.json(1 hunks)echo/directus/sync/snapshot/fields/quote/embedding.json(3 hunks)echo/frontend/src/Router.tsx(2 hunks)echo/frontend/src/components/project/ProjectPortalEditor.tsx(3 hunks)echo/frontend/src/lib/api.ts(1 hunks)echo/frontend/src/lib/query.ts(3 hunks)echo/frontend/src/lib/typesDirectus.d.ts(3 hunks)echo/frontend/src/locales/de-DE.po(47 hunks)echo/frontend/src/locales/en-US.po(48 hunks)echo/frontend/src/locales/es-ES.po(46 hunks)echo/frontend/src/locales/fr-FR.po(46 hunks)echo/frontend/src/locales/nl-NL.po(48 hunks)echo/frontend/src/routes/participant/ParticipantPostConversation.tsx(4 hunks)echo/frontend/src/routes/project/report/ProjectReportRoute.tsx(7 hunks)echo/frontend/src/routes/project/unsubscribe/ProjectUnsubscribe.tsx(1 hunks)echo/server/dembrane/api/participant.py(3 hunks)echo/server/dembrane/api/project.py(1 hunks)
🧰 Additional context used
🧬 Code Definitions (4)
echo/frontend/src/Router.tsx (1)
echo/frontend/src/routes/project/unsubscribe/ProjectUnsubscribe.tsx (1)
ProjectUnsubscribe(12-94)
echo/frontend/src/routes/project/unsubscribe/ProjectUnsubscribe.tsx (2)
echo/frontend/src/lib/query.ts (1)
useCheckUnsubscribeStatus(1772-1808)echo/frontend/src/lib/api.ts (1)
unsubscribeParticipant(552-561)
echo/frontend/src/routes/project/report/ProjectReportRoute.tsx (1)
echo/frontend/src/lib/query.ts (1)
useGetProjectParticipants(1810-1832)
echo/frontend/src/routes/participant/ParticipantPostConversation.tsx (1)
echo/frontend/src/lib/query.ts (2)
useCheckProjectNotificationParticipants(1834-1863)useSubmitNotification(1865-1919)
🔇 Additional comments (131)
echo/server/dembrane/api/project.py (1)
408-408: Ship it! Smart status change from "published" to "archived" for new reports 🚀Moving reports to "archived" by default instead of "published" aligns perfectly with your new notification workflow. This creates a proper staging step where reports can be reviewed before triggering the notification flow to participants.
echo/frontend/src/lib/api.ts (1)
552-561: Elegantly implemented unsubscribe function! 🧠This is a tight, well-structured API function for the unsubscribe feature. Using PATCH instead of PUT is the right call since you're partially updating the participant record. The token parameter provides security without requiring authentication, and the explicit boolean for email_opt_in gives flexibility for potential resubscribe functionality.
echo/frontend/src/Router.tsx (2)
42-42: Clean import syntax! 👌Nice explicit import of the ProjectUnsubscribe component.
210-213: Love the router config - straight to the point! 💯Perfect placement of the unsubscribe route at the root level, making it accessible through a clean URL pattern. This is crucial for email unsubscribe links that need to be simple and persistent.
I see from the related component that you're handling the token and project_id via query params, which is the standard pattern for unsubscribe flows - solid choice!
echo/directus/sync/snapshot/collections/project_report_notification_participants.json (1)
1-29: Solid Snapshot Definition!
This JSON file for theproject_report_notification_participantscollection looks rock solid. All the necessary metadata and schema definitions are here and align perfectly with Directus standards. LGTM!echo/directus/sync/snapshot/fields/project_report_notification_participants/sort.json (1)
1-44: Clean and Precise Sort Field!
The configuration for thesortfield is crisp and consistent. All metadata and schema properties are correctly configured. Nice work!echo/directus/sync/snapshot/fields/project/is_project_notification_subscription_allowed.json (1)
1-46: Notification Subscription Field on Point!
This new boolean field is configured with the proper defaults and metadata. It looks like a seamless addition to manage subscription permissions. LGTM!echo/directus/sync/snapshot/fields/project_report_notification_participants/email_opt_in.json (1)
1-46: Email Opt-in Field: Ready to Roll!
Great job setting up theemail_opt_infield with a solid default oftrueand the appropriate metadata. Everything is shipshape and ready for production. LGTM!echo/directus/sync/snapshot/fields/project_report_notification_participants/date_submitted.json (1)
1-48: Excellent JSON setup for thedate_submittedfield.
The field is properly defined as a timestamp with clear metadata settings (hidden, readonly, relative display, etc.) and the schema is consistent with the expected database type. LGTM.echo/directus/sync/snapshot/fields/project_report_notification_participants/date_updated.json (1)
1-48: Solid configuration for thedate_updatedfield.
The JSON nicely mirrors the setup of thedate_submittedfield—with the correct data type, proper display options (relative enabled), and an updated sort order—ensuring consistency across the collection. LGTM.echo/directus/sync/snapshot/fields/project_report_notification_participants/id.json (1)
1-46: Rock-solid UUID field configuration.
Theidfield is defined as a UUID with all the expected properties (primary key, non-nullable, hidden, and read-only), which is perfect for uniquely identifying notification participants. LGTM.echo/.devcontainer/docker-compose.yml (1)
77-84: Impressive addition of email configuration variables.
The new SMTP-related environment variables (EMAIL_TRANSPORT, EMAIL_FROM, EMAIL_SMTP_HOST, EMAIL_SMTP_PORT, EMAIL_SMTP_USER, EMAIL_SMTP_PASSWORD, EMAIL_SMTP_SECURE, EMAIL_SMTP_IGNORE_TLS) are well integrated into thedirectusservice configuration. Just remember to set appropriate values for production to avoid sending emails from empty addresses. LGTM.echo/directus/sync/collections/permissions.json (1)
3099-3162: Neat integration of notification participant permissions.
The new permission entries for theproject_report_notification_participantscollection are set up correctly with distinct actions for create, read, and update. The update permission even wisely includes a condition ensuring that only users owning the related project can update entries. This is a robust design that enhances our notification feature. LGTM.echo/directus/sync/collections/flows.json (1)
1-35: Solid email notification flow configuration 🚀This JSON structure defines two critical flows for the notification system:
- A general "Send Email" flow triggered by operations
- A specialized "Send Report Emails" flow that fires on project_report create/update events
The configuration looks clean with proper event scoping and collection targeting. This lays the foundation for the notification feature.
echo/frontend/src/routes/project/unsubscribe/ProjectUnsubscribe.tsx (1)
47-92: LGTM - clean UI implementation with proper state handlingThe component UI handles all states elegantly - loading, error, success, and normal states with appropriate visual feedback. The responsive layout with centered content works well for this unsubscribe workflow.
echo/server/dembrane/api/participant.py (1)
78-81: LGTM - clean schema definitionThe
UnsubscribeParticipantRequestschema is simple and focused, capturing just what's needed for the operation.echo/directus/sync/snapshot/fields/quote/embedding.json (2)
4-4: Changed field type from unknown to text - makes senseConverting from an ambiguous "unknown" type to a concrete "text" type improves schema clarity.
8-14: UI configuration updates to make field visible and editableThese changes make the embedding field visible in the UI and provide a multi-line input interface, improving usability.
echo/frontend/src/lib/typesDirectus.d.ts (3)
525-525: Solid addition to the Project type. LGTM!Boolean flag to control notification subscription features. Clean implementation.
615-621: Sweet type definition for notification participants! 🚀Nice data model with all the essentials - id, email, project relationship, opt-in status, and token for unsubscription flows. The type flexibility with string|number|null for project_id is on point for Directus relationships.
738-738: Good addition to CustomDirectusTypes. Ship it!Correctly integrates the new participant type into the global Directus types collection.
echo/frontend/src/components/project/ProjectPortalEditor.tsx (3)
43-43: Zod schema addition is clean and minimal. LGTM!Boolean type is correctly defined for form validation.
149-150: Proper default value initialization with nullish coalescing! 💯Fallback to false is the right move here for new projects.
520-563: Well-structured UI component for notification management! 🔥Excellent integration with the existing form patterns. The component hierarchy is clean, and you've properly handled form state management with react-hook-form Controller. Good use of the FormLabel component with dirty tracking and error handling.
echo/frontend/src/routes/project/report/ProjectReportRoute.tsx (6)
4-4: Proper imports for new functionality. LGTM!Importing the necessary hook and Modal component.
Also applies to: 22-22
110-113: Clean state initialization for modal and publish status.Good use of useDisclosure for modal state management and useState for tracking publish status.
113-123: Solid implementation of confirmation handler! 💪The handleConfirmPublish function properly validates data existence before proceeding. The consistent payload structure with project_id as an object is on point.
233-259: Smart conditional flow for the publish action!Love how this intelligently decides whether to show the confirmation modal based on participant count. The branching logic is clean and the payload structure is consistent across all update calls.
281-281: Consistent payload format for project_id. LGTM!Ensuring the same payload structure across all report update calls.
311-334: Sweet modal implementation with proper i18n support! 🌐The confirmation dialog is well-structured with clear messaging and proper handling of pluralization. Good use of the Mantine Modal component with appropriate actions and button styling.
echo/frontend/src/lib/query.ts (5)
1493-1497: Robust projectId extraction logic! 👌Smart handling of projectId that works with both object and primitive formats. This prevents potential bugs when the payload structure varies.
1772-1808: Rock-solid unsubscribe status checker!Great implementation with proper error handling for invalid tokens or project IDs. The filter criteria are precise, and disabling retries and window focus refetching is the right call for this use case.
1810-1832: Efficient participant counter implementation! 💯Clean and focused query that only counts opted-in participants. The null check with early return and the enabled condition ensure it only runs when necessary.
1834-1863: Smart subscription status checker with clear state outputs!The tri-state return value ("opted_out", "subscribed", "new") makes consuming this hook super intuitive. Using useMutation is the right pattern here since it's checking status before potential modifications.
1865-1919: Bulletproof notification submission implementation! 🚀Love the parallel processing with Promise.all for handling multiple emails efficiently. The flow for handling existing users is spot-on - regenerating tokens for previously opted-out users adds a nice touch. Proper error handling within the map function and the retry configuration show attention to production reliability.
echo/directus/sync/snapshot/fields/project_report_notification_participants/email_opt_out_token.json (3)
2-4: LGTM: UUID field for unsubscribe tokenSolid choice using UUID type for the email opt-out token. Perfect for generating secure, unique tokens that can be used in unsubscribe links while preventing enumeration attacks.
8-21: LGTM: Field metadata configurationThe metadata configuration is clean and appropriate for a UUID field. Setting
readonly: falseallows the system to generate and update tokens as needed, while thespecial: ["uuid"]annotation ensures proper handling of the UUID format.
28-30: LGTM: Schema definition is properly configuredThe schema definition correctly maps this field to the project_report_notification_participants table with the appropriate data type. This will ensure database-level enforcement of the UUID format.
echo/directus/sync/collections/operations.json (8)
2-23: LGTM: Project report publishing condition checkThe conditional operation flow is well-structured to check if a report is published before proceeding with email notifications. This ensures emails are only sent for published reports.
24-43: LGTM: Language detection operations for multi-language supportThe language detection operations are well-designed with proper fallbacks. The system checks first for English ("en"), then Dutch ("nl"), with a fallback logging operation when language detection fails.
Also applies to: 70-89, 129-131
44-68: LGTM: Report language retrieval operationThe operation correctly retrieves the language from the project_report collection, filtering by the project ID from the trigger payload.
90-117: LGTM: Error logging operationsGood error handling pattern with dedicated logging operations for both English and Dutch email failures.
146-168: LGTM: Participants retrieval operationThis operation efficiently queries all participants for a given project ID, setting up the data needed for email filtering.
169-182: LGTM: Report publishing status loggingNice touch with the logging operation to track when a report isn't published, making debugging easier.
202-220: LGTM: Well-formed English email templateThe English email template correctly uses the dynamic URL variables and has a well-structured HTML format.
221-236: LGTM: Email flow trigger operationThe trigger email flow operation properly chains all operations together, using serial mode for reliability and passing the filtered participant data.
echo/frontend/src/locales/nl-NL.po (20)
41-44: LGTM: Dutch translation for "Submit" buttonThe translation "Verzenden" for "Submit" is appropriate and maintains consistent terminology.
45-48: LGTM: Dutch translation for unsubscribe functionality"Afmelden voor meldingen" accurately translates "Unsubscribe from Notifications".
179-184: LGTM: Dutch translation for email notification confirmation dialogThe confirmation message about sending notifications to participants is properly translated, maintaining both the plural form handling and the variable placeholders.
391-394: LGTM: Dutch translation for checking status"Controleren..." is an appropriate translation for "Checking...".
413-416: LGTM: Dutch translation for confirmation dialog"Publiceren bevestigen" correctly translates "Confirm Publishing".
531-534: LGTM: Dutch translation for subscription prompt"Wil je blijven in de gaten?" is a good translation for "Do you want to stay in the loop?", maintaining the casual tone.
626-629: LGTM: Dutch translation for email placeholder"email@werk.com" is an appropriate localized placeholder for "email@work.com".
639-641: LGTM: Dutch translation for enabling report notifications"Berichtsbenachrichtigingen inschakelen" appropriately translates "Enable Report Notifications".
642-645: LGTM: Dutch translation for notification feature descriptionThe translation maintains all the key information about the feature's functionality.
709-710: LGTM: Dutch translations for loading state"bezig met laden..." is an appropriate translation for "loading...".
Also applies to: 926-929
1209-1213: LGTM: Dutch translation for email validation message"Voer een geldig e-mailadres in." correctly translates "Please enter a valid email."
1287-1290: LGTM: Dutch translation for proceeding with action"Doorgaan" is an appropriate translation for "Proceed".
1394-1397: LGTM: Dutch translation for removing email"E-mail verwijderen" accurately translates "Remove Email".
1430-1432: LGTM: Dutch translation for report notifications"Rapportmeldingen" is a good translation for "Report Notifications".
1553-1556: LGTM: Dutch translation for sharing details"Deel je gegevens hier" appropriately translates "Share your details here".
1665-1668: LGTM: Dutch translation for notification confirmationThe translation maintains the friendly tone and clearly communicates that users will be notified when the report is ready.
1709-1716: LGTM: Dutch translations for email validation messagesBoth error messages are properly translated, maintaining clear meaning.
1914-1916: LGTM: Dutch translation for unsubscribe button"Afmelden" is a concise and appropriate translation for "Unsubscribe".
1972-1975: LGTM: Dutch translation for privacy noticeThe translation maintains all the key privacy information regarding how user data is handled.
2016-2019: LGTM: Dutch translation for unsubscribe confirmation"U hebt succesvol afgemeld." appropriately confirms successful unsubscription.
echo/frontend/src/locales/de-DE.po (4)
41-47: Fresh UI translations for email subscription!Clean implementation for the new email subscription feature UI text. All the submit and unsubscribe copy is properly localized to German.
151-155: LGTM - notification placeholder handling is on point!The dynamic placeholder handling for participant count in notification messages is properly translated with appropriate pluralization support. This will handle singular/plural cases correctly.
522-528: Solid feature description translations!The feature description is well translated and maintains the original intent while being properly adapted to German language structure. This is exactly how localization should be done.
1710-1713: Privacy notice translation is rock solid!The privacy notice translation maintains all the key points from the original text about data usage and opt-out options. This is crucial for maintaining legal compliance across languages.
echo/frontend/src/routes/participant/ParticipantPostConversation.tsx (5)
40-49: Clean state management setup for the subscription flow!State variables are well-organized with clear separation of concerns. You're using React hooks properly for managing the complex state requirements of this feature.
117-144: Solid email verification with proper error handling!The email verification logic is solid with comprehensive status handling. You're gracefully handling all three status cases (subscribed/opted_out/new) and providing clear error messaging.
196-230: Clean TextInput implementation with robust error states!The TextInput component implementation is well-structured with proper loading states, error handling, and a nice right-section button for adding emails. UX details like disabling inputs during verification show attention to detail.
231-264: Slick UI for email list management!The Chip-based UI for managing added emails is elegant and intuitive. Good use of the Paper component to visually separate the list, and the tooltip on hover for removal action is a nice touch.
301-307: Privacy notice inclusion is crucial!Including this privacy notice about how emails will be used is not just good UX but essential for compliance. Clearly explaining that emails are only used for report notifications and including the opt-out information builds trust.
echo/frontend/src/locales/fr-FR.po (17)
41-47: Nice clean translations for Submit and Unsubscribe buttons!The translations for " Submit" and " Unsubscribe from Notifications" look correct and well-formatted. The space before the text matches the source strings, maintaining consistency in UI rendering.
91-93: LGTM! Simple translation for "Add" button.The translation "Ajouter" correctly reflects the meaning of "Add" in French.
119-121: Good email-related terminology choice!"E-mails ajoutés" is the correct translation for "Added emails" in French, maintaining the proper hyphenation for "E-mail" in French.
151-155: Solid translation with proper placeholder handling for participant notifications!The translation correctly preserves the placeholders {0} and {1} for dynamic content injection, which is crucial for proper functioning of the application.
318-320: On-point translation for status indicator!"Checking..." → "Vérification..." maintains the ellipsis and properly conveys the ongoing action state.
340-342: Clean translation for confirmation dialog!"Confirm Publishing" → "Publier" uses a more concise term in French, which is actually better UX than a literal translation would be.
442-444: Great conversational tone in translation!"Do you want to stay in the loop?" → "Voulez-vous rester dans la boucle?" maintains the casual, conversational tone of the original.
513-515: Smart localization of email placeholder!"email@work.com" → "email@travail.com" properly localizes the example domain name by translating "work" to "travail".
525-531: Excellent translation of feature description!The translation for the Report Notifications feature description is clear and maintains all the important details from the original text.
593-596: Good error message translation!"Failed to verify email status. Please try again." → "Échec de la vérification de l'état de l'e-mail. Veuillez réessayer." properly conveys the error state and action needed.
768-770: Simple but important translation!"loading..." → "chargement..." maintains lowercase style and ellipsis for consistency.
1022-1025: Proper validation message translation!"Please enter a valid email." → "Veuillez entrer une adresse e-mail valide." correctly expands "email" to "adresse e-mail" which is more formal in French.
1100-1102: Compact action button translation!"Proceed" → "Continuer" is the appropriate French term for this action.
1226-1228: Feature name properly translated!"Report Notifications" → "Notifications de rapports" follows French grammar rules correctly.
1656-1658: Clean translation for unsubscribe action!"Unsubscribe" → "Se désabonner" uses the reflexive verb form which is correct in French.
1711-1713: Privacy statement well translated!The translation properly communicates the privacy policy regarding email notifications and the opt-out option.
1739-1741: Successful action feedback translated correctly!"You have successfully unsubscribed." → "Vous avez été désinscrit avec succès." uses passive voice which is appropriate in this context in French.
echo/frontend/src/locales/es-ES.po (19)
41-47: Clean translations for action buttons!The Spanish translations for " Submit" and " Unsubscribe from Notifications" are accurate. Note that the space at the beginning of the strings is properly preserved.
91-93: Simple but effective translation!"Add" → "Añadir" is the correct Spanish term with proper diacritical mark.
119-121: Consistent email terminology!"Added emails" → "E-mails añadidos" maintains the hyphenated "E-mail" format consistently with the French translation.
151-155: Great plural handling in translation!The translation correctly manages the conditional plural form with placeholders {0} and {1}, ensuring grammatical correctness regardless of participant count.
318-320: Perfect status indicator translation!"Checking..." → "Comprobando..." maintains the ellipsis and properly conveys the ongoing action.
340-342: Concise action button translation!"Confirm Publishing" → "Publicar" simplifies to the verb form in Spanish, which is a better UX choice than a literal "Confirmar Publicación".
442-444: Idiomatic translation maintains intent!"Do you want to stay in the loop?" → "¿Quieres estar en el bucle?" correctly preserves the casual, conversational tone while using the equivalent Spanish idiom.
513-515: Smart localization of example domain!"email@work.com" → "email@trabajo.com" properly translates the domain example by changing "work" to "trabajo".
525-531: Clear feature explanation translation!The translation for enabling report notifications clearly conveys all the necessary information from the original text.
593-596: Appropriate error state translation!The Spanish error message for email verification failure is accurately translated and maintains the guidance to try again.
768-770: Simple but crucial state indicator!"loading..." → "cargando..." maintains lowercase style and the ellipsis for consistency with the design pattern.
1022-1025: Validation guidance properly translated!"Please enter a valid email." → "Por favor, ingrese un correo electrónico válido." expands "email" to the full "correo electrónico" which is more formal in Spanish.
1100-1102: Action button translation is spot on!"Proceed" → "Continuar" is the appropriate Spanish verb for this action.
1226-1228: Feature name well translated!"Report Notifications" → "Notificaciones de Reportes" follows Spanish capitalization and grammar conventions.
1485-1487: Validation feedback clearly translated!"This email is already in the list." → "Este correo electrónico ya está en la lista." correctly uses "correo electrónico" which is the proper formal term in Spanish.
1489-1491: Consistent terminology in error messages!"This email is already subscribed to notifications." → "Este correo electrónico ya está suscrito a notificaciones." maintains consistency with the previous message.
1656-1658: Clear action button translation!"Unsubscribe" → "Desuscribirse" uses the reflexive verb form which is correct in Spanish.
1711-1713: Privacy statement accurately translated!The Spanish translation properly communicates the privacy policy and opt-out information with appropriate language.
1739-1741: Success message properly translated!"You have successfully unsubscribed." → "Has desuscribido con éxito." uses second person singular appropriately for this user action confirmation.
echo/frontend/src/locales/en-US.po (20)
41-43: Adding submit UI string with leading space. LGTM!The inclusion of a leading space in " Submit" is likely intentional for UI spacing. Solid pattern for button text with icon.
45-47: Unsubscribe notification string added with leading space. LGTM!Similar to the submit button, this pattern with leading space suggests it's paired with an icon. Perfect for the unsubscribe functionality being implemented.
124-126: New translation for "Added emails" list label. LGTM!Clean addition for displaying emails that have been added to the notification list. Follows project naming conventions.
156-160: Notification confirmation string. LGTM!Good pluralization support in the notification confirmation dialog with proper variable substitution for {0} participant{1}. This handles both singular and plural cases elegantly.
335-337: Added "Checking..." loading state. LGTM!Smart addition for providing user feedback during email verification. Improves UX by indicating something is happening.
357-359: Confirm Publishing dialog title. LGTM!Clean, concise modal title for the publishing confirmation flow. Follows established pattern for confirmation dialogs.
463-465: New subscription prompt string. LGTM!Well-phrased question that encourages user engagement with the notification system. Conversational tone matches the app's style.
538-540: Email placeholder text. LGTM!Good example email format that clearly indicates what kind of input is expected. Standard practice for email fields.
550-557: Report notifications feature explanation. LGTM!Comprehensive description that clearly explains the notification feature's purpose and benefits. Gives users a complete understanding of what they're enabling.
626-629: Email verification error message. LGTM!Clear error message with actionable instruction when email verification fails. Good UX to let users know what happened and what to do next.
1137-1139: Proceed button text. LGTM!Short, actionable button label for confirmation dialogs. Perfectly paired with the Cancel option.
1240-1242: Remove Email action text. LGTM!Clear action label for removing an email from the notification list. Follows verb-noun pattern for action buttons.
1275-1277: Report Notifications section header. LGTM!Concise section title that clearly identifies the feature area. Consistent with other section headers in the system.
1391-1392: Share details prompt. LGTM!User-friendly instruction that encourages information sharing without being demanding.
1498-1500: Notification confirmation message. LGTM!Excellent confirmation message that sets expectations about when notifications will be sent. Clear and reassuring.
1542-1544: Duplicate email validation. LGTM!Good validation message for preventing duplicate emails in the list. Clear and prevents user confusion.
1546-1548: Already subscribed validation. LGTM!Similar validation message but specifically for emails already subscribed. Good distinction between "in the list" and "subscribed".
1732-1734: Unsubscribe button text. LGTM!Simple, direct action text for the unsubscribe button. Consistent with industry standards for this action.
1787-1790: Privacy explanation for notifications. LGTM!Excellent privacy reassurance that explains when messages will be sent and emphasizes data protection. Important for building user trust.
1819-1821: Unsubscribe success message. LGTM!Clear confirmation that the unsubscribe action was successful. Good UX to close the loop on user actions.
| @ParticipantRouter.patch("/projects/{project_id}/contacts/unsubscribe") | ||
| async def unsubscribe_participant( | ||
| project_id: str, | ||
| payload: UnsubscribeParticipantRequest, | ||
| ) -> dict: | ||
| """ | ||
| Update email_opt_in for project contacts in Directus securely. | ||
| """ | ||
| try: | ||
| # Fetch relevant IDs | ||
| submissions = directus.get_items( | ||
| "project_report_notification_participants", | ||
| { | ||
| "query": { | ||
| "filter": { | ||
| "_and": [ | ||
| {"project_id": {"_eq": project_id}}, | ||
| {"email_opt_out_token": {"_eq": payload.token}}, | ||
| ] | ||
| }, | ||
| "fields": ["id"], | ||
| }, | ||
| }, | ||
| ) | ||
|
|
||
| if not submissions or len(submissions) == 0: | ||
| raise HTTPException(status_code=404, detail="No data found") | ||
|
|
||
| ids = [item["id"] for item in submissions] | ||
|
|
||
| # Update email_opt_in status for fetched IDs | ||
| for item_id in ids: | ||
| directus.update_item( | ||
| "project_report_notification_participants", | ||
| item_id, | ||
| {"email_opt_in": payload.email_opt_in} | ||
| ) | ||
|
|
||
| return {"success": True, "message": "Subscription status updated"} | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Error updating project contacts: {str(e)}") | ||
| raise HTTPException(status_code=500, detail="Internal Server Error") # noqa: B904 No newline at end of file |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Robust implementation but error handling could be more granular
The endpoint correctly handles the unsubscribe flow, but catches all exceptions with a generic 500 error. This masks specific error conditions, making debugging harder.
@ParticipantRouter.patch("/projects/{project_id}/contacts/unsubscribe")
async def unsubscribe_participant(
project_id: str,
payload: UnsubscribeParticipantRequest,
) -> dict:
"""
Update email_opt_in for project contacts in Directus securely.
"""
try:
# Fetch relevant IDs
submissions = directus.get_items(
"project_report_notification_participants",
{
"query": {
"filter": {
"_and": [
{"project_id": {"_eq": project_id}},
{"email_opt_out_token": {"_eq": payload.token}},
]
},
"fields": ["id"],
},
},
)
if not submissions or len(submissions) == 0:
raise HTTPException(status_code=404, detail="No data found")
ids = [item["id"] for item in submissions]
# Update email_opt_in status for fetched IDs
for item_id in ids:
directus.update_item(
"project_report_notification_participants",
item_id,
{"email_opt_in": payload.email_opt_in}
)
return {"success": True, "message": "Subscription status updated"}
- except Exception as e:
+ except HTTPException:
+ # Re-raise HTTP exceptions
+ raise
+ except directus.errors.DirectusError as e:
+ logger.error(f"Directus error updating project contacts: {str(e)}")
+ raise HTTPException(status_code=503, detail="Service Unavailable")
+ except Exception as e:
logger.error(f"Error updating project contacts: {str(e)}")
raise HTTPException(status_code=500, detail="Internal Server Error") # noqa: B904Also, consider adding validation to check if the current opt-in status matches the requested change to avoid unnecessary updates.
🏁 Script executed:
#!/bin/bash
# Check for similar error handling patterns in other endpoints
rg -A 3 "except Exception as e:" --glob "*.py" | grep -B 1 "HTTPException"Length of output: 473
Attention: Granular Exception Handling and Opt-In Validation Needed
The new diff improves error handling in the unsubscribe endpoint by separating HTTP exceptions from Directus errors. Nice work! However, please consider the following:
- Uniform Granularity: The updated pattern in
participant.pyis an improvement. For consistency, similar granular error handling might be applied to other endpoints (e.g., inecho/server/dembrane/api/static.py). - Opt-In Status Check: Add validation to verify that the current opt-in status differs from the requested update. This will help avoid unnecessary update calls.
Please address these items accordingly.
📝 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.
| @ParticipantRouter.patch("/projects/{project_id}/contacts/unsubscribe") | |
| async def unsubscribe_participant( | |
| project_id: str, | |
| payload: UnsubscribeParticipantRequest, | |
| ) -> dict: | |
| """ | |
| Update email_opt_in for project contacts in Directus securely. | |
| """ | |
| try: | |
| # Fetch relevant IDs | |
| submissions = directus.get_items( | |
| "project_report_notification_participants", | |
| { | |
| "query": { | |
| "filter": { | |
| "_and": [ | |
| {"project_id": {"_eq": project_id}}, | |
| {"email_opt_out_token": {"_eq": payload.token}}, | |
| ] | |
| }, | |
| "fields": ["id"], | |
| }, | |
| }, | |
| ) | |
| if not submissions or len(submissions) == 0: | |
| raise HTTPException(status_code=404, detail="No data found") | |
| ids = [item["id"] for item in submissions] | |
| # Update email_opt_in status for fetched IDs | |
| for item_id in ids: | |
| directus.update_item( | |
| "project_report_notification_participants", | |
| item_id, | |
| {"email_opt_in": payload.email_opt_in} | |
| ) | |
| return {"success": True, "message": "Subscription status updated"} | |
| except Exception as e: | |
| logger.error(f"Error updating project contacts: {str(e)}") | |
| raise HTTPException(status_code=500, detail="Internal Server Error") # noqa: B904 | |
| @ParticipantRouter.patch("/projects/{project_id}/contacts/unsubscribe") | |
| async def unsubscribe_participant( | |
| project_id: str, | |
| payload: UnsubscribeParticipantRequest, | |
| ) -> dict: | |
| """ | |
| Update email_opt_in for project contacts in Directus securely. | |
| """ | |
| try: | |
| # Fetch relevant IDs | |
| submissions = directus.get_items( | |
| "project_report_notification_participants", | |
| { | |
| "query": { | |
| "filter": { | |
| "_and": [ | |
| {"project_id": {"_eq": project_id}}, | |
| {"email_opt_out_token": {"_eq": payload.token}}, | |
| ] | |
| }, | |
| "fields": ["id"], | |
| }, | |
| }, | |
| ) | |
| if not submissions or len(submissions) == 0: | |
| raise HTTPException(status_code=404, detail="No data found") | |
| ids = [item["id"] for item in submissions] | |
| # Update email_opt_in status for fetched IDs | |
| for item_id in ids: | |
| directus.update_item( | |
| "project_report_notification_participants", | |
| item_id, | |
| {"email_opt_in": payload.email_opt_in} | |
| ) | |
| return {"success": True, "message": "Subscription status updated"} | |
| except HTTPException: | |
| # Re-raise HTTP exceptions | |
| raise | |
| except directus.errors.DirectusError as e: | |
| logger.error(f"Directus error updating project contacts: {str(e)}") | |
| raise HTTPException(status_code=503, detail="Service Unavailable") | |
| except Exception as e: | |
| logger.error(f"Error updating project contacts: {str(e)}") | |
| raise HTTPException(status_code=500, detail="Internal Server Error") # noqa: B904 |
There was a problem hiding this comment.
1- try-catch already handles all errors generically
2- opt-in status always stays "false" in this case, so check not necessary
There was a problem hiding this comment.
@ussaama LGTM on the approach! Your point about generic error handling is valid here, and you're right that opt-in validation isn't necessary when it's always set to false in the unsubscribe flow. The implementation is clean and follows the project patterns. Ship it! 🚢
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
echo/directus/sync/collections/operations.json (1)
132-145: Email Filtering Functionality is Clever and Effective
The code properly filters submissions based on theemail_opt_inflag and maps the required details, including dynamic URL bases. A couple of minor suggestions:
- Consider ensuring that
data.get_all_participantsis defined before applying.filter()to avoid potential runtime issues.- The fallback
"empty"for language is acceptable, but a slightly more descriptive default might improve clarity.
Overall, a nimble implementation—LGTM with a tiny nitpick!
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
echo/directus/sync/collections/operations.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci-check-server
🔇 Additional comments (12)
echo/directus/sync/collections/operations.json (12)
1-23: Published Report Check is Solid
This operation correctly verifies that the report’s status is "published" with a clear and consistent JSON structure. LGTM.
24-43: Language Condition (EN) is Spot On
The condition checking that the trigger language is "en" is implemented with clarity and precision. LGTM.
44-69: Verify Payload Structure for Report Language
The query uses"{{$trigger.payload.project_id.id}}"which differs from other operations (e.g., see "Get All Participants"). Please confirm that the payload for the report indeed provides an object with anidproperty. This difference might be intentional for theproject_reportcollection, but a quick recheck is advised.
70-89: Dutch Language Condition is in Great Shape
The condition for detecting Dutch language ("nl") is concise and follows our schema perfectly. LGTM.
90-103: Email Failure Logging (English) Works as Expected
The logging operation captures email send failures well by leveraging the Directus template variable for the last error. Nothing to change here. LGTM.
104-117: Email Failure Logging (Dutch) is Equally Robust
This operation mirrors the English counterpart efficiently. The implementation is consistent and straightforward. LGTM.
118-131: Language Detection Failure Logged Neatly
Logging a failure in language detection with a simple message is perfect. LGTM.
146-168: Participants Retrieval Needs a Quick Consistency Check
This operation retrieves participants using"{{$trigger.payload.project_id}}"(without the.idaccessor) which contrasts with the earlier report language query. Please verify that this difference is deliberate due to different data structures in the payload.
169-182: Report Not Published Logging is Clean and Concise
The log message for unpublished reports is straightforward and does exactly what’s needed. LGTM.
183-201: Dutch Email Template is Impeccable
The Dutch email template uses dynamic base URL references and HTML formatting perfectly. Just ensure the URLs resolve as intended in production. LGTM.
202-220: English Email Template is On Point
This email template mirrors the Dutch version with appropriate localization, ensuring dynamic URLs are used. Everything looks robust here. LGTM.
221-236: Trigger Email Flow is Clean and Effective
The trigger operation efficiently passes the filtered email payload and is configured with a serial iteration mode. This setup is both clear and practical. LGTM.
spashii
changed the title
- Introduced new audio ETL pipelines for processing and analyzing audio data. - Added various utility functions for audio handling and processing. - Updated documentation to include new features and usage examples. - Added test cases for new functionalities to ensure reliability. - Included new audio files for testing purposes in the repository.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
echo/.devcontainer/docker-compose.yml(1 hunks)echo/.gitignore(1 hunks)echo/frontend/src/lib/api.ts(1 hunks)echo/frontend/src/lib/query.ts(3 hunks)echo/frontend/src/lib/typesDirectus.d.ts(3 hunks)echo/frontend/src/locales/de-DE.po(50 hunks)echo/frontend/src/locales/en-US.po(51 hunks)echo/frontend/src/locales/es-ES.po(49 hunks)echo/frontend/src/locales/fr-FR.po(49 hunks)echo/frontend/src/locales/nl-NL.po(51 hunks)echo/server/dembrane/api/participant.py(3 hunks)echo/server/dembrane/api/project.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- echo/.gitignore
- echo/server/dembrane/api/project.py
- echo/.devcontainer/docker-compose.yml
- echo/frontend/src/lib/api.ts
- echo/server/dembrane/api/participant.py
- echo/frontend/src/lib/typesDirectus.d.ts
- echo/frontend/src/locales/de-DE.po
- echo/frontend/src/locales/nl-NL.po
- echo/frontend/src/locales/es-ES.po
- echo/frontend/src/locales/fr-FR.po
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ci-build-servers (dbr-echo-server, ./echo/server, Dockerfile, dbr-echo-server)
- GitHub Check: ci-build-servers (dbr-echo-directus, ./echo/directus, Dockerfile, dbr-echo-directus)
🔇 Additional comments (14)
echo/frontend/src/locales/en-US.po (9)
41-47: New notification actions look solidAdded message strings for submit and unsubscribe actions in the participant flow - these will provide clear user affordances for the new notification features.
167-171: Good UX for notification confirmationSmart implementation of a confirmation dialog with dynamic participant count display. The
{0}and{1}placeholders handle pluralization elegantly.
520-522: User engagement prompt is on pointThe "Do you want to stay in the loop?" message with example email placeholder creates a frictionless subscription entry point. Good conversion optimization!
Also applies to: 605-607
617-619: Clean implementation of notification settingsThe toggle label and description text are clear and concise, making the admin-side configuration intuitive. The description perfectly explains the feature's value proposition.
Also applies to: 621-623
721-724: Robust error handling for email validationAll the edge cases are covered with specific error messages. Good defensive coding practice with separate strings for invalid format, duplicates, and verification failures.
Also applies to: 1229-1232, 1233-1235, 1237-1239
1781-1783: Effective success feedbackThe confirmation message provides clear closure to the subscription flow, setting proper expectations about when notifications will be sent.
2027-2029: Unsubscribe flow is completeThe unsubscribe button and success message provide a clean exit path from notifications. This implementation ensures compliance with email best practices.
Also applies to: 2150-2152
2113-2115: Privacy-forward messagingStrong privacy statement that builds trust with users. The clear explanation of data usage and opt-out options is essential for maintaining transparent communication.
1316-1317: Consistent UI messaging patternsThe loading indicator and action button text follow established UX patterns from the rest of the application.
Also applies to: 944-946
echo/frontend/src/lib/query.ts (5)
55-55: ImportingupdateItemsfrom Directus SDK. LGTM!Nice addition of the
updateItemsimport for batch operations on Directus collections. Clean import at the top of the file.
1535-1537: Impressive type safety handling. LGTM!Solid defensive programming here - handling both object and string
project_idformats ensures the invalidation works correctly regardless of input structure. This prevents subtle bugs when different parts of the codebase pass project IDs in different formats.
2000-2036: Robust unsubscribe validator. LGTM!This hook elegantly validates unsubscribe tokens with proper error handling. The disabled retry and refetchOnWindowFocus flags are spot-on for this type of validation check.
2038-2060: Optimized participant counter. LGTM!Smart implementation with the
enabled: !!project_idpattern to prevent unnecessary queries. The filtering approach efficiently counts only opted-in participants.
2062-2091: Clean subscription status checker. LGTM!Good choice using a mutation here instead of a query for on-demand checking. The three-state return value (
opted_out,subscribed,new) gives the UI all it needs to handle different scenarios.
| export const useSubmitNotification = () => { | ||
| return useMutation({ | ||
| mutationFn: async ({ | ||
| emails, | ||
| projectId, | ||
| }: { | ||
| emails: string[]; | ||
| projectId: string; | ||
| }) => { | ||
| await Promise.all( | ||
| emails.map(async (email) => { | ||
| try { | ||
| // Check if the user already exists | ||
| const existingEntries = await directus.request( | ||
| readItems("project_report_notification_participants", { | ||
| filter: { | ||
| email: { _eq: email }, | ||
| project_id: { _eq: projectId }, | ||
| }, | ||
| limit: 1, | ||
| }), | ||
| ); | ||
| if (existingEntries?.length > 0) { | ||
| const existingEntry = existingEntries[0]; | ||
| if (!existingEntry.email_opt_in) { | ||
| // Delete the old entry to regenerate the email_opt_out_token | ||
| await directus.request( | ||
| deleteItem( | ||
| "project_report_notification_participants", | ||
| existingEntry.id, | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
| // Create a new entry (this generates a new email_opt_out_token) | ||
| await directus.request( | ||
| createItem("project_report_notification_participants", { | ||
| email: email, | ||
| project_id: projectId, | ||
| email_opt_in: true, | ||
| }), | ||
| ); | ||
| } catch (error) { | ||
| console.error(`Failed to process email: ${email}`, error); | ||
| throw error; | ||
| } | ||
| }), | ||
| ); | ||
| }, | ||
| retry: 2, | ||
| onError: (error) => { | ||
| console.error('Notification submission failed:', error); | ||
| }, | ||
| }); | ||
| }; No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Notification submission handler needs error feedback for users
While the hook handles the complex scenarios well (especially regenerating tokens for previously opted-out users), it lacks user-facing error feedback and query invalidation.
Add toast notifications and query invalidation to match patterns in other mutations:
export const useSubmitNotification = () => {
const queryClient = useQueryClient();
return useMutation({
mutationFn: async ({
emails,
projectId,
}: {
emails: string[];
projectId: string;
}) => {
await Promise.all(
emails.map(async (email) => {
try {
// Check if the user already exists
const existingEntries = await directus.request(
readItems("project_report_notification_participants", {
filter: {
email: { _eq: email },
project_id: { _eq: projectId },
},
limit: 1,
}),
);
if (existingEntries?.length > 0) {
const existingEntry = existingEntries[0];
if (!existingEntry.email_opt_in) {
// Delete the old entry to regenerate the email_opt_out_token
await directus.request(
deleteItem(
"project_report_notification_participants",
existingEntry.id,
),
);
}
}
// Create a new entry (this generates a new email_opt_out_token)
await directus.request(
createItem("project_report_notification_participants", {
email: email,
project_id: projectId,
email_opt_in: true,
}),
);
} catch (error) {
console.error(`Failed to process email: ${email}`, error);
throw error;
}
}),
);
},
retry: 2,
onError: (error) => {
console.error('Notification submission failed:', error);
+ toast.error(`Failed to update notification preferences. Please try again.`);
},
+ onSuccess: (_, variables) => {
+ queryClient.invalidateQueries({
+ queryKey: ["projectParticipants", variables.projectId],
+ });
+ toast.success("Notification preferences updated successfully");
+ },
});
};📝 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.
| export const useSubmitNotification = () => { | |
| return useMutation({ | |
| mutationFn: async ({ | |
| emails, | |
| projectId, | |
| }: { | |
| emails: string[]; | |
| projectId: string; | |
| }) => { | |
| await Promise.all( | |
| emails.map(async (email) => { | |
| try { | |
| // Check if the user already exists | |
| const existingEntries = await directus.request( | |
| readItems("project_report_notification_participants", { | |
| filter: { | |
| email: { _eq: email }, | |
| project_id: { _eq: projectId }, | |
| }, | |
| limit: 1, | |
| }), | |
| ); | |
| if (existingEntries?.length > 0) { | |
| const existingEntry = existingEntries[0]; | |
| if (!existingEntry.email_opt_in) { | |
| // Delete the old entry to regenerate the email_opt_out_token | |
| await directus.request( | |
| deleteItem( | |
| "project_report_notification_participants", | |
| existingEntry.id, | |
| ), | |
| ); | |
| } | |
| } | |
| // Create a new entry (this generates a new email_opt_out_token) | |
| await directus.request( | |
| createItem("project_report_notification_participants", { | |
| email: email, | |
| project_id: projectId, | |
| email_opt_in: true, | |
| }), | |
| ); | |
| } catch (error) { | |
| console.error(`Failed to process email: ${email}`, error); | |
| throw error; | |
| } | |
| }), | |
| ); | |
| }, | |
| retry: 2, | |
| onError: (error) => { | |
| console.error('Notification submission failed:', error); | |
| }, | |
| }); | |
| }; | |
| export const useSubmitNotification = () => { | |
| const queryClient = useQueryClient(); | |
| return useMutation({ | |
| mutationFn: async ({ | |
| emails, | |
| projectId, | |
| }: { | |
| emails: string[]; | |
| projectId: string; | |
| }) => { | |
| await Promise.all( | |
| emails.map(async (email) => { | |
| try { | |
| // Check if the user already exists | |
| const existingEntries = await directus.request( | |
| readItems("project_report_notification_participants", { | |
| filter: { | |
| email: { _eq: email }, | |
| project_id: { _eq: projectId }, | |
| }, | |
| limit: 1, | |
| }), | |
| ); | |
| if (existingEntries?.length > 0) { | |
| const existingEntry = existingEntries[0]; | |
| if (!existingEntry.email_opt_in) { | |
| // Delete the old entry to regenerate the email_opt_out_token | |
| await directus.request( | |
| deleteItem( | |
| "project_report_notification_participants", | |
| existingEntry.id, | |
| ), | |
| ); | |
| } | |
| } | |
| // Create a new entry (this generates a new email_opt_out_token) | |
| await directus.request( | |
| createItem("project_report_notification_participants", { | |
| email: email, | |
| project_id: projectId, | |
| email_opt_in: true, | |
| }), | |
| ); | |
| } catch (error) { | |
| console.error(`Failed to process email: ${email}`, error); | |
| throw error; | |
| } | |
| }), | |
| ); | |
| }, | |
| retry: 2, | |
| onError: (error) => { | |
| console.error('Notification submission failed:', error); | |
| toast.error(`Failed to update notification preferences. Please try again.`); | |
| }, | |
| onSuccess: (_, variables) => { | |
| queryClient.invalidateQueries({ | |
| queryKey: ["projectParticipants", variables.projectId], | |
| }); | |
| toast.success("Notification preferences updated successfully"); | |
| }, | |
| }); | |
| }; |
* Add email notification subscription feature for project reports - Updated the Docker Compose configuration to include email transport settings. - Enhanced the ProjectPortalEditor to allow enabling/disabling project notification subscriptions. - Implemented email subscription management in the ParticipantPostConversation component, allowing users to add and validate email addresses for notifications. - Added new hooks in the query library for checking subscription status and updating participant email preferences. - Updated the ProjectReportRoute to confirm email notifications before publishing reports. - Introduced new types for project report notification participants in TypeScript definitions. - Adjusted the backend to handle email notification status during report creation. This feature allows participants to subscribe to notifications for project report updates, improving user engagement and communication. * Enhance error handling in useUpdateProjectContacts hook - Wrapped the request for fetching participant IDs in a try-catch block to handle potential errors during the unsubscribe process. - Added console error logging for better debugging and user feedback when the unsubscribe operation fails. - Ensured that a clear error message is thrown if the subscription status update fails, improving overall reliability of the feature. * Add unsubscribe participant feature to API and frontend - Implemented a new API endpoint to handle participant unsubscription, allowing updates to email preferences securely. - Created a new function in the frontend to manage the unsubscribe process, integrating it with the existing query hooks. - Removed the deprecated useUpdateProjectContacts hook to streamline the unsubscribe functionality. - Enhanced the ProjectUnsubscribe component to utilize the new API for managing participant email preferences, improving user experience and engagement. * Refactor ProjectUnsubscribe button to use Trans component for localization - Updated the unsubscribe button to utilize the Trans component for improved internationalization support. - Added a loading prop to the button for better user feedback during the unsubscribe process. * Update localization files for participant post-conversation and unsubscribe features - Added new translations for various UI elements related to participant interactions, including buttons and notifications in German, English, Spanish, French, and Dutch. - Enhanced user feedback messages for actions such as submitting responses and unsubscribing from notifications. - Updated existing translations to ensure consistency and clarity across all supported languages. * Add email notification and report management features - Introduced new email notification flows for sending report updates, including "Send Email" and "Send Report Emails" operations. - Implemented various conditions and triggers for email notifications based on project report status and language preferences. - Added permissions for managing project report notifications, allowing for create, read, and update actions. - Updated the schema for the 'quote' collection to change the embedding field type to 'text' and adjusted its display properties. - Removed the deprecated 'centroid_embedding' field from the 'aspect' collection, streamlining the data structure. * Update echo/directus/sync/collections/operations.json Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Update .gitignore to exclude neo4j_data directory --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
2 participants
Summary by CodeRabbit
New Features
Improvements