add [tasks] - push notifications when assigned a task#769
Merged
danielkweon merged 20 commits intomainfrom Dec 31, 2025
Merged
Conversation
danielkweon
changed the base branch from
main
to
daniel/m-5426--add-tasks-quick-create-menu-hexagon-migration-notification-permissions
Base automatically changed from
daniel/m-5426--add-tasks-quick-create-menu-hexagon-migration-notification-permissions
to
main
December 30, 2025 20:56
danielkweon
force-pushed
the
daniel/m-5426--add-tasks-quick-create-menu-hexagon-migration-notifications
branch
from
02ecbf3 to
c49048d
Compare
- Add grant_entity_permissions method to PermissionService trait - Implement grant_entity_permissions in PermissionServiceImpl using upsert_user_item_access_bulk - Update handle_task_assignee_permissions to grant edit permissions to assignees - Add macro_db_client dependency to properties crate - Tasks are granted edit permissions when assignees are updated
- Add TaskAssigned variant to NotificationEvent enum - Add TaskAssignedMetadata struct with task_id, task_name, and assigned_by fields - Update metadata_json and try_from_type_and_meta methods to handle TaskAssigned - Add impl_notification_metadata macro for TaskAssignedMetadata
- Add NotificationService trait to domain ports - Defines send_notification method for sending notification messages - Includes mockall support for testing
- Add NotificationServiceImpl using MacroNotifyClient - Implement NotificationService trait with send_notification method - Add macro_notify and model_notifications dependencies - Export NotificationService and NotificationServiceImpl from lib.rs - Add notification_service module to outbound
…eImpl - Add NotificationService generic parameter N to PropertiesServiceImpl - Add optional notification_service field to PropertiesServiceImpl - Update constructor to accept optional NotificationService - Update task_properties impl to include NotificationService generic - NotificationService is optional, allowing None for tests or when not needed
…nd document_storage_service - Initialize MacroNotifyClient in properties_service main.rs - Create NotificationServiceImpl and pass to PropertiesServiceImpl - Update PropertiesService type signature in properties_service context.rs - Add macro_notify dependency to properties_service Cargo.toml - Update document_storage_service to use NotificationService - Both services now pass NotificationServiceImpl to PropertiesServiceImpl
- Add MockNotificationService parameter to PropertiesServiceImpl::new calls - Tests now pass None::<MockNotificationService> for notification service - Maintains test compatibility with optional NotificationService
… entities Both Task and Document entities are stored in the Document table (tasks are documents with sub_type='task'), so they use the same query logic. Updated the implementation to handle both entity types in a single match arm.
- Send notifications to new assignees only (filters out existing assignees) - Filter out the assigner from notification recipients - Send notifications in parallel using futures::future::join_all - Grant edit permissions to all assignees (new and existing) - Get task name from repository for notification metadata - Handle edge cases: empty assignees, missing services, invalid user IDs
…ssions - Test permission granting for all assignees - Test notification sending to new assignees only - Test filtering out assigner from notifications - Test edge cases: empty assignees, no service, no new assignees - Test integration: both handlers called correctly - Test clearing assignees (None value)
…sions_to_task Make the method task-specific by: - Renaming to grant_permissions_to_task for clarity - Removing entity_type parameter (always Task) - Renaming entity_id parameter to task_id - Simplifying implementation (no entity_type matching needed)
…d show task name in browser title
danielkweon
force-pushed
the
daniel/m-5426--add-tasks-quick-create-menu-hexagon-migration-notifications
branch
from
c49048d to
78db6fb
Compare
synoet
approved these changes
Dec 31, 2025
Contributor
synoet
left a comment
synoet
left a comment
There was a problem hiding this comment.
Mostly Fine. Left a bunch of nits. Fix at your convenience.
| const hasTaskName = m.taskName != null && m.taskName.trim() !== ''; | ||
| return { | ||
| type: n.notificationEventType, | ||
| actor: m.assignedBy ? { id: m.assignedBy } : undefined, |
Contributor
There was a problem hiding this comment.
Is something wrong with type gen ? Shouldn't this always be defined ?
Contributor
Author
There was a problem hiding this comment.
other notifications also have undefined actors. i fixed up target to never be undefined
rust/cloud-storage/properties/src/domain/service_impl/task_properties.rs
Outdated
Show resolved
Hide resolved
rust/cloud-storage/properties/src/domain/service_impl/task_properties.rs
Outdated
Show resolved
Hide resolved
rust/cloud-storage/properties/src/domain/service_impl/task_properties.rs
Outdated
Show resolved
Hide resolved
rust/cloud-storage/properties/src/domain/service_impl/task_properties.rs
Outdated
Show resolved
Hide resolved
danielkweon
force-pushed
the
daniel/m-5426--add-tasks-quick-create-menu-hexagon-migration-notifications
branch
from
86ef7fd to
192460d
Compare
danielkweon
deleted the
daniel/m-5426--add-tasks-quick-create-menu-hexagon-migration-notifications
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
add [tasks] - add notifications when assigned a task