Skip to content
This repository was archived by the owner on May 22, 2025. It is now read-only.

DisputableVoting: Extend vote model functionalities#194

Merged
facuspagnuolo merged 1 commit intomasterfrom
disputable_voting/add_more_details_to_vote_model
Aug 11, 2020
Merged

DisputableVoting: Extend vote model functionalities#194
facuspagnuolo merged 1 commit intomasterfrom
disputable_voting/add_more_details_to_vote_model

Conversation

@facuspagnuolo
Copy link
Copy Markdown
Contributor

@facuspagnuolo facuspagnuolo commented Aug 7, 2020

This PR adds the following functionalities for the Vote model:

  • Expose vote duration
  • Compute end date
  • Vote results
  • Querying the collateral requirements
  • Knowing whether a voter has voted or not

@facuspagnuolo facuspagnuolo self-assigned this Aug 7, 2020
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 7, 2020

Codecov Report

Merging #194 into master will increase coverage by 4.44%.
The diff coverage is 77.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #194      +/-   ##
==========================================
+ Coverage   32.68%   37.12%   +4.44%     
==========================================
  Files          96      107      +11     
  Lines        1741     1907     +166     
  Branches      273      286      +13     
==========================================
+ Hits          569      708     +139     
- Misses       1172     1199      +27     
Flag Coverage Δ
#unittests 37.12% <77.04%> (+4.44%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ct-voting-disputable/src/thegraph/parsers/votes.ts 86.66% <ø> (ø)
...t-voting-disputable/src/models/DisputableVoting.ts 22.22% <25.00%> (+19.78%) ⬆️
...onnect-voting-disputable/src/thegraph/connector.ts 51.89% <57.14%> (+3.95%) ⬆️
...kages/connect-voting-disputable/src/models/Vote.ts 87.75% <82.60%> (-4.56%) ⬇️
...ble/src/thegraph/parsers/collateralRequirements.ts 85.71% <85.71%> (ø)
packages/connect-voting-disputable/src/index.ts 100.00% <100.00%> (ø)
...ing-disputable/src/models/CollateralRequirement.ts 100.00% <100.00%> (ø)
...oting-disputable/src/thegraph/parsers/castVotes.ts 92.30% <100.00%> (+25.64%) ⬆️
...ct-voting-disputable/src/thegraph/parsers/index.ts 100.00% <100.00%> (ø)
...ct-voting-disputable/src/thegraph/queries/index.ts 100.00% <100.00%> (ø)
... and 19 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 28b0bd6...3c55e87. Read the comment docs.

Copy link
Copy Markdown
Contributor

@0xGabi 0xGabi left a comment

Choose a reason for hiding this comment

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

Looking great, I like how you started extending the surface area that app connectors handle, doing more useful data transformations! 🙌

return this.votingPowerPct(this.nays)
}

votingPowerPct(num: string): string {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

this.script = data.script
}

get endDate(): string {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Leaving a comment to point a good utility on the Connectors layer.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It would be nice to have a better way to handle bignumber ops to be honest, a few helpers would be wonderful

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wonder what you have in mind? Maybe make sense to have a connect-utils, @bpierre what you think?

readonly challengeDuration: string

constructor(data: CollateralRequirementData, connector: IDisputableVotingConnector) {
this.#connector = connector
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder about having the connector object on every model. Maybe we should only include it on those models that implement queries/subscriptions requests?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I wasn't sure about that, but it makes sense, we can have it only if needed

@facuspagnuolo facuspagnuolo merged commit 628c0e9 into master Aug 11, 2020
@facuspagnuolo facuspagnuolo deleted the disputable_voting/add_more_details_to_vote_model branch August 11, 2020 23:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants