From 72590083f3a81dc9b5bac4fa51ed1a32467dc255 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 8 Apr 2021 12:06:27 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Added=20ability=20to=20bulk=20delet?= =?UTF-8?q?e=20members=20by=20label=20or=20status=20(#1883)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs https://github.com/TryGhost/Team/issues/585 requires https://github.com/TryGhost/Ghost/pull/12082 When a label or status filter is selected on the members screen show a "Delete selected" option in the actions dropdown. Bulk deleted members will _not_ have any subscription data modified in Stripe, if a member should be deleted and have their subscription cancelled it's necessary to do that on a per-member basis. - updated bulk delete handling to match API - added link to bulk delete confirmation modal in members actions dropdown (only shown when label, status, or search is used) - updated testing framework for members - added label factory for easier test setup - updated `GET /members` and `DEL /members` endpoints to work with label filters - updated test selectors for easier reference in tests --- .../app/components/gh-members-filter.hbs | 8 ++- .../app/components/gh-members-list-item.hbs | 2 +- .../app/components/modal-delete-members.hbs | 17 +++--- .../app/components/modal-delete-members.js | 2 +- ghost/admin/app/controllers/members.js | 16 ++++-- ghost/admin/app/templates/members.hbs | 23 ++++++-- ghost/admin/mirage/config/members.js | 56 ++++++++++++++++--- ghost/admin/mirage/config/posts.js | 34 +---------- ghost/admin/mirage/factories/label.js | 15 +++++ ghost/admin/mirage/factories/member.js | 18 +++++- ghost/admin/mirage/models/label.js | 5 ++ ghost/admin/mirage/models/member.js | 3 +- ghost/admin/mirage/serializers/label.js | 18 ++++++ ghost/admin/mirage/utils.js | 51 +++++++++++++++++ ghost/admin/tests/acceptance/members-test.js | 45 +++++++++++++++ 15 files changed, 245 insertions(+), 68 deletions(-) create mode 100644 ghost/admin/mirage/factories/label.js create mode 100644 ghost/admin/mirage/models/label.js create mode 100644 ghost/admin/mirage/serializers/label.js diff --git a/ghost/admin/app/components/gh-members-filter.hbs b/ghost/admin/app/components/gh-members-filter.hbs index be306abd5a..9a70562c9e 100644 --- a/ghost/admin/app/components/gh-members-filter.hbs +++ b/ghost/admin/app/components/gh-members-filter.hbs @@ -2,8 +2,10 @@ + @classNames="gh-contentfilter-menu-trigger" + @title="Member Labels" + data-test-button="labels-filter" + > {{@selectedLabel.name}} {{svg-jar "arrow-down-small"}} @@ -19,7 +21,7 @@ {{#each @availableLabels as |label|}}
  • - {{label.name}} + {{label.name}} {{#if label.slug}} {{svg-jar "pen"}} {{/if}} diff --git a/ghost/admin/app/components/gh-members-list-item.hbs b/ghost/admin/app/components/gh-members-list-item.hbs index 9d4cc52fc5..d38435c363 100644 --- a/ghost/admin/app/components/gh-members-list-item.hbs +++ b/ghost/admin/app/components/gh-members-list-item.hbs @@ -1,4 +1,4 @@ -
  • +
  • {{#if @member.is_loading}}
    diff --git a/ghost/admin/app/components/modal-delete-members.hbs b/ghost/admin/app/components/modal-delete-members.hbs index d3a65647fd..b59d8662ec 100644 --- a/ghost/admin/app/components/modal-delete-members.hbs +++ b/ghost/admin/app/components/modal-delete-members.hbs @@ -1,5 +1,5 @@
    -

    Are you sure you want to delete these members?

    +

    Delete selected members?

    {{svg-jar "close"}} @@ -8,7 +8,7 @@

    You're about to delete {{gh-pluralize this.model.memberCount "member"}}. - This is permanent! We warned you, k? + This is permanent! All Ghost data will be deleted, this will have no effect on subscriptions in Stripe.

    {{else}} @@ -26,21 +26,18 @@
    {{svg-jar "check-circle" class="w4 h4 stroke-green mr2"}}

    - {{gh-pluralize this.response.deleted.count "member"}} + {{gh-pluralize this.response.stats.successful "member"}} deleted

    - {{#if this.response.invalid.count}} + {{#if this.response.stats.unsuccessful}}
    {{svg-jar "warning" class="w4 h4 fill-red mr2 nudge-top--3"}}

    - {{gh-pluralize this.response.invalid.count "member"}} - skipped + {{gh-pluralize this.response.stats.unsuccessful "member"}} + failed to delete

    - {{#each this.response.invalid.errors as |error|}} -

    {{error.message}} {{error.count}}

    - {{/each}}
    {{/if}} @@ -55,7 +52,7 @@ - + {{svg-jar "settings"}} - +
  • Import members @@ -58,6 +64,13 @@ {{/if}}
    • + {{#if (and this.members.length this.isFiltered)}} +
  • + +
    • + {{/if}}     New member diff --git a/ghost/admin/mirage/config/members.js b/ghost/admin/mirage/config/members.js index 1fc7806ddb..8fdf0c5544 100644 --- a/ghost/admin/mirage/config/members.js +++ b/ghost/admin/mirage/config/members.js @@ -1,7 +1,8 @@ import faker from 'faker'; import moment from 'moment'; import {Response} from 'ember-cli-mirage'; -import {paginatedResponse} from '../utils'; +import {extractFilterParam, paginateModelCollection} from '../utils'; +import {isEmpty} from '@ember/utils'; export function mockMembersStats(server) { server.get('/members/stats/count', function (db, {queryParams}) { @@ -66,25 +67,64 @@ export default function mockMembers(server) { return members.create(Object.assign({}, attrs, {id: 99})); }); - server.get('/members/', paginatedResponse('members')); + server.get('/members/', function ({members}, {queryParams}) { + let {filter, page, limit} = queryParams; + + page = +page || 1; + limit = +limit || 15; + + let labelFilter = extractFilterParam('label', filter); + + let collection = members.all().filter((member) => { + let matchesLabel = true; + + if (!isEmpty(labelFilter)) { + matchesLabel = false; + + labelFilter.forEach((slug) => { + if (member.labels.models.find(l => l.slug === slug)) { + matchesLabel = true; + } + }); + } + + return matchesLabel; + }); + + return paginateModelCollection('members', collection, page, limit); + }); server.del('/members/', function ({members}, {queryParams}) { - if (queryParams.all !== 'true') { + if (!queryParams.filter && !queryParams.search && queryParams.all !== 'true') { return new Response(422, {}, {errors: [{ type: 'IncorrectUsageError', message: 'DELETE /members/ must be used with a filter, search, or all=true query parameter' }]}); } - let count = members.all().length; - members.all().destroy(); + let membersToDelete = members.all(); + + if (queryParams.filter) { + let labelFilter = extractFilterParam('label', queryParams.filter); + + membersToDelete = membersToDelete.filter((member) => { + let matches = false; + labelFilter.forEach((slug) => { + if (member.labels.models.find(l => l.slug === slug)) { + matches = true; + } + }); + return matches; + }); + } + + let count = membersToDelete.length; + membersToDelete.destroy(); return { meta: { stats: { - deleted: { - count - } + successful: count } } }; diff --git a/ghost/admin/mirage/config/posts.js b/ghost/admin/mirage/config/posts.js index dc6f827287..d3ad6dcc6e 100644 --- a/ghost/admin/mirage/config/posts.js +++ b/ghost/admin/mirage/config/posts.js @@ -1,40 +1,8 @@ import moment from 'moment'; import {Response} from 'ember-cli-mirage'; import {dasherize} from '@ember/string'; -import {isArray} from '@ember/array'; +import {extractFilterParam, paginateModelCollection} from '../utils'; import {isBlank, isEmpty} from '@ember/utils'; -import {paginateModelCollection} from '../utils'; - -function normalizeBooleanParams(arr) { - if (!isArray(arr)) { - return arr; - } - - return arr.map((i) => { - if (i === 'true') { - return true; - } else if (i === 'false') { - return false; - } else { - return i; - } - }); -} - -// TODO: use GQL to parse filter string? -function extractFilterParam(param, filter) { - let filterRegex = new RegExp(`${param}:(.*?)(?:\\+|$)`); - let match; - - let [, result] = filter.match(filterRegex) || []; - if (result.startsWith('[')) { - match = result.replace(/^\[|\]$/g, '').split(','); - } else if (result) { - match = [result]; - } - - return normalizeBooleanParams(match); -} // NOTE: mirage requires Model objects when saving relationships, however the // `attrs` on POST/PUT requests will contain POJOs for authors and tags so we diff --git a/ghost/admin/mirage/factories/label.js b/ghost/admin/mirage/factories/label.js new file mode 100644 index 0000000000..5dbf7840a2 --- /dev/null +++ b/ghost/admin/mirage/factories/label.js @@ -0,0 +1,15 @@ +import moment from 'moment'; +import {Factory} from 'ember-cli-mirage'; + +export default Factory.extend({ + createdAt() { return moment().toISOString(); }, + createdBy: 1, + name(i) { return `Label ${i}`; }, + slug(i) { return `label-${i}`; }, + updatedAt() { return moment().toISOString(); }, + updatedBy: 1, + count() { + // this gets updated automatically by the label serializer + return {members: 0}; + } +}); diff --git a/ghost/admin/mirage/factories/member.js b/ghost/admin/mirage/factories/member.js index b2ebbe2eaa..e0e37202c6 100644 --- a/ghost/admin/mirage/factories/member.js +++ b/ghost/admin/mirage/factories/member.js @@ -1,6 +1,6 @@ import faker from 'faker'; import moment from 'moment'; -import {Factory} from 'ember-cli-mirage'; +import {Factory, trait} from 'ember-cli-mirage'; let randomDate = function randomDate(start = moment().subtract(30, 'days').toDate(), end = new Date()) { return new Date(start.getTime() + Math.random() * (end.getTime() - start.getTime())); @@ -9,5 +9,19 @@ let randomDate = function randomDate(start = moment().subtract(30, 'days').toDat export default Factory.extend({ name() { return `${faker.name.firstName()} ${faker.name.lastName()}`; }, email: faker.internet.email, - createdAt() { return randomDate(); } + status: 'free', + subscribed: true, + createdAt() { return randomDate(); }, + + free: trait({ + status: 'free' + }), + + paid: trait({ + status: 'paid' + }), + + comped: trait({ + status: 'comped' + }) }); diff --git a/ghost/admin/mirage/models/label.js b/ghost/admin/mirage/models/label.js new file mode 100644 index 0000000000..1466b9ea45 --- /dev/null +++ b/ghost/admin/mirage/models/label.js @@ -0,0 +1,5 @@ +import {Model, hasMany} from 'ember-cli-mirage'; + +export default Model.extend({ + members: hasMany() +}); diff --git a/ghost/admin/mirage/models/member.js b/ghost/admin/mirage/models/member.js index 6bfe517e93..f97e3b4248 100644 --- a/ghost/admin/mirage/models/member.js +++ b/ghost/admin/mirage/models/member.js @@ -1,4 +1,5 @@ -import {Model} from 'ember-cli-mirage'; +import {Model, hasMany} from 'ember-cli-mirage'; export default Model.extend({ + labels: hasMany() }); diff --git a/ghost/admin/mirage/serializers/label.js b/ghost/admin/mirage/serializers/label.js new file mode 100644 index 0000000000..dc3b9d7ab7 --- /dev/null +++ b/ghost/admin/mirage/serializers/label.js @@ -0,0 +1,18 @@ +import BaseSerializer from './application'; + +export default BaseSerializer.extend({ + // make the label.count.members value dynamic + serialize(labelModelOrCollection, request) { + let updateMemberCount = (label) => { + label.update('count', {members: label.memberIds.length}); + }; + + if (this.isModel(labelModelOrCollection)) { + updateMemberCount(labelModelOrCollection); + } else { + labelModelOrCollection.models.forEach(updateMemberCount); + } + + return BaseSerializer.prototype.serialize.call(this, labelModelOrCollection, request); + } +}); diff --git a/ghost/admin/mirage/utils.js b/ghost/admin/mirage/utils.js index 57e82d3904..c615336bb7 100644 --- a/ghost/admin/mirage/utils.js +++ b/ghost/admin/mirage/utils.js @@ -1,5 +1,6 @@ /* eslint-disable max-statements-per-line */ import {Response} from 'ember-cli-mirage'; +import {isArray} from '@ember/array'; export function paginatedResponse(modelName) { return function (schema, request) { @@ -71,3 +72,53 @@ export function versionMismatchResponse() { }] }); } + +function normalizeBooleanParams(arr) { + if (!isArray(arr)) { + return arr; + } + + return arr.map((i) => { + if (i === 'true') { + return true; + } else if (i === 'false') { + return false; + } else { + return i; + } + }); +} + +function normalizeStringParams(arr) { + if (!isArray(arr)) { + return arr; + } + + return arr.map((i) => { + if (!i.replace) { + return i; + } + + return i.replace(/^['"]|['"]$/g, ''); + }); +} + +// TODO: use GQL to parse filter string? +export function extractFilterParam(param, filter) { + let filterRegex = new RegExp(`${param}:(.*?)(?:\\+|$)`); + let match; + + let [, result] = filter.match(filterRegex) || []; + + if (!result) { + return; + } + + if (result.startsWith('[')) { + match = result.replace(/^\[|\]$/g, '').split(','); + } else { + match = [result]; + } + + return normalizeBooleanParams(normalizeStringParams(match)); +} diff --git a/ghost/admin/tests/acceptance/members-test.js b/ghost/admin/tests/acceptance/members-test.js index 6cc048dfbe..da6b764624 100644 --- a/ghost/admin/tests/acceptance/members-test.js +++ b/ghost/admin/tests/acceptance/members-test.js @@ -145,5 +145,50 @@ describe('Acceptance: Members', function () { expect(find('[data-test-input="member-email"]').value, 'email has been preserved') .to.equal('example@domain.com'); }); + + it('can bulk delete members', async function () { + // members to be kept + this.server.createList('member', 6); + + // imported members to be deleted + const label = this.server.create('label'); + this.server.createList('member', 5, {labels: [label]}); + + await visit('/members'); + + expect(findAll('[data-test-member]').length).to.equal(11); + + await click('[data-test-button="members-actions"]'); + + expect(find('[data-test-button="delete-selected"]')).to.not.exist; + + // a filter is needed for the delete-selected button to show + await click('[data-test-button="labels-filter"]'); + await click(`[data-test-label-filter="${label.name}"]`); + + expect(findAll('[data-test-member]').length).to.equal(5); + expect(currentURL()).to.equal('/members?label=label-0'); + + await click('[data-test-button="members-actions"]'); + + expect(find('[data-test-button="delete-selected"]')).to.exist; + + await click('[data-test-button="delete-selected"]'); + + expect(find('[data-test-modal="delete-members"]')).to.exist; + expect(find('[data-test-text="delete-count"]')).to.have.text('5 members'); + + await click('[data-test-button="confirm"]'); + + expect(find('[data-test-text="deleted-count"]')).to.have.text('5 members'); + expect(find('[data-test-button="confirm"]')).to.not.exist; + // members filter is reset + // TODO: fix query params reset for empty strings + expect(currentURL()).to.equal('/members?search='); + + await click('[data-test-button="close-modal"]'); + + expect(find('[data-test-modal="delete-members"]')).to.not.exist; + }); }); });