Skip to content
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

feat(core): add DELETE /configs/jwt-customizer API #5467

Merged
merged 3 commits into from
Mar 7, 2024

Conversation

darcyYe
Copy link
Contributor

@darcyYe darcyYe commented Mar 4, 2024

Summary

Add DELETE /configs/jwt-customizer/:tokenType API

Testing

Covered by unit tests and integration tests.

Checklist

  • .changeset
  • unit tests
  • integration tests
  • necessary TSDoc comments

Copy link

github-actions bot commented Mar 4, 2024

COMPARE TO master

Total Size Diff 📈 +2.32 KB

Diff by File
Name Diff
packages/core/src/queries/logto-config.ts 📈 +53 Bytes
packages/core/src/routes/logto-config.openapi.json 📈 +588 Bytes
packages/core/src/routes/logto-config.test.ts 📈 +389 Bytes
packages/core/src/routes/logto-config.ts 📈 +584 Bytes
packages/integration-tests/src/api/logto-config.ts 📈 +164 Bytes
packages/integration-tests/src/tests/api/logto-config.test.ts 📈 +701 Bytes

Copy link
Contributor

@simeng-li simeng-li left a comment

Choose a reason for hiding this comment

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

overall LGTM

packages/core/src/queries/logto-config.ts Outdated Show resolved Hide resolved
@darcyYe darcyYe force-pushed the yemq-log-8283-add-GET-configs-jwt-customizer-API branch 2 times, most recently from 02aa9cd to 6847e84 Compare March 6, 2024 05:10
Copy link
Contributor

@wangsijie wangsijie left a comment

Choose a reason for hiding this comment

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

Can we set the token to nullable, then there is no need for "post" and "delete", just "put", if the user want to delete, he can pass { "token": null } to "put" payload

@darcyYe darcyYe force-pushed the yemq-log-8283-add-GET-configs-jwt-customizer-API branch 2 times, most recently from 31c36c4 to e327754 Compare March 6, 2024 11:33
Base automatically changed from yemq-log-8283-add-GET-configs-jwt-customizer-API to master March 7, 2024 06:14
@github-actions github-actions bot added size/xl and removed size/m labels Mar 7, 2024
@darcyYe darcyYe force-pushed the yemq-log-8285-add-DELETE-configs-jwt-customizer-API branch from 3540633 to 29d7642 Compare March 7, 2024 06:25
@github-actions github-actions bot added size/m and removed size/xl labels Mar 7, 2024
@darcyYe
Copy link
Contributor Author

darcyYe commented Mar 7, 2024

Can we set the token to nullable, then there is no need for "post" and "delete", just "put", if the user want to delete, he can pass { "token": null } to "put" payload

The whole JWT-customization-related data is stored as a record in our logto_configs table. Besides tokenSample we also have script (which refers to code snippets), envVars and contextSample (which is a user-customized context data sample for testing purpose). If the user deletes all these fields, we should delete the record if it is no longer used.

@darcyYe darcyYe merged commit a00badc into master Mar 7, 2024
19 checks passed
@darcyYe darcyYe deleted the yemq-log-8285-add-DELETE-configs-jwt-customizer-API branch March 7, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

3 participants