Skip to content

Remove create email api function from backend package #2548

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

Nikpolik
Copy link
Contributor

@Nikpolik Nikpolik commented Jan 10, 2024

Description

Remove createEmail function from @clerk/backend.

The equivalent /emails Backend API endpoint will also be dropped in the future, since this feature will no longer be available for new instances.

For a brief period it will still be accessible for instances that have used it in the past 7
days.

New instances will get a 403 forbidden response if they try to access it.

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Packages affected

  • @clerk/backend
  • @clerk/chrome-extension
  • @clerk/clerk-js
  • @clerk/clerk-expo
  • @clerk/fastify
  • gatsby-plugin-clerk
  • @clerk/localizations
  • @clerk/nextjs
  • @clerk/clerk-react
  • @clerk/remix
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/themes
  • @clerk/types
  • build/tooling/chore

Copy link

changeset-bot bot commented Jan 10, 2024

🦋 Changeset detected

Latest commit: c167623

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages
Name Type
@clerk/backend Patch
@clerk/fastify Patch
gatsby-plugin-clerk Patch
@clerk/nextjs Patch
@clerk/remix Patch
@clerk/clerk-sdk-node Patch
@clerk/elements Patch

Not sure what this means? Click here to learn what changesets are.

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

@Nikpolik Nikpolik self-assigned this Jan 10, 2024
@Nikpolik Nikpolik changed the title feat(backend): Remove create email api Remove create email api function from backend package Jan 10, 2024
@Nikpolik Nikpolik force-pushed the nikpolik/core-1382-drop-v1-create-email branch 2 times, most recently from 4f7a76f to 52ed204 Compare January 11, 2024 11:47
@Nikpolik Nikpolik marked this pull request as ready for review January 11, 2024 11:47
@Nikpolik Nikpolik mentioned this pull request Jan 11, 2024
24 tasks
@panteliselef panteliselef requested a review from dimkl January 11, 2024 12:03

Remove createEmail function from @clerk/backend.

The corresponding endpoint is also being removed from the API.
Copy link
Member

Choose a reason for hiding this comment

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

Discussed with @Nikpolik offline and we will not be providing an alternative method for the time being.

Can we please add a note informing any customers currently using this to reach out to our support team if they wish to update to v5?

Suggested change
The corresponding endpoint is also being removed from the API.
The `emails` endpoint helper and the corresponding `createEmail` method have been removed from the `@clerk/backend` SDK and `apiClint.emails.createEmail` will no longer be available. We will not be providing an alternative method for creating and sending emails directly from our JavaScript SDKs with this release. If you are currently using `createEmail` and you wish to update to the latest SDK version, please reach out to our support team (https://clerk.com/support) so we can assist you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Much better message thanks!

@@ -88,39 +86,6 @@ export default (QUnit: QUnit) => {
);
});

test('executes a successful backend API request for a resource that contains data (key related to pagination)', async assert => {
Copy link
Contributor

Choose a reason for hiding this comment

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

🔧 Instead of removing this test, replace the apiClient.emails.createEmail(body); with another API call. This test is not related to the EmailAPI, it was just used as an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it possible this test is no longer needed? 🤔 After looking around only SMSMessage and Emails contain the property readonly data?: Record<string, any> | null that is being checked here.

To be completely honest I am not 100% sure what is being tested here 😓 And whats the differences between this test and the first one that executes a successful backend API request for a single resource and parses the response

Copy link
Contributor

Choose a reason for hiding this comment

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

You are correct. This case is no longer applicable. let's drop it.

Copy link
Contributor

@dimkl dimkl left a comment

Choose a reason for hiding this comment

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

Please review the comment in the test file

@Nikpolik Nikpolik force-pushed the nikpolik/core-1382-drop-v1-create-email branch from 52ed204 to fd7ff13 Compare January 11, 2024 15:26
The corresponding api route is also being deprecated.
@nikosdouvlis nikosdouvlis force-pushed the nikpolik/core-1382-drop-v1-create-email branch from fd7ff13 to c167623 Compare January 12, 2024 00:03
@nikosdouvlis nikosdouvlis merged commit 935b088 into main Jan 12, 2024
@nikosdouvlis nikosdouvlis deleted the nikpolik/core-1382-drop-v1-create-email branch January 12, 2024 00:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants