search

openwallet-foundation / credo-ts-ext

Extension libraries for Credo
https://credo.js.org
Apache License 2.0
29 stars 38 forks source link

chore: release @credo-ts/rest 0.10.0 #240

Closed github-actions[bot] closed 7 months ago

github-actions[bot] commented 8 months ago

:robot: I have created a release *beep* *boop*

0.10.0 (2024-03-31)

⚠ BREAKING CHANGES

Features

Bug Fixes

Miscellaneous Chores

This PR was generated with Release Please. See documentation.

TimoGlastra commented 7 months ago

I think implementing it as auth in TSOA is fine as it's really only internal. Or do you also dislike that?

Or is it the header? I do think a header is the simplest approach but I'm open to other solutions.

Please let me know which parts you don't like (x-tenant-id header, using tsoa authentication logic for this (which is used internally), or the security mention in the swagger UI). Because all of them can be solved independently of the others

genaris commented 7 months ago

I think implementing it as auth in TSOA is fine as it's really only internal. Or do you also dislike that?

Or is it the header? I do think a header is the simplest approach but I'm open to other solutions.

Please let me know which parts you don't like (x-tenant-id header, using tsoa authentication logic for this (which is used internally), or the security mention in the swagger UI). Because all of them can be solved independently of the others

Yeah using the parameter from the header is absolutely fine. I was referring mostly to the fact of calling it 'auth' because it can lead to some confusions, like the question raised by Warren when you presented this in last Credo contributors call.

In practice, as you say, it is just something noticeable in Swagger UI, so it's not that important. Maybe the tenants header could be handled by a @Middleware, but I don't know if it will be easily possible to specify the header parameter in all routes as it is when using securityDefinitions.

TimoGlastra commented 7 months ago

@genaris I've made improvements in #263, please let me know what you think! I can make ammendments in a follow up version if you would like to see it differently. The behaviour in general stays the same: x-tenant-id header to specify which tenant. But we can tweak how it's shown in SwaggerUI/openapi.json as well as how it works internally (kept it as authentication for now as it applies perfectly for what we're trying to do)

codecov[bot] commented 7 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 57.95%. Comparing base (658cadc) to head (2aa6451).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #240 +/- ## ======================================= Coverage 57.95% 57.95% ======================================= Files 95 95 Lines 2528 2528 Branches 513 513 ======================================= Hits 1465 1465 Misses 983 983 Partials 80 80 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 7 months ago

:robot: Release is at https://github.com/openwallet-foundation/credo-ts-ext/releases/tag/rest-v0.10.0 :sunflower:

genaris commented 7 months ago

Nicely done, @TimoGlastra !