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

fix(settlements)!: SettlementModel currency is mandatory #811

Merged
merged 11 commits into from
Feb 2, 2021

Conversation

vgenev
Copy link
Contributor

@vgenev vgenev commented Jan 22, 2021

fix: settlementModel has currency as required
BREAKING CHANGE: currency is mandatory on /post settlementModels
creation of settlementModel on startup has been removed

vgenev and others added 5 commits January 20, 2021 19:22
BREAKING CHANGE: currency is mandatory on /post settlementModels
creation of settlementModel on startup has been removed
@vgenev vgenev requested a review from shashi165 January 22, 2021 14:10
@vgenev vgenev self-assigned this Jan 22, 2021
@vgenev vgenev requested a review from mdebarros January 22, 2021 14:10
@vgenev vgenev changed the title Fix/#1945 cgs fix(settlements)!: SettlementModel currency is mandatory #808 Jan 22, 2021
@vgenev vgenev changed the title fix(settlements)!: SettlementModel currency is mandatory #808 fix(settlements)!: SettlementModel currency is mandatory Jan 22, 2021
Copy link
Member

@mdebarros mdebarros left a comment

Choose a reason for hiding this comment

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

It looks good! Some minor comments for you to address.

Also, please add an additional comment to the POST /settlementModels API that it will create any associated ledgerAccountTypes for every participant that matches the settlementModel's currency

@vgenev vgenev requested a review from mdebarros January 22, 2021 16:05
Copy link
Member

@mdebarros mdebarros left a comment

Choose a reason for hiding this comment

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

See minor comment.

@vgenev vgenev requested a review from mdebarros January 22, 2021 17:09
mdebarros
mdebarros previously approved these changes Jan 25, 2021
Copy link
Member

@mdebarros mdebarros left a comment

Choose a reason for hiding this comment

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

+1

@vgenev vgenev merged commit ebeb3d8 into master Feb 2, 2021
@lewisdaly
Copy link
Contributor

Nice title!

@lewisdaly lewisdaly deleted the fix/#1945-CGS branch March 1, 2021 12:46
ggrg pushed a commit to ggrg/central-ledger that referenced this pull request Mar 6, 2021
* WIP participantcurrency_pcl_unique error when creating participant

* WIP

* WIP

* allow multiple settlementModels

* fix: settlementModel has currency as required
BREAKING CHANGE: currency is mandatory on /post settlementModels
creation of settlementModel on startup has been removed

* added codeowners

* cleaned and added comments and descriptions

* added more info on API

* added a commentS

* dependencies update

Co-authored-by: Shashikant Hirugade <shashi.pioneer@gmail.com>
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