fix(predicate): adds exclusive method to get user notification data

[pedroanastacio]

Description

The endpoint GET - /predicate/:predicateId was returning more information than it should have, in this case the user's email address. To obtain notification information about a user, an internal method must be created in the API so as not to expose critical information.}

Summary

• Adds getNotificationInfo method to obtain user notification information.
• Moves email sending logic to Notifications service.
• Removes email and notify from the return of the endpoint GET - /predicate/:predicateId.

Checklist

• I reviewed my PR code before submitting
• I ensured that the implementation is working correctly and did not impact other parts of the app
• I mentioned the PR link in the task

[guimroque]

🔴 CRITICAL: Test assertion will fail due to removed fields

Problem: The test expects element.owner to match member exactly, but member now excludes email and notify fields that were removed from the predicate endpoint. This will cause a deep equality assertion failure.

Suggestion: Update the assertion to only compare the fields that are actually returned by the endpoint, or create a filtered comparison object excluding the removed fields.

[pedroanastacio]

The email and notify properties are not returned to the owner of the predicate, so there is no error in the test caused by removing the properties from the return of the findById method, since the members do not have them either.

[guimroque]

🟡 IMPORTANT: Method lacks proper documentation

Problem: The new getNotificationInfo method is missing JSDoc documentation explaining its purpose, parameters, and return value. This is especially important since it's handling sensitive user data.

Suggestion: Add JSDoc documentation:

/**
 * Retrieves notification preferences and email for specified users
 * Used internally for sending notifications without exposing sensitive data
 * @param ids - Array of user IDs to fetch notification info for
 * @returns Promise<INotificationInfo[]> - User notification data (id, name, notify, email)
 */

[pedroanastacio]

Fixed: 823b599

[guimroque]

🔵 SUGGESTION: Early return condition could be more explicit

Problem: The early return for empty array is good, but the condition could be more descriptive for maintainability.

Suggestion: Consider adding a comment or making the condition more explicit:

if (membersToNotify.length === 0) {
  logger.debug('[VAULT_CREATE] No members to notify, skipping notifications');
  return;
}

[pedroanastacio]

Fixed: 139a738

[guimroque]

Code Review

Summary

This PR successfully addresses a security vulnerability by preventing exposure of user email addresses in the predicate endpoint. The refactoring moves notification logic to a dedicated service and introduces a new method to safely retrieve user notification data.

Strengths

• Excellent security fix removing email exposure from public endpoint
• Clean separation of concerns moving notification logic to NotificationService
• Proper error handling with structured logging
• Good use of parallel processing for notifications and emails

Issues

• 1 critical, 1 important, 1 suggestion