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

IMN 606 - BFF purpose scaffolding #645

Merged
merged 13 commits into from
Jun 18, 2024
Merged

Conversation

MalpenZibo
Copy link
Collaborator

@MalpenZibo MalpenZibo commented Jun 13, 2024

IMN-606

How we currently handle the "zodiosPatch" for the BFF package is not optimal. Before implementing other solutions that require refactoring on other packages I'd like to discuss the openApi spec first strategy.

Copy link
Collaborator

@ecamellini ecamellini left a comment

Choose a reason for hiding this comment

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

Left some minor comments, looks good!

packages/bff/src/providers/clientProvider.ts Outdated Show resolved Hide resolved
packages/bff/src/providers/clientProvider.ts Outdated Show resolved Hide resolved
packages/bff/src/model/domain/errors.ts Outdated Show resolved Hide resolved
packages/models/src/errors.ts Outdated Show resolved Hide resolved
Comment on lines +26 to +27
headers: { ...requestHeaders },
withCredentials: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will these two lines need to be duplicated in each client call? We could think about a way to avoid this and also the duplication of the definition of requestHeaders in each router call, even in a further PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't have a good solution right now, the only idea that I have is to wrap the client request in some way to inject these parameters

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

packages/bff/src/services/purposeService.ts Outdated Show resolved Hide resolved
packages/bff/.env Outdated Show resolved Hide resolved
@MalpenZibo MalpenZibo merged commit 610b181 into main Jun 18, 2024
8 checks passed
@MalpenZibo MalpenZibo deleted the IMN-606_bff-purpose-router branch June 18, 2024 09:29
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