From f8a36ef84ab276bdb115925baec50eaf589a3f26 Mon Sep 17 00:00:00 2001 From: Will Franklin Date: Fri, 23 Oct 2020 10:53:08 +0100 Subject: [PATCH 1/8] Fix input mixin not always respecting id option --- src/views/mixins.pug | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/views/mixins.pug b/src/views/mixins.pug index 4aad35992..27620cd80 100644 --- a/src/views/mixins.pug +++ b/src/views/mixins.pug @@ -10,7 +10,7 @@ mixin input( type, label, name, opts ) unless opts.id - opts.id = name .form-group - label( for=name, class=( opts.left ? 'col-md-' + opts.left : 'col-md-2' ) ).control-label #{ label } + label( for=opts.id, class=( opts.left ? 'col-md-' + opts.left : 'col-md-2' ) ).control-label #{ label } div( class=( opts.right ? 'col-md-' + opts.right : 'col-md-3' ) ) div( class=( opts.before || opts.after ? 'input-group' : null ) ) if opts.before @@ -29,8 +29,8 @@ mixin input( type, label, name, opts ) for text, value in opts.options - i++ div( class=type ) - label( for=name + "_" + i ) - input( name=name, type=type, value=value, required=opts.required, checked=( opts.value === true || value == opts.value ), id=name + "_" + i ) + label( for=opts.id + "_" + i ) + input( name=name, type=type, value=value, required=opts.required, checked=( opts.value === true || value == opts.value ), id=opts.id + "_" + i ) | #{ text } - break if opts.after From a92f3db9dd933d33c47146fcc246a2b6e23607de Mon Sep 17 00:00:00 2001 From: Will Franklin Date: Fri, 23 Oct 2020 10:53:25 +0100 Subject: [PATCH 2/8] Add post-gift welcome email --- apps/members/views/emails.pug | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/members/views/emails.pug b/apps/members/views/emails.pug index efa56a868..cf5e8cfe4 100644 --- a/apps/members/views/emails.pug +++ b/apps/members/views/emails.pug @@ -20,5 +20,6 @@ block contents select(name='email' id='email' required).form-control option(value='' selected disabled) Select email option(value='welcome') Welcome email + option(value='welcome-post-gift') Post gift welcome email +form_button('Send email', 'primary', {left: 3}) From d57c305d0d5e4a47e6bada23dd59fcfdeaca650d Mon Sep 17 00:00:00 2001 From: Will Franklin Date: Fri, 23 Oct 2020 10:54:12 +0100 Subject: [PATCH 3/8] Move handleUpdateSubscription to utils for reuse --- apps/profile/apps/direct-debit/app.js | 13 +------------ apps/profile/apps/direct-debit/utils.js | 18 +++++++++++++++--- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/apps/profile/apps/direct-debit/app.js b/apps/profile/apps/direct-debit/app.js index 5f7929be4..99c27cf1d 100644 --- a/apps/profile/apps/direct-debit/app.js +++ b/apps/profile/apps/direct-debit/app.js @@ -13,7 +13,7 @@ const config = require( __config ); const { createJoinFlow, completeJoinFlow } = require( __apps + '/join/utils' ); const { cancelSubscriptionSchema, completeFlowSchema, updateSubscriptionSchema } = require('./schemas.json'); -const { calcSubscriptionMonthsLeft, canChangeSubscription, getBankAccount, processUpdateSubscription } = require('./utils'); +const { calcSubscriptionMonthsLeft, canChangeSubscription, getBankAccount, handleUpdateSubscription } = require('./utils'); const app = express(); var app_config = {}; @@ -57,17 +57,6 @@ app.get( '/', wrapAsync( async function ( req, res ) { } ); } ) ); -async function handleUpdateSubscription(req, user, form) { - const wasGift = user.contributionPeriod === 'gift'; - await processUpdateSubscription(user, form); - if (wasGift) { - await mandrill.sendToMember('welcome-post-gift', user); - req.flash( 'success', 'contribution-gift-updated' ); - } else { - req.flash( 'success', 'contribution-updated' ); - } -} - app.post( '/', [ hasSchema(updateSubscriptionSchema).orFlash ], wrapAsync( async ( req, res ) => { diff --git a/apps/profile/apps/direct-debit/utils.js b/apps/profile/apps/direct-debit/utils.js index 711a27d52..f0e0112dd 100644 --- a/apps/profile/apps/direct-debit/utils.js +++ b/apps/profile/apps/direct-debit/utils.js @@ -1,9 +1,9 @@ const moment = require('moment'); -const gocardless = require( __js + '/gocardless' ); const { Payments } = require( __js + '/database' ); - +const gocardless = require( __js + '/gocardless' ); const log = require( __js + '/logging' ).log; +const mandrill = require( __js + '/mandrill' ); const { getChargeableAmount } = require( __js + '/utils' ); const { createSubscription, startMembership } = require( __apps + '/join/utils' ); @@ -152,9 +152,21 @@ async function processUpdateSubscription(user, {amount, period, prorate, payFee} } } +async function handleUpdateSubscription(req, user, form) { + const wasGift = user.contributionPeriod === 'gift'; + await processUpdateSubscription(user, form); + if (wasGift) { + await mandrill.sendToMember('welcome-post-gift', user); + req.flash( 'success', 'contribution-gift-updated' ); + } else { + req.flash( 'success', 'contribution-updated' ); + } +} + module.exports = { calcSubscriptionMonthsLeft, canChangeSubscription, getBankAccount, - processUpdateSubscription + processUpdateSubscription, + handleUpdateSubscription }; From 99e040c9c212556abc854258c33b71815d42a304 Mon Sep 17 00:00:00 2001 From: Will Franklin Date: Fri, 23 Oct 2020 10:54:44 +0100 Subject: [PATCH 4/8] Don't need to recheck canTakePayment --- apps/profile/apps/direct-debit/app.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/profile/apps/direct-debit/app.js b/apps/profile/apps/direct-debit/app.js index 99c27cf1d..bda499f85 100644 --- a/apps/profile/apps/direct-debit/app.js +++ b/apps/profile/apps/direct-debit/app.js @@ -71,7 +71,7 @@ app.post( '/', [ updateForm } } ); - if ( useMandate && user.canTakePayment ) { + if ( useMandate ) { await handleUpdateSubscription(req, user, updateForm); res.redirect( app.parent.mountpath + app.mountpath ); } else { From 972a7e5dacb64ba4fd5a05f0d1c89b8cd55132f7 Mon Sep 17 00:00:00 2001 From: Will Franklin Date: Fri, 23 Oct 2020 10:54:58 +0100 Subject: [PATCH 5/8] Clearer condition --- apps/profile/apps/direct-debit/utils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/profile/apps/direct-debit/utils.js b/apps/profile/apps/direct-debit/utils.js index f0e0112dd..191c315ac 100644 --- a/apps/profile/apps/direct-debit/utils.js +++ b/apps/profile/apps/direct-debit/utils.js @@ -48,10 +48,10 @@ async function getBankAccount(user) { return await gocardless.customerBankAccounts.get(mandate.links.customer_bank_account); } catch (err) { // 404s can happen on dev as we don't use real mandate IDs - if (!config.dev || !err.response || err.response.status !== 404) { - throw err; + if (config.dev && err.response && err.response.status === 404) { + return null; } - return null; + throw err; } } else { return null; From cc3a21f488c702f6298e1142cc30cddc1dafb04e Mon Sep 17 00:00:00 2001 From: Will Franklin Date: Fri, 23 Oct 2020 11:15:45 +0100 Subject: [PATCH 6/8] Add ability for admin to update subscriptions --- apps/members/app.js | 22 ++++++---- apps/members/views/gocardless.pug | 69 ++++++++++++++++++------------- 2 files changed, 55 insertions(+), 36 deletions(-) diff --git a/apps/members/app.js b/apps/members/app.js index 522b58968..d5e69769c 100644 --- a/apps/members/app.js +++ b/apps/members/app.js @@ -18,7 +18,8 @@ const { hasModel, hasSchema } = require( __js + '/middleware' ); const Options = require( __js + '/options' )(); const { cleanEmailAddress, wrapAsync } = require( __js + '/utils' ); -const { isValidCustomer, createMember, customerToMember, startMembership } = require( __apps + '/join/utils' ); +const { isValidCustomer, createMember, customerToMember } = require( __apps + '/join/utils' ); +const { calcSubscriptionMonthsLeft, canChangeSubscription, handleUpdateSubscription } = require( __apps + '/profile/apps/direct-debit/utils' ); const { syncMemberDetails } = require( __apps + '/profile/apps/account/utils' ); const exportTypes = require( __apps + '/tools/apps/exports/exports'); @@ -367,18 +368,23 @@ memberAdminRouter.post( '/exports', wrapAsync( async ( req, res ) => { res.redirect( req.baseUrl + '/exports' ); } ) ); -memberAdminRouter.get( '/gocardless', ( req, res ) => { - res.render( 'gocardless', { member: req.model } ); -} ); +memberAdminRouter.get( '/gocardless', wrapAsync( async ( req, res ) => { + res.render( 'gocardless', { + member: req.model, + canChange: await canChangeSubscription( req.model, req.model.canTakePayment ), + monthsLeft: calcSubscriptionMonthsLeft( req.model ) + } ); +} ) ); memberAdminRouter.post( '/gocardless', wrapAsync( async ( req, res ) => { const member = req.model; switch ( req.body.action ) { - case 'create-subscription': - await startMembership(member, { + case 'update-subscription': + await handleUpdateSubscription(req, member, { amount: Number(req.body.amount), period: req.body.period, + prorate: req.body.prorate === 'true', payFee: req.body.payFee === 'true' }); break; @@ -390,12 +396,12 @@ memberAdminRouter.post( '/gocardless', wrapAsync( async ( req, res ) => { 'gocardless.subscription_id': req.body.subscription_id, 'gocardless.amount': Number(req.body.amount), 'gocardless.period': req.body.period, - 'gocardless.paying_fee': req.body.paying_fee + 'gocardless.paying_fee': req.body.payFee === 'true' } }); + req.flash( 'success', 'gocardless-updated' ); break; } - req.flash( 'success', 'gocardless-updated' ); res.redirect( req.baseUrl + '/gocardless' ); } ) ); diff --git a/apps/members/views/gocardless.pug b/apps/members/views/gocardless.pug index 8d025627e..4652ed531 100644 --- a/apps/members/views/gocardless.pug +++ b/apps/members/views/gocardless.pug @@ -4,6 +4,25 @@ block prepend title - title = 'GoCardless' - page = 'gocardless' +mixin commonSubscriptionFields(idPrefix) + +input( 'number', 'Monthly amount', 'amount', { + left: 3, right: 4, before: '£', + value: member.contributionMonthlyAmount, + id: idPrefix + 'Amount' + }) + +input( 'radio', 'Period', 'period', { + left: 3, right: 4, + options: {'monthly': 'Monthly', 'annually': 'Annually'}, + value: member.contributionPeriod, + id: idPrefix + 'Period' + }) + +input( 'checkbox', 'Paying fee', 'payFee', { + left: 3, right: 4, + options: {'true': 'Yes'}, + value: member.gocardless.paying_fee, + id: idPrefix + 'PayFee' + }) + block contents .row .col-md-3 @@ -11,28 +30,32 @@ block contents .col-md-9 +page_header( title ) - h4 Create subscription - if member.hasActiveSubscription - p User has a subscription - else if !member.canTakePayment - p User does not have an active payment method + h4= member.hasActiveSubscription ? 'Update subscription' : 'Create subscription' + if !member.canTakePayment + .alert.alert-warning User does not have an active payment method + else if !canChange + .alert.alert-warning Can't change subscription at the moment, probably due to pending payments else form( method="post").form-horizontal +csrf - input( type='hidden' name='action' value='create-subscription' ) - +input( 'number', 'Monthly amount', 'amount', { left: 3, right: 4, before: '£' }) - +input( 'radio', 'Period', 'period', { - left: 3, right: 4, - options: {'monthly': 'Monthly', 'annually': 'Annually'} - }) - +input( 'checkbox', 'Paying fee', 'payFee', { - left: 3, right: 4, - options: {'true': 'Yes'} - }) - +form_button( 'Create', 'primary', { left: 3 } ) + input( type='hidden' name='action' value='update-subscription' ) + +commonSubscriptionFields('update') + + if monthsLeft > 0 + .form-group + label(for='prorate').col-md-3.control-label Prorate + .col-md-9 + .checkbox + label + input(type='checkbox' value='true')#prorate + | Yes + | + small (#{monthsLeft} months left on current subscription) + + +form_button( member.hasActiveSubscription ? 'Update' : 'Create', 'success', { left: 3 } ) h4 Manual update - .alert.alert-warning. + .alert.alert-danger. If done incorrectly this could result in payments being taken but not recorded form( method="post" ).form-horizontal @@ -41,15 +64,5 @@ block contents +input( 'text', 'Customer ID', 'customer_id', { value: member.gocardless.customer_id, left: 3, right: 4 } ) +input( 'text', 'Mandate ID', 'mandate_id', { value: member.gocardless.mandate_id, left: 3, right: 4 } ) +input( 'text', 'Subscription ID', 'subscription_id', { value: member.gocardless.subscription_id, left: 3, right: 4 } ) - +input( 'number', 'Monthly amount', 'amount', { value: member.gocardless.amount, left: 3, right: 4, before: '£' } ) - +input( 'radio', 'Period', 'period', { - value: member.gocardless.period, - left: 3, right: 4, - options: {'monthly': 'Monthly', 'annually': 'Annually'} - }) - +input( 'checkbox', 'Paying fee', 'paying_fee', { - value: member.gocardless.paying_fee, - left: 3, right: 4, - options: {'true': 'Yes'} - } ) + +commonSubscriptionFields('force') +form_button( 'Update', 'danger', { left: 3 } ) From a15d4b862ddbc7393785bf4e8f0da7d0f4fa2c77 Mon Sep 17 00:00:00 2001 From: Will Franklin Date: Fri, 23 Oct 2020 11:27:39 +0100 Subject: [PATCH 7/8] Make period readonly for update contribution --- apps/members/views/gocardless.pug | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/apps/members/views/gocardless.pug b/apps/members/views/gocardless.pug index 4652ed531..fbf14ecf2 100644 --- a/apps/members/views/gocardless.pug +++ b/apps/members/views/gocardless.pug @@ -4,18 +4,23 @@ block prepend title - title = 'GoCardless' - page = 'gocardless' -mixin commonSubscriptionFields(idPrefix) +mixin commonSubscriptionFields(idPrefix, periodReadOnly) +input( 'number', 'Monthly amount', 'amount', { left: 3, right: 4, before: '£', value: member.contributionMonthlyAmount, id: idPrefix + 'Amount' }) - +input( 'radio', 'Period', 'period', { - left: 3, right: 4, - options: {'monthly': 'Monthly', 'annually': 'Annually'}, - value: member.contributionPeriod, - id: idPrefix + 'Period' - }) + if periodReadOnly + .form-group + label.col-md-3.control-label Period + .col-md-4.form-control-static= member.contributionPeriod + else + +input( 'radio', 'Period', 'period', { + left: 3, right: 4, + options: {'monthly': 'Monthly', 'annually': 'Annually'}, + value: member.contributionPeriod, + id: idPrefix + 'Period' + }) +input( 'checkbox', 'Paying fee', 'payFee', { left: 3, right: 4, options: {'true': 'Yes'}, @@ -39,7 +44,7 @@ block contents form( method="post").form-horizontal +csrf input( type='hidden' name='action' value='update-subscription' ) - +commonSubscriptionFields('update') + +commonSubscriptionFields('update', member.hasActiveSubscription) if monthsLeft > 0 .form-group @@ -64,5 +69,5 @@ block contents +input( 'text', 'Customer ID', 'customer_id', { value: member.gocardless.customer_id, left: 3, right: 4 } ) +input( 'text', 'Mandate ID', 'mandate_id', { value: member.gocardless.mandate_id, left: 3, right: 4 } ) +input( 'text', 'Subscription ID', 'subscription_id', { value: member.gocardless.subscription_id, left: 3, right: 4 } ) - +commonSubscriptionFields('force') + +commonSubscriptionFields('force', false) +form_button( 'Update', 'danger', { left: 3 } ) From bfea94c9466b5bc9a79a2409a1e11daadb5c4fff Mon Sep 17 00:00:00 2001 From: Will Franklin Date: Fri, 23 Oct 2020 11:28:22 +0100 Subject: [PATCH 8/8] Update some wording --- apps/members/views/gocardless.pug | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/members/views/gocardless.pug b/apps/members/views/gocardless.pug index fbf14ecf2..13444474f 100644 --- a/apps/members/views/gocardless.pug +++ b/apps/members/views/gocardless.pug @@ -35,11 +35,11 @@ block contents .col-md-9 +page_header( title ) - h4= member.hasActiveSubscription ? 'Update subscription' : 'Create subscription' + h4= member.hasActiveSubscription ? 'Update contribution' : 'Start contribution' if !member.canTakePayment .alert.alert-warning User does not have an active payment method else if !canChange - .alert.alert-warning Can't change subscription at the moment, probably due to pending payments + .alert.alert-warning Can't change contribution at the moment, probably due to pending payments else form( method="post").form-horizontal +csrf @@ -55,9 +55,9 @@ block contents input(type='checkbox' value='true')#prorate | Yes | - small (#{monthsLeft} months left on current subscription) + small (#{monthsLeft} months left until next payment) - +form_button( member.hasActiveSubscription ? 'Update' : 'Create', 'success', { left: 3 } ) + +form_button( member.hasActiveSubscription ? 'Update' : 'Start', 'success', { left: 3 } ) h4 Manual update .alert.alert-danger.