🐛 Fixed missing validation of offer amounts in the admin panel (#16022)

closes https://github.com/TryGhost/Team/issues/2380

- improved offer validation for `amount` field to cover all type/amount cases
- added validate-on-blur to the amount field to match our standard validation behaviour
- added re-validation of the amount field when the type is changed and the amount gets reset
- removed the internal parsing of a decimal trial days entry to an integer so the field value matches what is set internally and we let the user know that partial trial days are not supported

Non-user-facing refactors:
- renamed `_saveOfferProperty` to `_updateOfferProperty` to better reflect what it does
- fixed missing indentation for conditional blocks in the offer template
This commit is contained in:
Kevin Ansfield 2023-01-03 09:23:11 +00:00 committed by GitHub
parent 235446b034
commit 581f0b34b4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 200 additions and 163 deletions

View File

@ -1101,3 +1101,5 @@ remove|ember-template-lint|no-passed-in-event-handlers|21|12|21|12|4069dec45ac2a
remove|ember-template-lint|no-passed-in-event-handlers|22|12|22|12|e53f64794fdd0fe8c8b027d1831942d7c78c503b|1662681600000|1673053200000|1678237200000|app/components/gh-benefit-item.hbs remove|ember-template-lint|no-passed-in-event-handlers|22|12|22|12|e53f64794fdd0fe8c8b027d1831942d7c78c503b|1662681600000|1673053200000|1678237200000|app/components/gh-benefit-item.hbs
add|ember-template-lint|no-passed-in-event-handlers|23|16|23|16|d47dcf0c8eea7584639b48d5a99b7db07436c259|1670976000000|1681340400000|1686524400000|app/components/gh-benefit-item.hbs add|ember-template-lint|no-passed-in-event-handlers|23|16|23|16|d47dcf0c8eea7584639b48d5a99b7db07436c259|1670976000000|1681340400000|1686524400000|app/components/gh-benefit-item.hbs
add|ember-template-lint|no-passed-in-event-handlers|24|16|24|16|14a806b3f993ec777b1a5ff7e00887e5840bbb77|1670976000000|1681340400000|1686524400000|app/components/gh-benefit-item.hbs add|ember-template-lint|no-passed-in-event-handlers|24|16|24|16|14a806b3f993ec777b1a5ff7e00887e5840bbb77|1670976000000|1681340400000|1686524400000|app/components/gh-benefit-item.hbs
remove|ember-template-lint|no-passed-in-event-handlers|150|56|150|56|37bf29e93ffc35c71cdddd0ab98edeb60097e826|1662681600000|1673053200000|1678237200000|app/templates/offer.hbs
remove|ember-template-lint|no-passed-in-event-handlers|161|56|161|56|37bf29e93ffc35c71cdddd0ab98edeb60097e826|1662681600000|1673053200000|1678237200000|app/templates/offer.hbs

View File

@ -263,19 +263,34 @@ export default class OffersController extends Controller {
@action @action
setProperty(propKey, value) { setProperty(propKey, value) {
this._saveOfferProperty(propKey, value); this._updateOfferProperty(propKey, value);
}
@action
validateProperty(property) {
this.offer.validate({property});
}
@action
clearPropertyValidations(property) {
this.offer.errors.remove(property);
} }
@action @action
setDiscountType(discountType) { setDiscountType(discountType) {
if (!this.isDiscountSectionDisabled) { if (!this.isDiscountSectionDisabled) {
const amount = this.offer.amount || 0; const amount = this.offer.amount || 0;
this._saveOfferProperty('type', discountType);
this._updateOfferProperty('type', discountType);
if (this.offer.type === 'fixed' && this.offer.amount !== '') { if (this.offer.type === 'fixed' && this.offer.amount !== '') {
this.offer.amount = amount * 100; this.offer.amount = amount * 100;
} else if (this.offer.amount !== '') { } else if (this.offer.amount !== '') {
this.offer.amount = amount / 100; this.offer.amount = amount / 100;
} }
this.validateProperty('amount');
this.updatePortalPreview({forceRefresh: false}); this.updatePortalPreview({forceRefresh: false});
} }
} }
@ -283,53 +298,52 @@ export default class OffersController extends Controller {
@action @action
setDiscountAmount(e) { setDiscountAmount(e) {
let amount = e.target.value; let amount = e.target.value;
if (this.offer.type === 'fixed' && amount !== '') { if (this.offer.type === 'fixed' && amount !== '') {
amount = parseFloat(amount) * 100; amount = parseFloat(amount) * 100;
} }
this._saveOfferProperty('amount', amount);
this._updateOfferProperty('amount', amount);
} }
@action @action
setTrialDuration(e) { setTrialDuration(e) {
let amount = e.target.value; let amount = e.target.value;
if (amount !== '') { this._updateOfferProperty('amount', amount);
amount = parseInt(amount);
}
this._saveOfferProperty('amount', amount);
} }
@action @action
setOfferName(e) { setOfferName(e) {
this._saveOfferProperty('name', e.target.value); this._updateOfferProperty('name', e.target.value);
if (!this.isDisplayTitleEdited && this.offer.isNew) { if (!this.isDisplayTitleEdited && this.offer.isNew) {
this._saveOfferProperty('displayTitle', e.target.value); this._updateOfferProperty('displayTitle', e.target.value);
} }
if (!this.isOfferCodeEdited && this.offer.isNew) { if (!this.isOfferCodeEdited && this.offer.isNew) {
this._saveOfferProperty('code', slugify(e.target.value)); this._updateOfferProperty('code', slugify(e.target.value));
} }
} }
@action @action
setPortalTitle(e) { setPortalTitle(e) {
this.isDisplayTitleEdited = true; this.isDisplayTitleEdited = true;
this._saveOfferProperty('displayTitle', e.target.value); this._updateOfferProperty('displayTitle', e.target.value);
} }
@action @action
setPortalDescription(e) { setPortalDescription(e) {
this._saveOfferProperty('displayDescription', e.target.value); this._updateOfferProperty('displayDescription', e.target.value);
} }
@action @action
setOfferCode(e) { setOfferCode(e) {
this.isOfferCodeEdited = true; this.isOfferCodeEdited = true;
this._saveOfferProperty('code', e.target.value); this._updateOfferProperty('code', e.target.value);
} }
@action @action
setDurationInMonths(e) { setDurationInMonths(e) {
this._saveOfferProperty('durationInMonths', e.target.value); this._updateOfferProperty('durationInMonths', e.target.value);
} }
@action @action
@ -403,7 +417,7 @@ export default class OffersController extends Controller {
} }
]; ];
if (this.offer.duration === 'repeating') { if (this.offer.duration === 'repeating') {
this._saveOfferProperty('duration', 'once'); this._updateOfferProperty('duration', 'once');
} }
} }
} }
@ -412,13 +426,17 @@ export default class OffersController extends Controller {
@action @action
changeType(type) { changeType(type) {
if (type === 'trial') { if (type === 'trial') {
this._saveOfferProperty('type', 'trial'); this._updateOfferProperty('type', 'trial');
this._saveOfferProperty('amount', 7); this._updateOfferProperty('amount', 7);
this._saveOfferProperty('duration', 'trial'); this._updateOfferProperty('duration', 'trial');
this.validateProperty('amount');
} else { } else {
this._saveOfferProperty('type', 'percent'); this._updateOfferProperty('type', 'percent');
this._saveOfferProperty('amount', 0); this._updateOfferProperty('amount', 0);
this._saveOfferProperty('duration', 'once'); this._updateOfferProperty('duration', 'once');
this.clearPropertyValidations('amount');
} }
} }
@ -449,12 +467,12 @@ export default class OffersController extends Controller {
@action @action
updateDuration(duration) { updateDuration(duration) {
this._saveOfferProperty('duration', duration); this._updateOfferProperty('duration', duration);
} }
// Private ----------------------------------------------------------------- // Private -----------------------------------------------------------------
_saveOfferProperty(propKey, newValue) { _updateOfferProperty(propKey, newValue) {
let currentValue = this.offer[propKey]; let currentValue = this.offer[propKey];
// avoid modifying empty values and triggering inadvertant unsaved changes modals // avoid modifying empty values and triggering inadvertant unsaved changes modals

View File

@ -13,7 +13,7 @@
{{else}} {{else}}
{{this.offer.name}} {{this.offer.name}}
{{#if (eq this.offer.status "archived")}} {{#if (eq this.offer.status "archived")}}
<span class="gh-badge gh-badge-title">Archived</span> <span class="gh-badge gh-badge-title">Archived</span>
{{/if}} {{/if}}
{{/if}} {{/if}}
</h2> </h2>
@ -74,157 +74,145 @@
</div> </div>
</div> </div>
</GhFormGroup> </GhFormGroup>
{{#if this.isTrialOffer}} {{#if this.isTrialOffer}}
<div class="gh-offer-tier-and-trial"> <div class="gh-offer-tier-and-trial">
<GhFormGroup @errors={{this.errors}} @property="product-cadence"> <GhFormGroup @errors={{this.errors}} @property="product-cadence">
<label for="product-cadence" class="fw6">Tier cadence</label> <label for="product-cadence" class="fw6">Tier cadence</label>
<span class="gh-select gh-select-product-cadence"> <span class="gh-select gh-select-product-cadence">
<OneWaySelect <OneWaySelect
@value={{this.cadence}} @value={{this.cadence}}
@options={{this.cadences}} @options={{this.cadences}}
@optionValuePath="name" @optionValuePath="name"
@optionLabelPath="label" @optionLabelPath="label"
@optionTargetPath="name" @optionTargetPath="name"
@title="{{this.offer.tier.name}} - {{if (eq this.offer.cadence "month") "Monthly" "Yearly" }}" @title="{{this.offer.tier.name}} - {{if (eq this.offer.cadence "month") "Monthly" "Yearly" }}"
@disabled={{this.isDiscountSectionDisabled}}
@update={{this.updateCadence}}
/>
{{svg-jar "arrow-down-small"}}
</span>
<GhErrorMessage @errors={{this.errors}} @property="product-cadence" />
</GhFormGroup>
<div class="gh-offer-trial-duration">
<GhFormGroup @errors={{this.offer.errors}} @property="trialDuration">
<label for="trial-duration" class="fw6">Trial duration</label>
<div class="trial-duration">
<GhTextInput
@name="trial-duration"
@type="number"
@placeholder=""
@disabled={{this.isDiscountSectionDisabled}} @disabled={{this.isDiscountSectionDisabled}}
@value={{readonly this.offer.amount}} @update={{this.updateCadence}}
{{on "input" this.setTrialDuration}}
@id="trial-duration"
@class="gh-input"
/> />
</div> {{svg-jar "arrow-down-small"}}
<span class="error">
<GhErrorMessage @errors={{this.offer.errors}} @property="amount" />
</span> </span>
<GhErrorMessage @errors={{this.errors}} @property="product-cadence" />
</GhFormGroup> </GhFormGroup>
</div> <div class="gh-offer-trial-duration">
</div> <GhFormGroup @errors={{this.offer.errors}} @property="trialDuration">
<p class="gh-offer-trial-info">Members will be subscribed at full price once the trial ends. <a class="green" href="https://ghost.org/help/free-trials">Learn more</a></p> <label for="trial-duration" class="fw6">Trial duration</label>
{{else}} <div class="trial-duration">
<div class="gh-offer-tier-and-trial"> <input
<GhFormGroup @errors={{this.errors}} @property="product-cadence"> type="number"
<label for="product-cadence" class="fw6">Tier cadence</label> id="trial-duration"
<span class="gh-select gh-select-product-cadence"> class="gh-input"
<OneWaySelect name="trial-duration"
@value={{this.cadence}} value={{this.offer.amount}}
@options={{this.cadences}} disabled={{this.isDiscountSectionDisabled}}
@optionValuePath="name" {{on "input" this.setTrialDuration}}
@optionLabelPath="label" {{on "blur" (fn this.validateProperty "amount")}}
@optionTargetPath="name" />
@title="{{this.offer.tier.name}} - {{if (eq this.offer.cadence "month") "Monthly" "Yearly" }}"
@disabled={{this.isDiscountSectionDisabled}}
@update={{this.updateCadence}}
/>
{{svg-jar "arrow-down-small"}}
</span>
<GhErrorMessage @errors={{this.errors}} @property="product-cadence" />
</GhFormGroup>
<div class="gh-offer-discount">
<label for="amount" class="fw6 mb1">Amount off</label>
<div class="flex items-start">
<GhFormGroup @errors={{this.offer.errors}} @property="amount" @hasValidated={{this.offer.hasValidated}}>
<div class="gh-offer-value percentage">
{{#if (eq this.offer.type 'fixed')}}
<GhTextInput
@name="amount"
@type="number"
@placeholder=""
@disabled={{this.isDiscountSectionDisabled}}
@value={{readonly (gh-price-amount this.offer.amount)}}
@input={{this.setDiscountAmount}}
@id="amount"
@class="gh-input"
/>
{{else}}
<GhTextInput
@name="amount"
@type="number"
@placeholder=""
@disabled={{this.isDiscountSectionDisabled}}
@value={{readonly this.offer.amount}}
@input={{this.setDiscountAmount}}
@id="amount"
@class="gh-input"
/>
{{/if}}
</div> </div>
<span class="error"> <span class="error">
<GhErrorMessage @errors={{this.offer.errors}} @property="amount" /> <GhErrorMessage @errors={{this.offer.errors}} @property="amount" />
</span> </span>
</GhFormGroup> </GhFormGroup>
<div class="gh-offer-type"> </div>
<GhFormGroup @errors={{this.offer.errors}} @property="type" @hasValidated={{this.offer.hasValidated}} class="no-margin"> </div>
<span class="gh-select"> <p class="gh-offer-trial-info">Members will be subscribed at full price once the trial ends. <a class="green" href="https://ghost.org/help/free-trials">Learn more</a></p>
<OneWaySelect {{else}}
@value={{this.offer.type}} <div class="gh-offer-tier-and-trial">
@options={{this.offertypes}} <GhFormGroup @errors={{this.errors}} @property="product-cadence">
@optionValuePath="offertype" <label for="product-cadence" class="fw6">Tier cadence</label>
@disabled={{this.isDiscountSectionDisabled}} <span class="gh-select gh-select-product-cadence">
@optionLabelPath="label" <OneWaySelect
@optionTargetPath="offertype" @value={{this.cadence}}
@update = {{this.setDiscountType}} @options={{this.cadences}}
@optionValuePath="name"
@optionLabelPath="label"
@optionTargetPath="name"
@title="{{this.offer.tier.name}} - {{if (eq this.offer.cadence "month") "Monthly" "Yearly" }}"
@disabled={{this.isDiscountSectionDisabled}}
@update={{this.updateCadence}}
/>
{{svg-jar "arrow-down-small"}}
</span>
<GhErrorMessage @errors={{this.errors}} @property="product-cadence" />
</GhFormGroup>
<div class="gh-offer-discount">
<label for="amount" class="fw6 mb1">Amount off</label>
<div class="flex items-start">
<GhFormGroup @errors={{this.offer.errors}} @property="amount" @hasValidated={{this.offer.hasValidated}}>
<div class="gh-offer-value percentage">
<input
type="number"
id="amount"
class="gh-input"
name="amount"
value={{if (eq this.offer.type 'fixed') (gh-price-amount this.offer.amount) this.offer.amount}}
disabled={{this.isDiscountSectionDisabled}}
{{on "input" this.setDiscountAmount}}
{{on "blur" (fn this.validateProperty "amount")}}
/> />
{{svg-jar "arrow-down-small"}} </div>
<span class="error">
<GhErrorMessage @errors={{this.offer.errors}} @property="amount" />
</span> </span>
<GhErrorMessage @errors={{this.offer.errors}} @property="type" />
</GhFormGroup> </GhFormGroup>
<div class="gh-offer-type">
<GhFormGroup @errors={{this.offer.errors}} @property="type" @hasValidated={{this.offer.hasValidated}} class="no-margin">
<span class="gh-select">
<OneWaySelect
@value={{this.offer.type}}
@options={{this.offertypes}}
@optionValuePath="offertype"
@disabled={{this.isDiscountSectionDisabled}}
@optionLabelPath="label"
@optionTargetPath="offertype"
@update={{this.setDiscountType}}
/>
{{svg-jar "arrow-down-small"}}
</span>
<GhErrorMessage @errors={{this.offer.errors}} @property="type" />
</GhFormGroup>
</div>
</div> </div>
</div> </div>
</div> </div>
</div> <div class="gh-offer-duration">
<div class="gh-offer-duration"> <GhFormGroup @errors={{this.errors}} @property="duration">
<GhFormGroup @errors={{this.errors}} @property="duration"> <label for="product-cadence" class="fw6">Duration</label>
<label for="product-cadence" class="fw6">Duration</label> <span class="gh-select">
<span class="gh-select"> <OneWaySelect
<OneWaySelect @data-test-select="offer-duration"
@data-test-select="offer-duration" @value={{this.offer.duration}}
@value={{this.offer.duration}} @options={{this.durations}}
@options={{this.durations}} @optionValuePath="duration"
@optionValuePath="duration" @disabled={{this.isDiscountSectionDisabled}}
@disabled={{this.isDiscountSectionDisabled}} @optionLabelPath="label"
@optionLabelPath="label" @optionTargetPath="duration"
@optionTargetPath="duration" @update={{this.updateDuration}}
@update = {{this.updateDuration}} />
/> {{svg-jar "arrow-down-small"}}
{{svg-jar "arrow-down-small"}} </span>
</span> <span class="error">
<span class="error"> <GhErrorMessage @errors={{this.errors}} @property="duration" />
<GhErrorMessage @errors={{this.errors}} @property="duration" /> </span>
</span> </GhFormGroup>
</GhFormGroup> {{#if (eq this.offer.duration "repeating")}}
{{#if (eq this.offer.duration "repeating")}} <GhFormGroup @errors={{this.offer.errors}} @property="durationInMonths">
<GhFormGroup @errors={{this.offer.errors}} @property="durationInMonths"> <label for="duration-months" class="fw6">Number of months</label>
<label for="duration-months" class="fw6">Number of months</label> <div class="duration-months">
<div class="duration-months"> <GhTextInput
<GhTextInput @name="duration-months"
@name="duration-months" @value={{readonly this.offer.durationInMonths}}
@value={{readonly this.offer.durationInMonths}} @input={{this.setDurationInMonths}}
@input={{this.setDurationInMonths}} @disabled={{this.isDiscountSectionDisabled}}
@disabled={{this.isDiscountSectionDisabled}} @id="duration-months"
@id="duration-months" @class="gh-input" />
@class="gh-input" /> </div>
</div> <span class="error">
<span class="error"> <GhErrorMessage @errors={{this.offer.errors}} @property="durationInMonths" />
<GhErrorMessage @errors={{this.offer.errors}} @property="durationInMonths" /> </span>
</span> </GhFormGroup>
</GhFormGroup> {{/if}}
{{/if}} </div>
</div>
{{/if}} {{/if}}
</div> </div>

View File

@ -22,10 +22,39 @@ export default BaseValidator.create({
} else { } else {
model.errors.add('amount', 'Please enter the amount.'); model.errors.add('amount', 'Please enter the amount.');
} }
this.invalidate();
} else if (model.type === 'trial' && model.amount < 0) { return this.invalidate();
model.errors.add('amount', 'Free trial must be at least 1 day.'); }
this.invalidate();
if (model.type === 'trial') {
if (model.amount < 1) {
model.errors.add('amount', 'Free trial must be at least 1 day.');
return this.invalidate();
}
if (!model.amount.toString().match(/^\d+$/)) {
model.errors.add('amount', 'Trial days must be a whole number.');
return this.invalidate();
}
}
if (model.type === 'percent') {
if (model.amount < 0 || model.amount > 100) {
model.errors.add('amount', 'Amount must be between 0 and 100%.');
return this.invalidate();
}
if (!model.amount.toString().match(/^\d+$/)) {
model.errors.add('amount', 'Amount must be a whole number.');
return this.invalidate();
}
}
if (model.type === 'fixed') {
if (model.amount < 0) {
model.errors.add('amount', 'Amount must be greater than 0.');
return this.invalidate();
}
} }
}, },