From ebc4b8ceee828531838f1842f54f204ab04d900d Mon Sep 17 00:00:00 2001 From: Will O'Beirne Date: Tue, 19 Feb 2019 16:42:40 -0500 Subject: [PATCH 1/2] Stricter milestone validaiton. Dont try to be smart with percentages. --- .../components/CreateFlow/Milestones.tsx | 26 +++++-------------- .../client/components/CreateFlow/Review.less | 8 ++++++ .../client/components/CreateFlow/Review.tsx | 6 +++-- .../client/components/CreateFlow/index.tsx | 2 +- frontend/client/modules/create/utils.ts | 16 +++++++++--- 5 files changed, 31 insertions(+), 27 deletions(-) diff --git a/frontend/client/components/CreateFlow/Milestones.tsx b/frontend/client/components/CreateFlow/Milestones.tsx index aca5d948..4222589d 100644 --- a/frontend/client/components/CreateFlow/Milestones.tsx +++ b/frontend/client/components/CreateFlow/Milestones.tsx @@ -19,7 +19,7 @@ const DEFAULT_STATE: State = { title: '', content: '', dateEstimated: moment().unix(), - payoutPercent: '100', + payoutPercent: '', immediatePayout: false, }, ], @@ -52,19 +52,7 @@ export default class CreateFlowMilestones extends React.Component addMilestone = () => { const { milestones: oldMilestones } = this.state; - const lastMilestone = oldMilestones[oldMilestones.length - 1]; - const halfPayout = parseInt(lastMilestone.payoutPercent, 10) / 2; - const milestones = [ - ...oldMilestones, - { - ...DEFAULT_STATE.milestones[0], - payoutPercent: halfPayout.toString(), - }, - ]; - milestones[milestones.length - 2] = { - ...lastMilestone, - payoutPercent: halfPayout.toString(), - }; + const milestones = [...oldMilestones, { ...DEFAULT_STATE.milestones[0] }]; this.setState({ milestones }); }; @@ -124,7 +112,7 @@ const MilestoneFields = ({
onChange(index, { ...milestone, content: ev.currentTarget.value }) @@ -176,14 +164,12 @@ const MilestoneFields = ({ } /> onChange(index, { ...milestone, - payoutPercent: ev.currentTarget.value || '0', + payoutPercent: ev.currentTarget.value, }) } addonAfter="%" diff --git a/frontend/client/components/CreateFlow/Review.less b/frontend/client/components/CreateFlow/Review.less index 193bf3d2..593bac5a 100644 --- a/frontend/client/components/CreateFlow/Review.less +++ b/frontend/client/components/CreateFlow/Review.less @@ -71,6 +71,10 @@ &-title { font-size: 1.4rem; margin: 0 0 0.3rem; + + em { + opacity: 0.7; + } } &-info { @@ -82,6 +86,10 @@ &-description { font-size: 1rem; + + em { + opacity: 0.7; + } } } diff --git a/frontend/client/components/CreateFlow/Review.tsx b/frontend/client/components/CreateFlow/Review.tsx index 6d7cc901..9935bdbe 100644 --- a/frontend/client/components/CreateFlow/Review.tsx +++ b/frontend/client/components/CreateFlow/Review.tsx @@ -180,13 +180,15 @@ const ReviewMilestones = ({ {milestones.map(m => (
-
{m.title}
+
{m.title || No title}
{moment(m.dateEstimated * 1000).format('MMMM YYYY')} {' – '} {m.payoutPercent}% of funds
-
{m.content}
+
+ {m.content || No description} +
))} diff --git a/frontend/client/components/CreateFlow/index.tsx b/frontend/client/components/CreateFlow/index.tsx index 2b1cf881..0cf25241 100644 --- a/frontend/client/components/CreateFlow/index.tsx +++ b/frontend/client/components/CreateFlow/index.tsx @@ -79,7 +79,7 @@ const STEP_INFO: { [key in CREATE_STEP]: StepInfo } = { title: 'Set up milestones for deliverables', subtitle: 'Make a timeline of when you’ll complete tasks, and receive funds', help: - 'Contributors are more willing to fund proposals with funding spread across multiple deadlines', + 'Contributors are more willing to fund proposals with funding spread across multiple milestones', component: Milestones, }, [CREATE_STEP.PAYMENT]: { diff --git a/frontend/client/modules/create/utils.ts b/frontend/client/modules/create/utils.ts index 023d9e57..3afab9e9 100644 --- a/frontend/client/modules/create/utils.ts +++ b/frontend/client/modules/create/utils.ts @@ -101,7 +101,7 @@ export function getCreateErrors( const milestoneErrors = milestones.map((ms, idx) => { if (!ms.title || !ms.content || !ms.dateEstimated || !ms.payoutPercent) { didMilestoneError = true; - return ''; + return `Milestone ${idx + 1} is missing fields`; } let err = ''; @@ -111,10 +111,18 @@ export function getCreateErrors( err = 'Description can only be 200 characters maximum'; } - // Last one shows percentage errors + if (Number.isNaN(parseInt(ms.payoutPercent, 10))) { + err = 'Payout percent must be a valid number'; + } + + // Last one shows percentage errors (if number is valid) cumulativeMilestonePct += parseInt(ms.payoutPercent, 10); - if (idx === milestones.length - 1 && cumulativeMilestonePct !== 100) { - err = `Payout percentages doesn’t add up to 100% (currently ${cumulativeMilestonePct}%)`; + if ( + idx === milestones.length - 1 && + cumulativeMilestonePct !== 100 && + !Number.isNaN(cumulativeMilestonePct) + ) { + err = `Payout percentages don’t add up to 100% (currently ${cumulativeMilestonePct}%)`; } didMilestoneError = didMilestoneError || !!err; From 55687a81af30d34828d430e2888943bc072fa377 Mon Sep 17 00:00:00 2001 From: Will O'Beirne Date: Wed, 20 Feb 2019 16:34:05 -0500 Subject: [PATCH 2/2] More granular milestone errors. --- frontend/client/modules/create/utils.ts | 35 ++++++++++++++----------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/frontend/client/modules/create/utils.ts b/frontend/client/modules/create/utils.ts index 3afab9e9..63938fc6 100644 --- a/frontend/client/modules/create/utils.ts +++ b/frontend/client/modules/create/utils.ts @@ -96,39 +96,42 @@ export function getCreateErrors( // Milestones if (milestones) { - let didMilestoneError = false; let cumulativeMilestonePct = 0; const milestoneErrors = milestones.map((ms, idx) => { - if (!ms.title || !ms.content || !ms.dateEstimated || !ms.payoutPercent) { - didMilestoneError = true; - return `Milestone ${idx + 1} is missing fields`; + if (!ms.title) { + return 'Title is required'; + } else if (ms.title.length > 40) { + return 'Title length can only be 40 characters maximum'; } - let err = ''; - if (ms.title.length > 40) { - err = 'Title length can only be 40 characters maximum'; + if (!ms.content) { + return 'Description is required'; } else if (ms.content.length > 200) { - err = 'Description can only be 200 characters maximum'; + return 'Description can only be 200 characters maximum'; } - if (Number.isNaN(parseInt(ms.payoutPercent, 10))) { - err = 'Payout percent must be a valid number'; + if (!ms.dateEstimated) { + return 'Estimate date is required'; } - // Last one shows percentage errors (if number is valid) + if (!ms.payoutPercent) { + return 'Payout percent is required'; + } else if (Number.isNaN(parseInt(ms.payoutPercent, 10))) { + return 'Payout percent must be a valid number'; + } + + // Last one shows percentage errors cumulativeMilestonePct += parseInt(ms.payoutPercent, 10); if ( idx === milestones.length - 1 && cumulativeMilestonePct !== 100 && !Number.isNaN(cumulativeMilestonePct) ) { - err = `Payout percentages don’t add up to 100% (currently ${cumulativeMilestonePct}%)`; + return `Payout percentages don’t add up to 100% (currently ${cumulativeMilestonePct}%)`; } - - didMilestoneError = didMilestoneError || !!err; - return err; + return ''; }); - if (didMilestoneError) { + if (milestoneErrors.find(err => !!err)) { errors.milestones = milestoneErrors; } }