0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-03-18 02:21:47 -05:00

🐛 Fixed confusing error state when publishing if member count is over hosting plan limit (#15473)

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

- adds limit check for members to the `PublishOptions` class when it's constructed to set a `publishDisabledError` property if the limit check fails
  - if `publishOptions.publishDisabledCheck` is present, all publish options in the publish flow are disabled, the underlying error message is shown, and the continue button is removed to prevent filling in everything only to find at the end of the process that publishing fails
- added handling for a `HostLimitError` error from the API when confirming publishing so the proper underlying message is displayed instead of the confusing "Host limit error, cannot edit post" error
  - this is a backup measure for any instances where you're under the max members limit when starting the publish flow but are over the limit when you reach the end of the publish flow
This commit is contained in:
Kevin Ansfield 2022-09-26 17:37:35 +01:00 committed by GitHub
parent 86022b136b
commit e3db911108
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 140 additions and 61 deletions

View file

@ -67,6 +67,7 @@
@class="gh-btn gh-btn-large"
@idleClass="gh-btn-pulse"
@runningClass="gh-btn-green gh-btn-icon"
@failureClass="gh-btn-red gh-btn-icon"
data-test-button="confirm-publish"
/>
<button type="button" class="gh-btn gh-btn-link gh-btn-large gh-publish-cta-secondary" {{on "click" @cancel}} data-test-button="back-to-options">Back to settings</button>

View file

@ -101,15 +101,19 @@ export default class PublishFlowOptions extends Component {
let errorMessage = '';
const payloadError = e?.payload?.errors?.[0];
if (isServerUnreachableError(e)) {
errorMessage = 'Unable to connect, please check your internet connection and try again.';
} else if (payloadError?.type === 'HostLimitError') {
errorMessage = htmlSafe(payloadError.context.replace(/please upgrade/i, '<a href="#/pro">$&</a>'));
} else if (e && isString(e)) {
errorMessage = e;
} else if (e && isArray(e)) {
// This is here because validation errors are returned as an array
// TODO: remove this once validations are fixed
errorMessage = e[0];
} else if (e?.payload?.errors?.[0].message) {
} else if (payloadError?.message) {
errorMessage = e.payload.errors[0].message;
} else {
errorMessage = 'Unknown Error';

View file

@ -2,13 +2,23 @@
<div class="green">Ready, set, publish.</div>
<div>Share it with the world.</div>
</div>
<div class="gh-publish-settings">
<div class="gh-publish-setting" data-test-setting="publish-type">
{{#if @publishOptions.emailUnavailable}}
<div class="gh-publish-setting-title" data-test-setting-title>
{{#if (or @publishOptions.emailUnavailable @publishOptions.publishDisabledError)}}
<div class="gh-publish-setting-title {{if @publishOptions.publishDisabledError "disabled"}}" data-test-setting-title>
{{svg-jar "send-email"}}
<div class="gh-publish-setting-trigger">Publish on site</div>
{{#if @publishOptions.publishDisabledError}}
<span>{{svg-jar "arrow-down" class="icon-expand"}}</span>
{{/if}}
</div>
{{#if @publishOptions.publishDisabledError}}
<p class="gh-box gh-content-box" data-test-publish-type-error="publish-disabled">
{{@publishOptions.publishDisabledError}}
</p>
{{/if}}
{{else}}
<button type="button" class="gh-publish-setting-title" {{on "click" (fn this.toggleSection "publishType")}} data-test-setting-title>
{{svg-jar "send-email"}}
@ -31,7 +41,21 @@
{{#unless @publishOptions.emailUnavailable}}
<div class="gh-publish-setting" data-test-setting="email-recipients">
{{#if (not-eq @publishOptions.publishType "publish")}}
{{#if (or (eq @publishOptions.publishType "publish") @publishOptions.publishDisabledError)}}
<button
type="button"
class="gh-publish-setting-title disabled"
data-test-setting-title
>
{{svg-jar "member"}}
<div class="gh-publish-setting-trigger">
Not sent as newsletter
</div>
<span>
{{svg-jar "arrow-down" class="icon-expand"}}
</span>
</button>
{{else}}
<button
type="button"
class="gh-publish-setting-title"
@ -67,20 +91,6 @@
{{svg-jar "arrow-down" class="icon-expand"}}
</span>
</button>
{{else}}
<button
type="button"
class="gh-publish-setting-title disabled"
data-test-setting-title
>
{{svg-jar "member"}}
<div class="gh-publish-setting-trigger">
Not sent as newsletter
</div>
<span>
{{svg-jar "arrow-down" class="icon-expand"}}
</span>
</button>
{{/if}}
{{#liquid-if (eq this.openSection "emailRecipients")}}
<div class="gh-publish-setting-form">
@ -110,7 +120,7 @@
{{/if}}
<div class="gh-publish-setting last" data-test-setting="publish-at">
<button type="button" class="gh-publish-setting-title" {{on "click" (fn this.toggleSection "publishAt")}} data-test-setting-title>
<button type="button" class="gh-publish-setting-title {{if @publishOptions.publishDisabledError "disabled"}}" {{on "click" (if @publishOptions.publishDisabledError (optional) (fn this.toggleSection "publishAt"))}} data-test-setting-title>
{{svg-jar "clock"}}
<div class="gh-publish-setting-trigger">
<span>
@ -136,8 +146,10 @@
</div>
<div class="gh-publish-cta">
<button type="button" class="gh-btn gh-btn-black gh-btn-large" {{on "click" @confirm}} data-test-button="continue">
<span>Continue, final review &rarr;</span>
</button>
</div>
{{#unless @publishOptions.publishDisabledError}}
<div class="gh-publish-cta">
<button type="button" class="gh-btn gh-btn-black gh-btn-large" {{on "click" @confirm}} data-test-button="continue">
<span>Continue, final review &rarr;</span>
</button>
</div>
{{/unless}}

View file

@ -32,30 +32,20 @@ export default class MembersStatsService extends Service {
let daysChanged = this._lastFetchedDays !== this.days;
let staleData = this._lastFetched && (new Date() - this._lastFetched) > ONE_MINUTE;
// return an already in-progress promise unless params have changed
if (this._fetchTask.isRunning && !this._forceRefresh && !daysChanged) {
// return existing stats unless data is > 1 min old or days param has changed
if (this.stats && !this._forceRefresh && !daysChanged && !staleData && this._fetchTask.last) {
return this._fetchTask.last;
}
// return existing stats unless data is > 1 min old
if (this.stats && !this._forceRefresh && !daysChanged && !staleData) {
return Promise.resolve(this.stats);
}
return this._fetchTask.perform();
}
fetchCounts() {
let staleData = this._lastFetchedCounts && (new Date() - this._lastFetchedCounts) > ONE_MINUTE;
// return an already in-progress promise unless params have changed
if (this._fetchCountsTask.isRunning) {
return this._fetchCountsTask.last;
}
// return existing stats unless data is > 1 min old
if (this.countStats && !this._forceRefresh && !staleData) {
return Promise.resolve(this.countStats);
if (this.countStats && !this._forceRefresh && !staleData && this._fetchCountsTask.last) {
return this._fetchCountsTask.last;
}
return this._fetchCountsTask.perform();
@ -64,14 +54,9 @@ export default class MembersStatsService extends Service {
fetchMemberCount() {
let staleData = this._lastFetchedMemberCounts && (new Date() - this._lastFetchedMemberCounts) > ONE_MINUTE;
// return an already in-progress promise unless params have changed
if (this._fetchMemberCountsTask.isRunning) {
return this._fetchMemberCountsTask.last;
}
// return existing stats unless data is > 1 min old
if (this.totalMemberCount && !this._forceRefresh && !staleData) {
return Promise.resolve(this.totalMemberCount);
if (this.totalMemberCount && !this._forceRefresh && !staleData && this._fetchMemberCountsTask.last) {
return this._fetchMemberCountsTask.last;
}
return this._fetchMemberCountsTask.perform();
@ -80,14 +65,9 @@ export default class MembersStatsService extends Service {
fetchNewsletterStats() {
let staleData = this._lastFetchedNewsletterStats && (new Date() - this._lastFetchedNewsletterStats) > ONE_MINUTE;
// return an already in-progress promise unless params have changed
if (this._fetchNewsletterStatsTask.isRunning) {
return this._fetchNewsletterStatsTask.last;
}
// return existing stats unless data is > 1 min old
if (this.newsletterStats && !this._forceRefresh && !staleData) {
return Promise.resolve(this.newsletterStats);
if (this.newsletterStats && !this._forceRefresh && !staleData && this._fetchNewsletterStatsTask.last) {
return this._fetchNewsletterStatsTask.last;
}
return this._fetchNewsletterStatsTask.perform();
@ -160,14 +140,9 @@ export default class MembersStatsService extends Service {
fetchMRR() {
let staleData = this._lastFetchedMRR && (new Date() - this._lastFetchedMRR) > ONE_MINUTE;
// return an already in-progress promise unless params have changed
if (this._fetchMRRTask.isRunning) {
return this._fetchMRRTask.last;
}
// return existing stats unless data is > 1 min old
if (this.mrrStats && !this._forceRefresh && !staleData) {
return Promise.resolve(this.mrrStats);
if (this.mrrStats && !this._forceRefresh && !staleData && this._fetchMRRTask) {
return this._fetchMRRTask.last;
}
return this._fetchMRRTask.perform();

View file

@ -1,5 +1,6 @@
import moment from 'moment';
import {action, get} from '@ember/object';
import {htmlSafe} from '@ember/template';
import {task} from 'ember-concurrency';
import {tracked} from '@glimmer/tracking';
@ -14,6 +15,7 @@ export default class PublishOptions {
post = null;
user = null;
@tracked publishDisabledError = null;
@tracked totalMemberCount = 0;
get isLoading() {
@ -270,8 +272,10 @@ export default class PublishOptions {
this.totalMemberCount = 1;
}
// email limits
// limits
promises.push(this._checkSendingLimit());
promises.push(this._checkPublishingLimit());
// newsletters
promises.push(this.store.query('newsletter', {status: 'active', limit: 'all', include: 'count.members'}));
@ -377,4 +381,15 @@ export default class PublishOptions {
this.emailDisabledError = e.message;
}
}
async _checkPublishingLimit() {
try {
if (this.limit.limiter?.isLimited('members')) {
await this.limit.limiter.errorIfIsOverLimit('members');
}
} catch (e) {
const linkedMessage = htmlSafe(e.message.replace(/please upgrade/i, '<a href="#/pro">$&</a>'));
this.publishDisabledError = linkedMessage;
}
}
}

View file

@ -444,7 +444,79 @@ describe('Acceptance: Publish flow', function () {
expect(find('[data-test-publish-type="send"]')).to.have.attribute('disabled');
});
it('handles member limits');
it('handles over-member limit before publish', async function () {
// set members limit
const config = this.server.db.configs.find(1);
config.hostSettings = {
limits: {
members: {
max: 9,
error: 'Your plan supports up to {{max}} members. Please upgrade to reenable publishing.'
}
}
};
this.server.db.configs.update(1, config);
// go over limit (7 created by default in beforeEach)
this.server.createList('member', 3);
// simulate /members/stats/count/ endpoint that's used to get total member count
// TODO: can the default endpoint mock handle this?
this.server.get('/members/stats/count', function () {
return {
total: 10,
resource: 'members',
data: []
};
});
// try to publish post
const post = this.server.create('post', {status: 'draft'});
await visit(`/editor/post/${post.id}`);
await click('[data-test-button="publish-flow"]');
expect(find('[data-test-publish-type-error]'), 'publish disabled error').to.exist;
expect(find('[data-test-publish-type-error="publish-disabled"]'), 'publish disabled error')
.to.have.trimmed.text('Your plan supports up to 9 members. Please upgrade to reenable publishing.');
expect(find('[data-test-button="continue"]'), 'continue button').to.not.exist;
});
it('handles over-member limit when confirming', async function () {
const post = this.server.create('post', {status: 'draft'});
await visit(`/editor/post/${post.id}`);
await click('[data-test-button="publish-flow"]');
await click('[data-test-button="continue"]');
this.server.put('/posts/:id/', function () {
return {
errors: [
{
message: 'Host Limit error, cannot edit post.',
context: 'Your plan supports up to 1,000 members. Please upgrade to reenable publishing.',
type: 'HostLimitError',
details: {
name: 'members',
limit: 1000,
total: 37406
},
property: null,
help: 'https://ghost.org/help/',
code: null,
id: '212d9110-3db6-11ed-9651-e9a82ad49a7a',
ghostErrorCode: null
}
]
};
});
await click('[data-test-button="confirm-publish"]');
expect(find('[data-test-confirm-error]'), 'confirm error').to.exist;
expect(find('[data-test-confirm-error]'), 'confirm error')
.to.have.trimmed.text('Your plan supports up to 1,000 members. Please upgrade to reenable publishing.');
});
it('handles server error when confirming');
it('handles email sending error');
});