-
Notifications
You must be signed in to change notification settings - Fork 41
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
Extract allowed apps to SwapOrderHelper #1479
Conversation
39165db
to
21694de
Compare
Pull Request Test Coverage Report for Build 8886059347Details
💛 - Coveralls |
/** | ||
* Checks if the app associated with an order is allowed. | ||
* | ||
* @param order |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: this is missing a description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 4f1ee88
@@ -16,6 +16,9 @@ import { SwapsRepositoryModule } from '@/domain/swaps/swaps-repository.module'; | |||
|
|||
@Injectable() | |||
export class SwapOrderHelper { | |||
private readonly restrictApps = | |||
this.configurationService.getOrThrow('swaps.restrictApps'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: restrictApps
is of type unknown
, I'd declare it as boolean:
private readonly restrictApps: boolean = ...
Or parametrise the function call:
this.configurationService.getOrThrow<boolean>('swaps.restrictApps')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 4f1ee88
Summary
Moves the "allowed apps" logic from the
SwapOrderMapper
to theSwapOrderHelper
. The mapper is specific to the transaction history/queue logic while the helper is a class that can be used across the different route.With this new functionality in
SwapOrderHelper
, we can now check if a specific application is allowed in other parts of the service.Changes
SwapOrderMapper
to theSwapOrderHelper