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(console): delete app modal checks custom domain config #2373

Merged
merged 4 commits into from
Jun 12, 2023

Conversation

szkl
Copy link
Contributor

@szkl szkl commented Jun 7, 2023

Description

The delete application modal displays a message that application has a custom
domain configured.

Related Issues

Testing

  • Manual

Checklist

  • I have read the CONTRIBUTING guidelines
  • I have tested my code (manually and/or automated if applicable)
  • I have updated the documentation (if necessary)

@szkl szkl self-assigned this Jun 7, 2023
@szkl szkl added the enhancement Indicates new feature requests label Jun 7, 2023
@szkl
Copy link
Contributor Author

szkl commented Jun 7, 2023

image

before you can delete the application.
</Text>

<Link to={`/apps/${appDetails.clientId}/domain-wip`} className="self-end">
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be set to final route as part of #2374

betimshahini
betimshahini previously approved these changes Jun 7, 2023
@betimshahini betimshahini dismissed their stale review June 7, 2023 07:38

Missing logic in delete route

<HasCustomDomain appDetails={appDetails}></HasCustomDomain>
)}
{!hasCustomDomain && (
<DeleteModalAppForm
Copy link
Contributor

Choose a reason for hiding this comment

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

The form inside DeleteModalAppForm makes a post to /apps/delete. This API route also needs to do this check on the backend and throw an error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@szkl szkl force-pushed the feat/console/delete-app/custom-domain branch from 37f8309 to b4d56ed Compare June 7, 2023 09:19
@szkl szkl requested a review from betimshahini June 7, 2023 09:21
@@ -28,6 +29,11 @@ export const deleteApp = async ({
ctx.StarbaseApp
)

if (await appDO.storage.get('customDomain'))
throw new BadRequestError({
message: 'HAS_CUSTOM_DOMAIN',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code property is restricted but error objects could have another property for enum-like names. In this case, the error representation doesn't depend on the message in the object so using the code name in the message property is pragmatic.

@szkl szkl force-pushed the feat/console/delete-app/custom-domain branch 4 times, most recently from 27c0844 to f567f93 Compare June 8, 2023 13:13
@szkl szkl force-pushed the feat/console/delete-app/custom-domain branch from f567f93 to 0654794 Compare June 9, 2023 19:24
@szkl szkl force-pushed the feat/console/delete-app/custom-domain branch from 0654794 to fe9a37a Compare June 9, 2023 19:27
@szkl
Copy link
Contributor Author

szkl commented Jun 9, 2023

Updated.

@maurerbot maurerbot merged commit 7c71148 into main Jun 12, 2023
@maurerbot maurerbot deleted the feat/console/delete-app/custom-domain branch June 12, 2023 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(console): Deleting apps with custom domains throws error
3 participants