Skip to content

fix: convert mailer endpoint to new pattern#35712

Open
rishitha4303 wants to merge 23 commits into
RocketChat:developfrom
rishitha4303:rishitha-mailer-fix
Open

fix: convert mailer endpoint to new pattern#35712
rishitha4303 wants to merge 23 commits into
RocketChat:developfrom
rishitha4303:rishitha-mailer-fix

Conversation

@rishitha4303
Copy link
Copy Markdown

@rishitha4303 rishitha4303 commented Apr 7, 2025

This PR updates the /mailer endpoint to use the modern API.v1.post() structure instead of the deprecated API.v1.addRoute().
Changes include:

  1. Migrated endpoint to new pattern

2.Added ajv response validation schema

  1. Removed old/unused import statements

4.Ensured permissions and parameter validation remain intact

Tested:
Verified functionality locally after changes

Confirmed schema validation works as expected

Endpoint returns expected response format ({ success: true })

💬 GSoC Context:
This change contributes to my GSoC proposal by helping modernize Rocket.Chat’s API routes for maintainability and consistency.


This pull request refactors the /mailer and /mailer.unsubscribe API endpoints in the apps/meteor/app/api/server/v1/mailer.ts file to adopt the newer API.v1.post method, replacing the older API.v1.addRoute. It also introduces explicit response validation using ajv for both endpoints and includes temporary debugging logs. Additionally, a new tsconfig-esm.json file is added to the packages/fuselage-ui-kit directory.

@dionisio-bot
Copy link
Copy Markdown
Contributor

dionisio-bot Bot commented Apr 7, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 7, 2025

⚠️ No Changeset found

Latest commit: 4860ba1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 7, 2025

CLA assistant check
All committers have signed the CLA.

@rishitha4303 rishitha4303 changed the title Migrate /mailer endpoint to new API endpoint pattern fix: convert mailer endpoint to new pattern Apr 7, 2025
@rishitha4303
Copy link
Copy Markdown
Author

Hi! This PR updates the mailer endpoint to the new pattern. Kindly review when possible. Thanks a lot!

@kody-ai
Copy link
Copy Markdown

kody-ai Bot commented Apr 19, 2025

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration
Interacting with Kody
  • Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

  • Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration
Review Options

The following review options are enabled or disabled:

Options Enabled
Security
Code Style
Kody Rules
Refactoring
Error Handling
Maintainability
Potential Issues
Documentation And Comments
Performance And Optimization
Breaking Changes

Access your configuration settings here.

Comment thread apps/meteor/app/api/server/v1/mailer.ts
Comment thread apps/meteor/app/api/server/v1/mailer.ts Outdated
Comment thread apps/meteor/app/api/server/v1/mailer.ts
Comment thread apps/meteor/app/api/server/v1/mailer.ts
Comment thread apps/meteor/app/api/server/v1/mailer.ts Outdated
Comment thread apps/meteor/app/api/server/v1/mailer.ts
Comment thread packages/fuselage-ui-kit/tsconfig-esm.json Outdated
@rishitha4303 rishitha4303 requested a review from cardoso April 19, 2025 17:53
Copy link
Copy Markdown
Member

@cardoso cardoso left a comment

Choose a reason for hiding this comment

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

@rishitha4303 please keep your changes confined to apps/meteor/app/api/server/v1/mailer.ts.

Comment thread .gitignore Outdated
Comment thread packages/fuselage-ui-kit/tsconfig-esm.json Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.16%. Comparing base (d8eb824) to head (d8a8c4b).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #35712   +/-   ##
========================================
  Coverage    61.16%   61.16%           
========================================
  Files         3003     3003           
  Lines        71350    71350           
  Branches     16333    16333           
========================================
  Hits         43638    43638           
- Misses       24745    24746    +1     
+ Partials      2967     2966    -1     
Flag Coverage Δ
unit 74.99% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rishitha4303 rishitha4303 requested a review from cardoso April 20, 2025 19:07
@rishitha4303
Copy link
Copy Markdown
Author

@kody updated the response schema as suggested. Can you re-check?

@cardoso
Copy link
Copy Markdown
Member

cardoso commented Apr 23, 2025

@rishitha4303 please format the apps/meteor/app/api/server/v1/mailer.ts file using prettier. If you use vscode this extension can do it for you: https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode

Comment on lines -2 to -5

import Ajv from 'ajv';
import { API } from '../api';
import { sendMail } from '../../../mail-messages/server/functions/sendMail';
import { Mailer } from '../../../mail-messages/server/lib/Mailer';
import { API } from '../api';
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.

The import order of API here is an unneeded change. Please install the eslint vscode extension and fix the lint issues: https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint

Copy link
Copy Markdown
Author

@rishitha4303 rishitha4303 Apr 25, 2025

Choose a reason for hiding this comment

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

I installed the ESLint VSCode extension and checked the mailer.ts file. After running it, linting issues were found.
but it seems there are some other linting issues preventing to next step, they might be in other files. I'm currently checking the rest

@rishitha4303 rishitha4303 requested a review from cardoso April 25, 2025 14:29
@ggazzo ggazzo requested a review from a team as a code owner March 5, 2026 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants