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

Adding polling comments & Removing votes from ballot #272

Merged
merged 23 commits into from
Dec 15, 2021

Conversation

rafinskipg
Copy link
Contributor

@rafinskipg rafinskipg commented Dec 9, 2021

Link to Shortcut ticket:

https://app.shortcut.com/dux-makerdao/story/44/adding-comments-to-polling
https://app.shortcut.com/dux-makerdao/story/810/allow-removing-poll-votes-from-ballot

What does this PR do?

Adds polling comments.

  • Creates two buttons to sign the message + post the transaction
  • Creates API endpoints for polling comments
  • Refactors comments into it's own module
  • Display polling comments + cross data with vote history
  • Adds sharing button on the comment list (disabled for now)
  • Allow to remove votes from ballot
  • Fix a bug where the total deposited on the chief was not updated
  • Allows to post comments on executives on other networks different than mainnet

Steps for testing:

Go to a poll:

  • put some comment and submit your ballot
  • check all the executive comments

Screenshots (if relevant):

image

Any additional helpful information?:

To connect to the DB locally you will need the environment variables for mongoDb, kindly ask for them

Add a GIF:

![image](https://user-images.githubusercontent.com/1152768/145597696-65365a85-42d0-45ae-89ee-fc3228a0083d.png)

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #44: Adding comments to Polling.

@vercel
Copy link

vercel bot commented Dec 9, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/dux-core-unit/governance-portal-v2/H61upSaR3AAVzUxYF63MQ8e6XWRX
✅ Preview: https://governance-portal-v2-git-feature-sc-44addi-dc100e-dux-core-unit.vercel.app

@codecov
Copy link

codecov bot commented Dec 10, 2021

Codecov Report

Merging #272 (84857b7) into develop (fba92b9) will decrease coverage by 0.36%.
The diff coverage is 1.47%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #272      +/-   ##
===========================================
- Coverage    32.89%   32.52%   -0.37%     
===========================================
  Files           59       61       +2     
  Lines         1523     1534      +11     
  Branches       510      510              
===========================================
- Hits           501      499       -2     
- Misses        1015     1028      +13     
  Partials         7        7              
Impacted Files Coverage Δ
lib/maker/index.ts 30.64% <0.00%> (ø)
pages/api/address/[address]/index.ts 0.00% <0.00%> (ø)
pages/api/address/[address]/stats.ts 0.00% <ø> (ø)
pages/api/comments/executive/[address].ts 0.00% <0.00%> (ø)
pages/api/comments/executive/add/[address].ts 0.00% <0.00%> (ø)
pages/api/comments/polling/[poll-id].ts 0.00% <0.00%> (ø)
pages/api/comments/polling/add.ts 0.00% <0.00%> (ø)
pages/api/polling/[slug].ts 0.00% <ø> (ø)
pages/api/polling/all-polls.ts 0.00% <ø> (ø)
pages/api/polling/tally/[poll-id].ts 0.00% <ø> (ø)
... and 4 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 fba92b9...84857b7. Read the comment docs.

@rafinskipg rafinskipg changed the title Adding polling comments Adding polling comments & Removing votes from ballot Dec 13, 2021
…omments API call, include correct address balance , fix executive contract deposit refresh UI
@@ -0,0 +1,48 @@
import { SupportedNetworks } from 'lib/constants';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

refactored this from the api and moved here

@@ -0,0 +1,47 @@
import { Delegate } from '../types';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored the delegate avatar + name into a component for reusing between comments and delegate cards

@@ -0,0 +1,350 @@
import { useBreakpointIndex } from '@theme-ui/match-media';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

extracted from the gigantic vote modal + added some logic for signing the message

@@ -0,0 +1,100 @@
import { useState } from 'react';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved from the previous voteModal file, most of the logic is the same, i removed one useMemo and just rendered normally

const { mutate: mutateComments } = useExecutiveComments(proposal.address);

const [comment, setComment] = useState('');
const [signedMessage, setSignedMessage] = useState('');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is new

}, []);

const [hatChecked, setHatChecked] = useState(true);
const { data: currentSlate, mutate: mutateVotedProposals } = useVotedProposals();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

brought the currentSlate from the hook instead of nesting it 3 levels from the previous parent props

@@ -115,6 +115,8 @@ const ModalContent = ({ address, voteProxy, close, ...props }) => {

const txId = await track(freeTxCreator, 'Withdrawing MKR', {
mined: txId => {
// Mutate locked amount
mutateLocked();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to mutate and refresh the total locked MKR in the contract, was not working before

<Button
onClick={() => {
removeFromBallot(poll.pollId);
}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New remove from ballot functionality

@@ -156,6 +165,8 @@ export default function PollOverviewCard({ poll, reviewPage, showVoting, ...prop
)}
</Flex>
</Box>

{children && <Box>{children}</Box>}
</Box>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Children used to add the comment form

@@ -0,0 +1,59 @@
import useBallotStore from '../stores/ballotStore';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These buttons sign and submit the ballot if there are multiple comments

@rafinskipg rafinskipg marked this pull request as ready for review December 14, 2021 18:25
@@ -130,7 +130,7 @@ function isTestnet(): boolean {
return getNetwork() === SupportedNetworks.TESTNET || !!config.TESTNET;
}

async function personalSign(message) {
async function personalSign(message: string): Promise<any> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Safety first 👷‍♂️

modules/address/components/AddressDetail.tsx Outdated Show resolved Hide resolved
modules/comments/api/getExecutiveComments.ts Show resolved Hide resolved
Copy link
Collaborator

@adamgoth adamgoth left a comment

Choose a reason for hiding this comment

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

Looks good! Left some minor comments.

I was able to confirm two possible bugs.

  1. Updating my poll vote with a new comment doesn't reflect the latest comment. It showed the previous one.

  2. The comment is getting saved to the DB before the transaction is mined. I cancelled the tx before it was mined but the comment still got saved.

modules/polling/stores/ballotStore.ts Outdated Show resolved Hide resolved
body: JSON.stringify(commentsRequest)
})
.then(() => {
console.log('comment successfully added');
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this log is necessary for production

pages/polling/review.tsx Outdated Show resolved Hide resolved
@rafinskipg rafinskipg merged commit c5e911a into develop Dec 15, 2021
@rafinskipg rafinskipg deleted the feature/sc-44/adding-comments-to-polling branch December 15, 2021 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants