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

Certain Proposal does not show gracing period in hours/blocks (#3903) #3959

Closed
wants to merge 27 commits into from

Conversation

chrlschwb
Copy link
Contributor

fix #3903
Issue is that Set Membership Lead Invitation Quota Proposal does not show gracing period in hours/blocks.
Added the time counter part to
[pioneer\packages\ui\src\app\pages\Proposals\ProposalPreview.tsx]

@vercel
Copy link

vercel bot commented Dec 9, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dao ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 27, 2023 4:43pm
pioneer-2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 27, 2023 4:43pm
pioneer-2-storybook ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 27, 2023 4:43pm

@chrlschwb chrlschwb changed the title certain Proposal does not show gracing period in hours/blocks Certain Proposal does not show gracing period in hours/blocks (#3903) Dec 9, 2022
Copy link
Collaborator

@traumschule traumschule left a comment

Choose a reason for hiding this comment

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

Some tests need updating.

● ProposalPreview › Vote button › Member is a council member › Member has not voted yet
 | 
 |     Unable to find an element with the text: /Vote on Proposal/i. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

● ProposalPreview › Vote button › Member is a council member › Member has already voted
 | 
 |     Unable to find an element with the text: /Already voted/i. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

● ProposalPreview › "You voted for" section › Voted for rejecting
 | 
 |     Unable to find an element with the text: /You voted for:/i. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

@@ -2,6 +2,10 @@ import React, { useEffect, useMemo, useState } from 'react'
import { useLocation } from 'react-router'
import { useHistory, useParams } from 'react-router-dom'
import styled from 'styled-components'
import { TokenValueStat } from '@/common/components/statistics'//
import { useRewardPeriod } from '@/working-groups/hooks/useRewardPeriod'//
import { useOpening } from '@/working-groups/hooks/useOpening'//
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove comments

Comment on lines 5 to 7
import { TokenValueStat } from '@/common/components/statistics'//
import { useRewardPeriod } from '@/working-groups/hooks/useRewardPeriod'//
import { useOpening } from '@/working-groups/hooks/useOpening'//
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import { TokenValueStat } from '@/common/components/statistics'//
import { useRewardPeriod } from '@/working-groups/hooks/useRewardPeriod'//
import { useOpening } from '@/working-groups/hooks/useOpening'//
import { TokenValueStat } from '@/common/components/statistics'
import { useRewardPeriod } from '@/working-groups/hooks/useRewardPeriod'
import { useOpening } from '@/working-groups/hooks/useOpening'

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, you can just apply the suggestion, the rest looks good!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

applied changes

Copy link
Collaborator

@thesan thesan left a comment

Choose a reason for hiding this comment

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

In addition is #3903 still happening ?

It's not urgent but could you add a test for it on the ProposalPreview stories please ?
It would be nice to be able to the status with the args in these stories and infer the updates the from it, instead of the current way of setting the parameters.statuses. It would allow to quickly check most of the proposal preview the states (I'll try to do that when I get some time).

Comment on lines 163 to 170
<TokenValueStat
title={`Reward per ${rewardPeriod?.toString()} blocks`}
value={rewardPeriod?.mul(opening.rewardPerBlock)}
/>
<TokenValueStat
title="Minimal stake"
tooltipText="Minimal amount of tokens required to be staked for any applicant to such role."
value={opening.stake}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @chrlschwb I don't understand how does adding these statistics relates to #3903 ?

@@ -52,6 +56,9 @@ export const ProposalPreview = () => {
const votingRounds = useVotingRounds(proposal?.votes, proposal?.proposalStatusUpdates)
const [currentVotingRound, setVotingRound] = useState(0)

const { opening } = useOpening(urlParamToOpeningId(id))//
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also this is wrong the url parameter is a proposal id not a opening id. As a result all proposal pages will show a 404. E.g: https://dao-git-fork-chrlschwb-fix-3903-gracingperiod-joystream.vercel.app/#/proposals/preview/606

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-dev issue suitable for community-dev pipeline
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[USERSNAP] GRACING Period
4 participants