From 581f0b34b4b06f081fa3946a13fb7ceffdf65804 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Tue, 3 Jan 2023 09:23:11 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20missing=20validation=20o?= =?UTF-8?q?f=20offer=20amounts=20in=20the=20admin=20panel=20(#16022)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ghost/admin/.lint-todo | 2 + ghost/admin/app/controllers/offer.js | 64 ++++--- ghost/admin/app/templates/offer.hbs | 260 +++++++++++++-------------- ghost/admin/app/validators/offer.js | 37 +++- 4 files changed, 200 insertions(+), 163 deletions(-) diff --git a/ghost/admin/.lint-todo b/ghost/admin/.lint-todo index ff93d47cba..a2cf637ce0 100644 --- a/ghost/admin/.lint-todo +++ b/ghost/admin/.lint-todo @@ -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 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 +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 diff --git a/ghost/admin/app/controllers/offer.js b/ghost/admin/app/controllers/offer.js index a9a02fdf0e..3fe7e73c22 100644 --- a/ghost/admin/app/controllers/offer.js +++ b/ghost/admin/app/controllers/offer.js @@ -263,19 +263,34 @@ export default class OffersController extends Controller { @action 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 setDiscountType(discountType) { if (!this.isDiscountSectionDisabled) { const amount = this.offer.amount || 0; - this._saveOfferProperty('type', discountType); + + this._updateOfferProperty('type', discountType); + if (this.offer.type === 'fixed' && this.offer.amount !== '') { this.offer.amount = amount * 100; } else if (this.offer.amount !== '') { this.offer.amount = amount / 100; } + + this.validateProperty('amount'); + this.updatePortalPreview({forceRefresh: false}); } } @@ -283,53 +298,52 @@ export default class OffersController extends Controller { @action setDiscountAmount(e) { let amount = e.target.value; + if (this.offer.type === 'fixed' && amount !== '') { amount = parseFloat(amount) * 100; } - this._saveOfferProperty('amount', amount); + + this._updateOfferProperty('amount', amount); } @action setTrialDuration(e) { let amount = e.target.value; - if (amount !== '') { - amount = parseInt(amount); - } - this._saveOfferProperty('amount', amount); + this._updateOfferProperty('amount', amount); } @action setOfferName(e) { - this._saveOfferProperty('name', e.target.value); + this._updateOfferProperty('name', e.target.value); if (!this.isDisplayTitleEdited && this.offer.isNew) { - this._saveOfferProperty('displayTitle', e.target.value); + this._updateOfferProperty('displayTitle', e.target.value); } if (!this.isOfferCodeEdited && this.offer.isNew) { - this._saveOfferProperty('code', slugify(e.target.value)); + this._updateOfferProperty('code', slugify(e.target.value)); } } @action setPortalTitle(e) { this.isDisplayTitleEdited = true; - this._saveOfferProperty('displayTitle', e.target.value); + this._updateOfferProperty('displayTitle', e.target.value); } @action setPortalDescription(e) { - this._saveOfferProperty('displayDescription', e.target.value); + this._updateOfferProperty('displayDescription', e.target.value); } @action setOfferCode(e) { this.isOfferCodeEdited = true; - this._saveOfferProperty('code', e.target.value); + this._updateOfferProperty('code', e.target.value); } @action setDurationInMonths(e) { - this._saveOfferProperty('durationInMonths', e.target.value); + this._updateOfferProperty('durationInMonths', e.target.value); } @action @@ -403,7 +417,7 @@ export default class OffersController extends Controller { } ]; if (this.offer.duration === 'repeating') { - this._saveOfferProperty('duration', 'once'); + this._updateOfferProperty('duration', 'once'); } } } @@ -412,13 +426,17 @@ export default class OffersController extends Controller { @action changeType(type) { if (type === 'trial') { - this._saveOfferProperty('type', 'trial'); - this._saveOfferProperty('amount', 7); - this._saveOfferProperty('duration', 'trial'); + this._updateOfferProperty('type', 'trial'); + this._updateOfferProperty('amount', 7); + this._updateOfferProperty('duration', 'trial'); + + this.validateProperty('amount'); } else { - this._saveOfferProperty('type', 'percent'); - this._saveOfferProperty('amount', 0); - this._saveOfferProperty('duration', 'once'); + this._updateOfferProperty('type', 'percent'); + this._updateOfferProperty('amount', 0); + this._updateOfferProperty('duration', 'once'); + + this.clearPropertyValidations('amount'); } } @@ -449,12 +467,12 @@ export default class OffersController extends Controller { @action updateDuration(duration) { - this._saveOfferProperty('duration', duration); + this._updateOfferProperty('duration', duration); } // Private ----------------------------------------------------------------- - _saveOfferProperty(propKey, newValue) { + _updateOfferProperty(propKey, newValue) { let currentValue = this.offer[propKey]; // avoid modifying empty values and triggering inadvertant unsaved changes modals diff --git a/ghost/admin/app/templates/offer.hbs b/ghost/admin/app/templates/offer.hbs index 471b6976d2..e43b44358b 100644 --- a/ghost/admin/app/templates/offer.hbs +++ b/ghost/admin/app/templates/offer.hbs @@ -13,7 +13,7 @@ {{else}} {{this.offer.name}} {{#if (eq this.offer.status "archived")}} - Archived + Archived {{/if}} {{/if}} @@ -74,157 +74,145 @@ + {{#if this.isTrialOffer}} -
- - - - - {{svg-jar "arrow-down-small"}} - - - -
- - -
- + + + + -
- - + {{svg-jar "arrow-down-small"}} + -
-
-

Members will be subscribed at full price once the trial ends. Learn more

- {{else}} -
- - - - - {{svg-jar "arrow-down-small"}} - - - -
- -
- -
- {{#if (eq this.offer.type 'fixed')}} - - {{else}} - - {{/if}} +
+ + +
+
-
- - - +
+

Members will be subscribed at full price once the trial ends. Learn more

+ {{else}} +
+ + + + + {{svg-jar "arrow-down-small"}} + + + +
+ +
+ +
+ - {{svg-jar "arrow-down-small"}} +
+ + - +
+ + + + {{svg-jar "arrow-down-small"}} + + + +
-
-
- - - - - {{svg-jar "arrow-down-small"}} - - - - - - {{#if (eq this.offer.duration "repeating")}} - - -
- -
- - - - - {{/if}} -
+
+ + + + + {{svg-jar "arrow-down-small"}} + + + + + + {{#if (eq this.offer.duration "repeating")}} + + +
+ +
+ + + + + {{/if}} +
{{/if}}
diff --git a/ghost/admin/app/validators/offer.js b/ghost/admin/app/validators/offer.js index 9b3a501d24..02fe513e7d 100644 --- a/ghost/admin/app/validators/offer.js +++ b/ghost/admin/app/validators/offer.js @@ -22,10 +22,39 @@ export default BaseValidator.create({ } else { model.errors.add('amount', 'Please enter the amount.'); } - this.invalidate(); - } else if (model.type === 'trial' && model.amount < 0) { - model.errors.add('amount', 'Free trial must be at least 1 day.'); - this.invalidate(); + + return 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(); + } } },