-
Notifications
You must be signed in to change notification settings - Fork 5
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(api): cost-surface: application logic #196
Conversation
This pull request is being automatically deployed with Vercel (learn more). marxan – ./app🔍 Inspect: https://vercel.com/vizzuality1/marxan/6TXFkn5dByLcDBL8gYiodK8zR8sc marxan-storybook – ./app🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/CuMFwmWfwvTqQmRZic5toUbCLvjy |
1da4309
to
cf20031
Compare
api/src/modules/scenarios/cost-surface/__mocks__/cost-surface-events-fake.ts
Show resolved
Hide resolved
import { CostSurfaceInputDto } from '../../analysis/entry-points/adjust-cost-surface-input'; | ||
|
||
export abstract class ResolvePuWithCost { | ||
abstract fromShapefile( |
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.
Which is the purpose of this class?
Are we introducing Hex. approach?
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.
Its just an interface what main logic will use.
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.
Dependency Inversion Principle
|
||
@Injectable() | ||
export class CostSurfaceFacade { | ||
constructor( | ||
private readonly adjustCostSurfaceService: AdjustCostSurface, | ||
private readonly proxyService: ProxyService, | ||
private readonly shapefileConverter: ResolvePuWithCost, |
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.
ResolvePuWithCost is instantiated as shapefileConverter? Naming is confusing for me
Can you give further explanation?
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.
@kgajowy Sorry, I forgot to click 'Start review...'
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.
@alexeh its input is a shapefile and returns base PU ids with cost. Suitable names are always appreciated ;)
9605002
to
35fcfd9
Compare
Cost Surface update - application logic
Overview
Part of
Users should be able to upload shapefiles of scenario cost surfaces
- subtask https://vizzuality.atlassian.net/browse/MARXAN-413Designs
Link to the related design prototypes (if applicable)
Testing instructions
Please explain how to test the PR: ID of a dataset, steps to reach the feature, etc.
Feature relevant tickets
Link to the related task manager tickets
Checklist before submitting
develop
.deploying to staging/production, please add brief testing instructions
to the deploy checklist (
docs/deployment-checklist.md
)