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

Add interceptors for auto-resolvers #6270

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

thomtrp
Copy link
Contributor

@thomtrp thomtrp commented Jul 15, 2024

Services exceptions are not catch when the endpoint comes from an auto-resolver.
We want to remove auto-resolver but it requires to implement pagination by ourselves.

As a quick fix, here are interceptors that will trigger the exception handler.
I had a hard time making it generic so I finally added one interceptor for each since this is not supposed to stay

@thomtrp thomtrp force-pushed the tt-add-interceptors-for-auto-resolvers branch from 89909ac to aa7e6ae Compare July 15, 2024 15:46
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

  • Added FieldMetadataGraphqlApiExceptionInterceptor in /packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.module.ts
  • Introduced /packages/twenty-server/src/engine/metadata-modules/field-metadata/interceptors/field-metadata-graphql-api-exception.interceptor.ts
  • Added ObjectMetadataGraphqlApiExceptionInterceptor in /packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.module.ts
  • Introduced /packages/twenty-server/src/engine/metadata-modules/object-metadata/interceptors/object-metadata-graphql-api-exception.interceptor.ts
  • Added RelationMetadataGraphqlApiExceptionInterceptor in /packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.module.ts
  • Introduced /packages/twenty-server/src/engine/metadata-modules/relation-metadata/interceptors/relation-metadata-graphql-api-exception.interceptor.ts

6 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

  • Added FieldMetadataGraphqlApiExceptionInterceptor in /packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.module.ts
  • Introduced /packages/twenty-server/src/engine/metadata-modules/field-metadata/interceptors/field-metadata-graphql-api-exception.interceptor.ts
  • Added ObjectMetadataGraphqlApiExceptionInterceptor in /packages/twenty-server/src/engine/metadata-modules/object-metadata/object-metadata.module.ts
  • Introduced /packages/twenty-server/src/engine/metadata-modules/object-metadata/interceptors/object-metadata-graphql-api-exception.interceptor.ts
  • Added RelationMetadataGraphqlApiExceptionInterceptor in /packages/twenty-server/src/engine/metadata-modules/relation-metadata/relation-metadata.module.ts

6 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

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

I'm surprised this couldn't be made generic but as you said it's not meant to stay so LGTM!

@thomtrp
Copy link
Contributor Author

thomtrp commented Jul 16, 2024

@Weiko not sure I will keep this or not. But I spent 2 hours yesterday to make it work with auto-resolvers and I could not. So I prefer work on it once we deprecate nestjs query

@thomtrp thomtrp merged commit 34cbba5 into main Jul 16, 2024
6 checks passed
@thomtrp thomtrp deleted the tt-add-interceptors-for-auto-resolvers branch July 16, 2024 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants