diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 924d60c50a4..bea4891273a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -369,6 +369,8 @@ jobs: - name: Run migrations working-directory: ghost/core run: yarn knex-migrator init + env: + NODE_OPTIONS: "--import=tsx" - name: Setup Playwright uses: ./.github/actions/setup-playwright diff --git a/apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx b/apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx index 1f6823bf39b..daf660977f0 100644 --- a/apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx +++ b/apps/admin-x-settings/src/components/settings/advanced/labs/private-features.tsx @@ -71,10 +71,6 @@ const features: Feature[] = [{ title: 'Sniper Links', description: 'Enable mail app links on signup/signin', flag: 'sniperlinks' -}, { - title: 'Paid Breakdown Charts', - description: 'Show paid member change and subscription cadence breakdown charts on the Growth page', - flag: 'paidBreakdownCharts' }]; const AlphaFeatures: React.FC = () => { diff --git a/apps/stats/src/views/Stats/Growth/components/growth-kpis.tsx b/apps/stats/src/views/Stats/Growth/components/growth-kpis.tsx index 825b252304e..6bcb9d739f8 100644 --- a/apps/stats/src/views/Stats/Growth/components/growth-kpis.tsx +++ b/apps/stats/src/views/Stats/Growth/components/growth-kpis.tsx @@ -1,12 +1,10 @@ import React, {useEffect, useMemo, useState} from 'react'; -import moment from 'moment'; -import {BarChartLoadingIndicator, ChartConfig, ChartContainer, ChartTooltip, ChartTooltipContent, DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger, GhAreaChart, GhAreaChartDataItem, KpiDropdownButton, KpiTabTrigger, KpiTabValue, Recharts, Separator, Tabs, TabsContent, TabsList, TabsTrigger, centsToDollars, formatDisplayDateWithRange, formatNumber, getRangeDates} from '@tryghost/shade'; +import {BarChartLoadingIndicator, DropdownMenu, DropdownMenuContent, DropdownMenuItem, DropdownMenuTrigger, GhAreaChart, GhAreaChartDataItem, KpiDropdownButton, KpiTabTrigger, KpiTabValue, Separator, Tabs, TabsList, centsToDollars, formatDisplayDateWithRange, formatNumber} from '@tryghost/shade'; import {DiffDirection} from '@hooks/use-growth-stats'; import {STATS_RANGES} from '@src/utils/constants'; -import {determineAggregationStrategy, sanitizeChartData} from '@src/utils/chart-helpers'; +import {sanitizeChartData} from '@src/utils/chart-helpers'; import {useAppContext} from '@src/app'; import {useGlobalData} from '@src/providers/global-data-provider'; -import {useLabsFlag} from '@src/hooks/use-labs-flag'; import {useNavigate, useSearchParams} from '@tryghost/admin-x-framework'; type ChartDataItem = { @@ -104,21 +102,18 @@ const PaidMembersTooltipContent = ({active, payload, range, color, showBreakdown const GrowthKPIs: React.FC<{ chartData: ChartDataItem[]; - subscriptionData?: {date: string; signups: number; cancellations: number}[]; totals: Totals; initialTab?: string; currencySymbol: string; isLoading: boolean; onTabChange?: (tab: KpiTab) => void; -}> = ({chartData: allChartData, subscriptionData, totals, initialTab, currencySymbol, isLoading, onTabChange}) => { +}> = ({chartData: allChartData, totals, initialTab, currencySymbol, isLoading, onTabChange}) => { const validatedInitialTab = isValidTab(initialTab) ? initialTab : 'total-members'; const [currentTab, setCurrentTab] = useState(validatedInitialTab); - const [paidChartTab, setPaidChartTab] = useState('total'); const {range} = useGlobalData(); const {appSettings} = useAppContext(); const navigate = useNavigate(); const [searchParams] = useSearchParams(); - const showPaidBreakdown = useLabsFlag('paidBreakdownCharts'); // Update current tab if initialTab changes useEffect(() => { @@ -139,66 +134,6 @@ const GrowthKPIs: React.FC<{ const {totalMembers, freeMembers, paidMembers, mrr, percentChanges, directions} = totals; - // Helper function to fill missing data points with zeros - const fillMissingDataPoints = (data: {date: string; signups: number; cancellations: number}[], dateRange: number) => { - // For "Today" (dateRange = 1), show just one data point for the current date - if (dateRange === 1) { - const today = moment().format('YYYY-MM-DD'); - const todayData = data.find(item => item.date === today); - - return [{ - date: today, - signups: todayData?.signups || 0, - cancellations: todayData?.cancellations || 0 - }]; - } - - const {startDate, endDate} = getRangeDates(dateRange); - const dateSpan = moment(endDate).diff(moment(startDate), 'days'); - const strategy = determineAggregationStrategy(dateRange, dateSpan, 'sum'); - - // Create a map of existing data by date - const dataMap = new Map(data.map(item => [item.date, item])); - - const filledData: {date: string; signups: number; cancellations: number}[] = []; - const currentDate = moment(startDate); - const endMoment = moment(endDate); - - while (currentDate.isSameOrBefore(endMoment)) { - let dateKey: string; - let increment: moment.unitOfTime.DurationConstructor; - - switch (strategy) { - case 'weekly': - dateKey = currentDate.startOf('week').format('YYYY-MM-DD'); - increment = 'week'; - break; - case 'monthly': - dateKey = currentDate.startOf('month').format('YYYY-MM-DD'); - increment = 'month'; - break; - default: - dateKey = currentDate.format('YYYY-MM-DD'); - increment = 'day'; - } - - const existingData = dataMap.get(dateKey); - if (existingData) { - filledData.push(existingData); - } else { - filledData.push({ - date: dateKey, - signups: 0, - cancellations: 0 - }); - } - - currentDate.add(1, increment); - } - - return filledData; - }; - // Create chart data based on selected tab const chartData = useMemo(() => { if (!allChartData || allChartData.length === 0) { @@ -293,105 +228,6 @@ const GrowthKPIs: React.FC<{ } }; - // Paid change chart data (only computed when flag is disabled) - const paidChangeChartData = useMemo(() => { - // Only compute when flag is disabled and we're on paid-members tab - if (showPaidBreakdown || currentTab !== 'paid-members') { - return []; - } - - // Use subscription data if available (like Ember dashboard), otherwise fall back to member data - if (subscriptionData && subscriptionData.length > 0) { - // For "Today" range, show just the change for today - if (range === 1) { - const today = moment().format('YYYY-MM-DD'); - const todayData = subscriptionData.find(item => item.date === today); - - return [{ - date: formatDisplayDateWithRange(today, range), - new: todayData?.signups || 0, - cancelled: -(todayData?.cancellations || 0) // Negative for the stacked bar chart - }]; - } - - // Apply proper aggregation to subscription data using 'sum' aggregation type FIRST - // This will properly sum signups and cancellations within each time period - const signupsData = sanitizeChartData(subscriptionData, range, 'signups', 'sum'); - const cancellationsData = sanitizeChartData(subscriptionData, range, 'cancellations', 'sum'); - - // Combine the aggregated data - const combinedData = signupsData.map(item => ({ - date: item.date, - signups: item.signups || 0, - cancellations: cancellationsData.find(c => c.date === item.date)?.cancellations || 0 - })); - - // Add any cancellation-only dates that might be missing from signups - cancellationsData.forEach((cancelItem) => { - if (!combinedData.find(item => item.date === cancelItem.date)) { - combinedData.push({ - date: cancelItem.date, - signups: 0, - cancellations: cancelItem.cancellations || 0 - }); - } - }); - - // Sort by date - combinedData.sort((a, b) => new Date(a.date).getTime() - new Date(b.date).getTime()); - - // Now fill missing data points with zeros to ensure consistent display - const filledData = fillMissingDataPoints(combinedData, range); - - return filledData.map((item) => { - return { - date: formatDisplayDateWithRange(item.date, range), - new: item.signups || 0, - cancelled: -(item.cancellations || 0) // Negative for the stacked bar chart - }; - }); - } else { - // Fall back to member count data - if (!allChartData || allChartData.length === 0) { - return []; - } - - // For "Today" range, show just today's change - if (range === 1) { - const today = moment().format('YYYY-MM-DD'); - const todayData = allChartData.find(item => item.date === today); - - return [{ - date: formatDisplayDateWithRange(today, range), - new: todayData?.paid_subscribed || 0, - cancelled: -(todayData?.paid_canceled || 0) // Negative for the stacked bar chart - }]; - } - - const sanitizedData = sanitizeChartData(allChartData, range, 'paid', 'exact'); - - return sanitizedData.map((item) => { - return { - date: formatDisplayDateWithRange(item.date, range), - new: item.paid_subscribed || 0, - cancelled: -(item.paid_canceled || 0) // Negative for the stacked bar chart - }; - }); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [showPaidBreakdown, currentTab, allChartData, subscriptionData, range]); - - const paidChangeChartConfig = { - new: { - label: 'New', - color: 'hsl(var(--chart-teal))' - }, - cancelled: { - label: 'Cancelled', - color: 'hsl(var(--chart-rose))' - } - } satisfies ChartConfig; - if (isLoading) { return (
@@ -508,186 +344,20 @@ const GrowthKPIs: React.FC<{ }
- {currentTab === 'paid-members' && !showPaidBreakdown ? ( - // Legacy behavior: Total/Change tabs with embedded bar chart - { - setPaidChartTab(value); - }} - > -
- - - Total - - - Change - - -
- - - - - - - - - - - - - - - - - - - ('')} - tickLine={false} - tickMargin={10} - /> - { - return value < 0 ? formatNumber(value * -1) : formatNumber(value); - }} - tickLine={false} - /> - { - const rawValue = Number(value); - let displayValue = '0'; - if (rawValue === 0) { - displayValue = '0'; - } else { - displayValue = rawValue < 0 ? formatNumber(rawValue * -1) : formatNumber(rawValue); - } - - // Calculate net change (new + cancelled, since cancelled is negative) - const newValue = Number(payload?.payload?.new || 0); - const cancelledValue = Number(payload?.payload?.cancelled || 0); - const netChange = newValue + cancelledValue; - const netChangeFormatted = netChange === 0 ? '0' : (netChange > 0 ? `+${formatNumber(netChange)}` : formatNumber(netChange)); - - return ( -
- {index === 0 && -
- {payload?.payload?.date} -
- } -
-
-
- - {paidChangeChartConfig[name as keyof typeof paidChangeChartConfig]?.label || name} - -
-
- {displayValue} -
-
- {index === 1 && -
- - Net change - -
- {netChangeFormatted} -
-
- } -
- ); - }} - hideLabel - />} - cursor={false} - isAnimationActive={false} - position={{y: 10}} - /> - - - - -
-
- - New -
-
- - Cancelled -
-
- - - ) : ( - // New behavior (flag enabled) or non-paid-members tabs - { - return `${currencySymbol}${formatNumber(value)}`; - } : - formatNumber} - id={currentTab} - range={range} - tooltipContent={currentTab === 'paid-members' && showPaidBreakdown ? : undefined} - /> - )} + { + return `${currencySymbol}${formatNumber(value)}`; + } : + formatNumber} + id={currentTab} + range={range} + tooltipContent={currentTab === 'paid-members' ? : undefined} + />
); diff --git a/apps/stats/src/views/Stats/Growth/growth.tsx b/apps/stats/src/views/Stats/Growth/growth.tsx index d4629c665aa..60bcf060d4f 100644 --- a/apps/stats/src/views/Stats/Growth/growth.tsx +++ b/apps/stats/src/views/Stats/Growth/growth.tsx @@ -16,7 +16,6 @@ import {getPeriodText} from '@src/utils/chart-helpers'; import {useAppContext} from '@src/app'; import {useGlobalData} from '@src/providers/global-data-provider'; import {useGrowthStats} from '@hooks/use-growth-stats'; -import {useLabsFlag} from '@hooks/use-labs-flag'; import {useNavigate, useSearchParams} from '@tryghost/admin-x-framework'; import {useTopPostsStatsWithRange} from '@hooks/use-top-posts-stats-with-range'; import type {TopPostStatItem} from '@tryghost/admin-x-framework/api/stats'; @@ -52,7 +51,6 @@ const Growth: React.FC = () => { const [selectedContentType, setSelectedContentType] = useState(CONTENT_TYPES.POSTS_AND_PAGES); const [searchParams] = useSearchParams(); const {appSettings} = useAppContext(); - const paidBreakdownChartsEnabled = useLabsFlag('paidBreakdownCharts'); // Get the initial tab from URL search parameters const initialTab = searchParams.get('tab') || 'total-members'; @@ -156,13 +154,12 @@ const Growth: React.FC = () => { currencySymbol={currencySymbol} initialTab={initialTab} isLoading={isPageLoading} - subscriptionData={subscriptionData} totals={totals} onTabChange={setCurrentKpiTab} /> - {appSettings?.paidMembersEnabled && currentKpiTab === 'paid-members' && paidBreakdownChartsEnabled && ( + {appSettings?.paidMembersEnabled && currentKpiTab === 'paid-members' && (
diff --git a/ghost/admin/app/templates/mentions.hbs b/ghost/admin/app/templates/mentions.hbs index 9c9f93f5769..1187684fb85 100644 --- a/ghost/admin/app/templates/mentions.hbs +++ b/ghost/admin/app/templates/mentions.hbs @@ -104,7 +104,7 @@ {{/if}}
{{#if this.mentionsInfinityModel}} diff --git a/ghost/admin/app/templates/pages.hbs b/ghost/admin/app/templates/pages.hbs index edabc8d1c6e..b87cee8aba6 100644 --- a/ghost/admin/app/templates/pages.hbs +++ b/ghost/admin/app/templates/pages.hbs @@ -56,19 +56,19 @@ {{#if @model.scheduledInfinityModel}} {{/if}} {{#if (and @model.draftInfinityModel (or (not @model.scheduledInfinityModel) (and @model.scheduledInfinityModel @model.scheduledInfinityModel.reachedInfinity)))}} {{/if}} {{#if (and @model.publishedAndSentInfinityModel (and (or (not @model.scheduledInfinityModel) @model.scheduledInfinityModel.reachedInfinity) (or (not @model.draftInfinityModel) @model.draftInfinityModel.reachedInfinity)))}} {{/if}} diff --git a/ghost/admin/app/templates/posts.hbs b/ghost/admin/app/templates/posts.hbs index 65a67ff8129..41beb01eb70 100644 --- a/ghost/admin/app/templates/posts.hbs +++ b/ghost/admin/app/templates/posts.hbs @@ -58,19 +58,19 @@ {{#if @model.scheduledInfinityModel}} {{/if}} {{#if (and @model.draftInfinityModel (or (not @model.scheduledInfinityModel) (and @model.scheduledInfinityModel @model.scheduledInfinityModel.reachedInfinity)))}} {{/if}} {{#if (and @model.publishedAndSentInfinityModel (and (or (not @model.scheduledInfinityModel) @model.scheduledInfinityModel.reachedInfinity) (or (not @model.draftInfinityModel) @model.draftInfinityModel.reachedInfinity)))}} {{/if}} diff --git a/ghost/admin/package.json b/ghost/admin/package.json index d41682ea60e..ed4e3af6d0a 100644 --- a/ghost/admin/package.json +++ b/ghost/admin/package.json @@ -1,6 +1,6 @@ { "name": "ghost-admin", - "version": "6.15.0", + "version": "6.16.0", "description": "Ember.js admin client for Ghost", "author": "Ghost Foundation", "homepage": "http://ghost.org", diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js index 33c2ae8b96f..4905d73ecc1 100644 --- a/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js +++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/mappers/comments.js @@ -2,8 +2,6 @@ const _ = require('lodash'); const utils = require('../../..'); const url = require('../utils/url'); const htmlToPlaintext = require('@tryghost/html-to-plaintext'); -const {MemberCommentingCodec} = require('../../../../../../services/members/commenting'); -const {serializeCommenting} = require('../utils/member-commenting'); const commentFields = [ 'id', @@ -30,7 +28,9 @@ const memberFieldsAdmin = [ 'name', 'email', 'expertise', - 'avatar_image' + 'avatar_image', + 'can_comment', + 'commenting' ]; const postFields = [ @@ -73,14 +73,6 @@ const commentMapper = (model, frame) => { if (jsonModel.member) { response.member = _.pick(jsonModel.member, isPublicRequest ? memberFields : memberFieldsAdmin); - - // For admin requests, always add computed commenting fields for consistent response shape - // MemberCommentingCodec.parse() handles null/undefined by returning enabled state - if (!isPublicRequest) { - const commenting = MemberCommentingCodec.parse(jsonModel.member.commenting); - response.member.can_comment = commenting.canComment; - response.member.commenting = serializeCommenting(commenting); - } } else { response.member = null; } diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/members.js b/ghost/core/core/server/api/endpoints/utils/serializers/output/members.js index a085e543f5d..4263b20ed95 100644 --- a/ghost/core/core/server/api/endpoints/utils/serializers/output/members.js +++ b/ghost/core/core/server/api/endpoints/utils/serializers/output/members.js @@ -5,8 +5,6 @@ const {unparse} = require('@tryghost/members-csv'); const mappers = require('./mappers'); const {Transform} = require('stream'); const papaparse = require('papaparse'); -const {serializeCommenting} = require('./utils/member-commenting'); - module.exports = { browse: createSerializer('browse', paginatedMembers), read: createSerializer('read', singleMember), @@ -182,7 +180,7 @@ function serializeMember(member, options) { attribution: serializeAttribution(json.attribution), unsubscribe_url: json.unsubscribe_url, can_comment: json.can_comment, - commenting: serializeCommenting(json.commenting) + commenting: json.commenting }; if (json.products) { diff --git a/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/member-commenting.ts b/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/member-commenting.ts deleted file mode 100644 index bf9cda7219f..00000000000 --- a/ghost/core/core/server/api/endpoints/utils/serializers/output/utils/member-commenting.ts +++ /dev/null @@ -1,23 +0,0 @@ -import type {MemberCommenting} from '../../../../../../services/members/commenting/member-commenting'; - -interface SerializedCommenting { - disabled: boolean; - disabled_reason: string | null; - disabled_until: string | null; -} - -/** - * Serializes a MemberCommenting domain instance for API responses. - * Converts camelCase domain properties to snake_case API format. - */ -export function serializeCommenting(commenting: MemberCommenting | null): SerializedCommenting | null { - if (!commenting) { - return commenting; - } - - return { - disabled: commenting.disabled, - disabled_reason: commenting.disabledReason, - disabled_until: commenting.disabledUntil?.toISOString() ?? null - }; -} diff --git a/ghost/core/core/server/models/member.js b/ghost/core/core/server/models/member.js index 7eaaa507f54..fe854bf9b73 100644 --- a/ghost/core/core/server/models/member.js +++ b/ghost/core/core/server/models/member.js @@ -2,6 +2,7 @@ const ghostBookshelf = require('./base'); const crypto = require('crypto'); const _ = require('lodash'); const config = require('../../shared/config'); +const {MemberCommentingCodec} = require('../services/members/commenting'); const Member = ghostBookshelf.Model.extend({ tableName: 'members', @@ -21,6 +22,40 @@ const Member = ghostBookshelf.Model.extend({ }; }, + /** + * Transform data coming from the database. + * Parses the `commenting` JSON string into a MemberCommenting domain object + * and computes the `can_comment` boolean. + */ + parse(attrs) { + attrs = ghostBookshelf.Model.prototype.parse.call(this, attrs); + + if (attrs.commenting !== undefined) { + const commenting = MemberCommentingCodec.parse(attrs.commenting); + attrs.commenting = commenting; + attrs.can_comment = commenting.canComment; + } + + return attrs; + }, + + /** + * Transform data going to the database. + * Converts the MemberCommenting domain object back to a JSON string + * and removes the computed `can_comment` field. + */ + format(attrs) { + // Remove computed field - it should not be persisted + delete attrs.can_comment; + + // Convert MemberCommenting domain object to JSON string for storage + if (attrs.commenting) { + attrs.commenting = MemberCommentingCodec.format(attrs.commenting); + } + + return ghostBookshelf.Model.prototype.format.call(this, attrs); + }, + filterExpansions() { return [{ key: 'label', @@ -403,6 +438,11 @@ const Member = ghostBookshelf.Model.extend({ attrs.avatar_image = gravatar.url(attrs.email, {size: 250, default: 'blank'}); } + // Serialize commenting domain object to API format + if (attrs.commenting) { + attrs.commenting = MemberCommentingCodec.toJSON(attrs.commenting); + } + return attrs; } }, { diff --git a/ghost/core/core/server/services/members/commenting/member-commenting-codec.ts b/ghost/core/core/server/services/members/commenting/member-commenting-codec.ts index 276e21dcdc0..9964f09f92a 100644 --- a/ghost/core/core/server/services/members/commenting/member-commenting-codec.ts +++ b/ghost/core/core/server/services/members/commenting/member-commenting-codec.ts @@ -10,6 +10,12 @@ interface StoredCommenting { * Codec for bidirectional serialization of MemberCommenting. * Handles conversion between domain objects and persistence format. */ +interface SerializedCommenting { + disabled: boolean; + disabled_reason: string | null; + disabled_until: string | null; +} + export const MemberCommentingCodec = { /** * Parse a raw JSON string from storage into a MemberCommenting domain object. @@ -47,5 +53,21 @@ export const MemberCommentingCodec = { disabledReason: commenting.disabledReason, disabledUntil: commenting.disabledUntil?.toISOString() ?? null }); + }, + + /** + * Serialize a MemberCommenting domain object for API responses. + * Converts camelCase domain properties to snake_case API format. + */ + toJSON(commenting: MemberCommenting | null): SerializedCommenting | null { + if (!commenting) { + return commenting; + } + + return { + disabled: commenting.disabled, + disabled_reason: commenting.disabledReason, + disabled_until: commenting.disabledUntil?.toISOString() ?? null + }; } }; diff --git a/ghost/core/core/server/services/members/members-api/controllers/router-controller.js b/ghost/core/core/server/services/members/members-api/controllers/router-controller.js index da756b69d13..25234053720 100644 --- a/ghost/core/core/server/services/members/members-api/controllers/router-controller.js +++ b/ghost/core/core/server/services/members/members-api/controllers/router-controller.js @@ -704,7 +704,7 @@ module.exports = class RouterController { // Honeypot field is filled, this is a bot. // Pretend that the email was sent successfully. - res.writeHead(201); + res.writeHead(201, {'Content-Type': 'application/json'}); return res.end('{}'); } diff --git a/ghost/core/core/server/services/members/members-api/repositories/member-repository.js b/ghost/core/core/server/services/members/members-api/repositories/member-repository.js index df90b121cbd..cbde95f13fa 100644 --- a/ghost/core/core/server/services/members/members-api/repositories/member-repository.js +++ b/ghost/core/core/server/services/members/members-api/repositories/member-repository.js @@ -10,7 +10,6 @@ const validator = require('@tryghost/validator'); const crypto = require('crypto'); const StartOutboxProcessingEvent = require('../../../outbox/events/start-outbox-processing-event'); const {MEMBER_WELCOME_EMAIL_SLUGS} = require('../../../member-welcome-emails/constants'); -const {MemberCommentingCodec} = require('../../commenting'); const messages = { noStripeConnection: 'Cannot {action} without a Stripe Connection', moreThanOneProduct: 'A member cannot have more than one Product', @@ -2031,7 +2030,7 @@ module.exports = class MemberRepository { */ async saveCommenting(memberId, commenting, actionName, context) { return this._Member.edit( - {commenting: MemberCommentingCodec.format(commenting)}, + {commenting}, { id: memberId, context, diff --git a/ghost/core/core/server/services/members/members-api/services/member-bread-service.js b/ghost/core/core/server/services/members/members-api/services/member-bread-service.js index b2cd1fd3523..ca2d726b3d5 100644 --- a/ghost/core/core/server/services/members/members-api/services/member-bread-service.js +++ b/ghost/core/core/server/services/members/members-api/services/member-bread-service.js @@ -2,7 +2,6 @@ const errors = require('@tryghost/errors'); const logging = require('@tryghost/logging'); const tpl = require('@tryghost/tpl'); const moment = require('moment'); -const {MemberCommentingCodec} = require('../../commenting'); const messages = { stripeNotConnected: 'Missing Stripe connection.', @@ -271,10 +270,6 @@ module.exports = class MemberBREADService { const unsubscribeUrl = this.settingsHelpers.createUnsubscribeUrl(member.uuid); member.unsubscribe_url = unsubscribeUrl; - const commenting = MemberCommentingCodec.parse(member.commenting); - member.can_comment = commenting.canComment; - member.commenting = commenting; - return member; } @@ -402,15 +397,16 @@ module.exports = class MemberBREADService { * @returns {Promise} */ async disableCommenting(memberId, reason, until, context) { - const member = await this.read({id: memberId}); + const model = await this.memberRepository.get({id: memberId}); - if (!member) { + if (!model) { throw new errors.NotFoundError({ message: tpl(messages.memberNotFound) }); } - const updated = member.commenting.disable(reason, until); + const commenting = model.get('commenting'); + const updated = commenting.disable(reason, until); await this.memberRepository.saveCommenting( memberId, @@ -428,15 +424,16 @@ module.exports = class MemberBREADService { * @returns {Promise} */ async enableCommenting(memberId, context) { - const member = await this.read({id: memberId}); + const model = await this.memberRepository.get({id: memberId}); - if (!member) { + if (!model) { throw new errors.NotFoundError({ message: tpl(messages.memberNotFound) }); } - const updated = member.commenting.enable(); + const commenting = model.get('commenting'); + const updated = commenting.enable(); await this.memberRepository.saveCommenting( memberId, @@ -512,10 +509,6 @@ module.exports = class MemberBREADService { }; member.unsubscribe_url = this.settingsHelpers.createUnsubscribeUrl(member.uuid); - const commenting = MemberCommentingCodec.parse(member.commenting); - member.can_comment = commenting.canComment; - member.commenting = commenting; - return member; }); diff --git a/ghost/core/core/server/services/members/utils.js b/ghost/core/core/server/services/members/utils.js index 5b7a49867fb..d3e5a8cf546 100644 --- a/ghost/core/core/server/services/members/utils.js +++ b/ghost/core/core/server/services/members/utils.js @@ -1,5 +1,3 @@ -const {serializeCommenting} = require('../../api/endpoints/utils/serializers/output/utils/member-commenting'); - function formatNewsletterResponse(newsletters) { return newsletters.map(({id, uuid, name, description, sort_order: sortOrder}) => { return { @@ -31,7 +29,7 @@ module.exports.formattedMemberResponse = function formattedMemberResponse(member created_at: member.created_at, enable_comment_notifications: member.enable_comment_notifications, can_comment: member.can_comment, - commenting: serializeCommenting(member.commenting) + commenting: member.commenting }; if (member.newsletters) { data.newsletters = formatNewsletterResponse(member.newsletters); diff --git a/ghost/core/core/server/web/comments/routes.js b/ghost/core/core/server/web/comments/routes.js index f57d7615c37..486013e7bcc 100644 --- a/ghost/core/core/server/web/comments/routes.js +++ b/ghost/core/core/server/web/comments/routes.js @@ -21,7 +21,7 @@ function checkMemberCommenting(req, res, next) { if (req.member && req.member.can_comment === false) { return next(new errors.NoPermissionError({ message: tpl(messages.memberCommentingDisabled), - context: req.member.commenting?.disabledReason + context: req.member.commenting?.disabled_reason })); } next(); diff --git a/ghost/core/core/shared/labs.js b/ghost/core/core/shared/labs.js index 1a320b3a1ac..19d45419a71 100644 --- a/ghost/core/core/shared/labs.js +++ b/ghost/core/core/shared/labs.js @@ -54,8 +54,7 @@ const PRIVATE_FEATURES = [ 'featurebaseFeedback', 'transistor', 'disableMemberCommenting', - 'sniperlinks', - 'paidBreakdownCharts' + 'sniperlinks' ]; module.exports.GA_KEYS = [...GA_FEATURES]; diff --git a/ghost/core/package.json b/ghost/core/package.json index 5f317c55f9c..4f6a458e058 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -1,6 +1,6 @@ { "name": "ghost", - "version": "6.15.0", + "version": "6.16.0", "description": "The professional publishing platform", "author": "Ghost Foundation", "homepage": "https://ghost.org", diff --git a/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap b/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap index 8717846eb59..352aed50d0e 100644 --- a/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap +++ b/ghost/core/test/e2e-api/admin/__snapshots__/config.test.js.snap @@ -25,7 +25,6 @@ Object { "indexnow": true, "lexicalIndicators": true, "members": true, - "paidBreakdownCharts": true, "sniperlinks": true, "stripeAutomaticTax": true, "superEditors": true, diff --git a/ghost/core/test/e2e-api/admin/member-commenting.test.js b/ghost/core/test/e2e-api/admin/member-commenting.test.js index abdd4b15da1..28923f94541 100644 --- a/ghost/core/test/e2e-api/admin/member-commenting.test.js +++ b/ghost/core/test/e2e-api/admin/member-commenting.test.js @@ -4,6 +4,7 @@ const models = require('../../../core/server/models'); const assert = require('assert/strict'); const sinon = require('sinon'); const settingsCache = require('../../../core/shared/settings-cache'); +const {MemberCommenting} = require('../../../core/server/services/members/commenting'); describe('Member Commenting API', function () { let agent; @@ -225,13 +226,9 @@ describe('Member Commenting API', function () { it('Members with expired disable have can_comment: true', async function () { const pastDate = new Date(Date.now() - 1000).toISOString(); // 1 second ago - // Directly set an expired disable in the database (must use stringified JSON) + // Directly set an expired disable in the database await models.Member.edit({ - commenting: JSON.stringify({ - disabled: true, - disabledReason: 'Expired disable', - disabledUntil: pastDate - }) + commenting: MemberCommenting.disabled('Expired disable', new Date(pastDate)) }, {id: member.id}); const {body} = await agent @@ -362,13 +359,9 @@ describe('Member Commenting Service Behavior', function () { it('Member with expired disable has can_comment: true', async function () { const pastDate = new Date(Date.now() - 60000).toISOString(); // 1 minute ago - // Set an expired disable directly (must use stringified JSON) + // Set an expired disable directly await models.Member.edit({ - commenting: JSON.stringify({ - disabled: true, - disabledReason: 'Expired disable', - disabledUntil: pastDate - }) + commenting: MemberCommenting.disabled('Expired disable', new Date(pastDate)) }, {id: member.id}); // Verify via API (can_comment is computed by service) @@ -668,11 +661,7 @@ describe('Member with Commenting Disabled - Comment Restriction', function () { // Set an expired disable directly in the database await models.Member.edit({ - commenting: JSON.stringify({ - disabled: true, - disabledReason: 'Expired disable', - disabledUntil: pastDate - }) + commenting: MemberCommenting.disabled('Expired disable', new Date(pastDate)) }, {id: member.id}); // Member should be able to comment since disable is expired diff --git a/ghost/core/test/e2e-server/__snapshots__/click-tracking.test.js.snap b/ghost/core/test/e2e-server/__snapshots__/click-tracking.test.js.snap index 2df638b8a33..bfe4dd1128a 100644 --- a/ghost/core/test/e2e-server/__snapshots__/click-tracking.test.js.snap +++ b/ghost/core/test/e2e-server/__snapshots__/click-tracking.test.js.snap @@ -15,7 +15,12 @@ Object { "member": Object { "current": Object { "avatar_image": null, - "commenting": null, + "can_comment": true, + "commenting": Object { + "disabled": false, + "disabled_reason": null, + "disabled_until": null, + }, "comped": false, "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "email": "with-product@test.com", diff --git a/ghost/core/test/e2e-webhooks/__snapshots__/members.test.js.snap b/ghost/core/test/e2e-webhooks/__snapshots__/members.test.js.snap index 56dee6599fe..b14079ce08e 100644 --- a/ghost/core/test/e2e-webhooks/__snapshots__/members.test.js.snap +++ b/ghost/core/test/e2e-webhooks/__snapshots__/members.test.js.snap @@ -15,7 +15,12 @@ Object { "member": Object { "current": Object { "avatar_image": null, - "commenting": null, + "can_comment": true, + "commenting": Object { + "disabled": false, + "disabled_reason": null, + "disabled_until": null, + }, "comped": false, "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "email": "testemail@example.com", @@ -62,7 +67,12 @@ Object { "member": Object { "current": Object {}, "previous": Object { - "commenting": null, + "can_comment": true, + "commenting": Object { + "disabled": false, + "disabled_reason": null, + "disabled_until": null, + }, "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "email": "testemail2@example.com", "email_count": 0, @@ -104,7 +114,12 @@ Object { "member": Object { "current": Object { "avatar_image": null, - "commenting": null, + "can_comment": true, + "commenting": Object { + "disabled": false, + "disabled_reason": null, + "disabled_until": null, + }, "comped": false, "created_at": StringMatching /\\\\d\\{4\\}-\\\\d\\{2\\}-\\\\d\\{2\\}T\\\\d\\{2\\}:\\\\d\\{2\\}:\\\\d\\{2\\}\\\\\\.000Z/, "email": "testemail3@example.com", diff --git a/ghost/core/test/unit/api/canary/utils/serializers/output/mapper.test.js b/ghost/core/test/unit/api/canary/utils/serializers/output/mapper.test.js index 1d55088bc6b..36c480a0630 100644 --- a/ghost/core/test/unit/api/canary/utils/serializers/output/mapper.test.js +++ b/ghost/core/test/unit/api/canary/utils/serializers/output/mapper.test.js @@ -387,6 +387,8 @@ describe('Unit: utils/serializers/output/mappers', function () { name: 'name1', expertise: 'expertise1', avatar_image: 'avatar_image1', + can_comment: true, + commenting: {disabled: false, disabled_reason: null, disabled_until: null}, foo: 'bar' }, post: { @@ -624,11 +626,19 @@ describe('Unit: utils/serializers/output/mappers', function () { const model = { id: 'comment3', html: '

comment 3

', - member: {id: 'member1'}, + member: { + id: 'member1', + can_comment: true, + commenting: {disabled: false, disabled_reason: null, disabled_until: null} + }, parent: { id: 'comment1', html: '

comment 1

', - member: {id: 'member1'} + member: { + id: 'member1', + can_comment: true, + commenting: {disabled: false, disabled_reason: null, disabled_until: null} + } }, in_reply_to_id: 'comment2', inReplyTo: { diff --git a/ghost/core/test/unit/server/services/adapter-manager/adapter-manager.test.js b/ghost/core/test/unit/server/services/adapter-manager/adapter-manager.test.js index a3f9f17c2c0..210f90b1451 100644 --- a/ghost/core/test/unit/server/services/adapter-manager/adapter-manager.test.js +++ b/ghost/core/test/unit/server/services/adapter-manager/adapter-manager.test.js @@ -1,5 +1,5 @@ +const assert = require('node:assert/strict'); const sinon = require('sinon'); -const should = require('should'); const AdapterManager = require('../../../../../core/server/services/adapter-manager/adapter-manager'); class BaseMailAdapter { @@ -32,14 +32,12 @@ describe('AdapterManager', function () { adapterManager.registerAdapter('mail', BaseMailAdapter); - try { + assert.throws(() => { adapterManager.getAdapter('mail'); - should.fail(null, null, 'Should not have created'); - } catch (err) { - should.exist(err); - should.equal(err.errorType, 'IncorrectUsageError'); - should.equal(err.message, 'getAdapter must be called with a adapterName and a adapterClassName.'); - } + }, { + errorType: 'IncorrectUsageError', + message: 'getAdapter must be called with a adapterName and a adapterClassName.' + }); }); it('getAdapter throws if config does not contain adapter type key', function () { @@ -55,14 +53,12 @@ describe('AdapterManager', function () { adapterManager.registerAdapter('some_other_adapter_type', BaseMailAdapter); - try { + assert.throws(() => { adapterManager.getAdapter('mail', 'custom'); - should.fail(null, null, 'Should not have created'); - } catch (err) { - should.exist(err); - should.equal(err.errorType, 'NotFoundError'); - should.equal(err.message, 'Unknown adapter type mail. Please register adapter.'); - } + }, { + errorType: 'NotFoundError', + message: 'Unknown adapter type mail. Please register adapter.' + }); }); it('getAdapter can handle looking up from node_modules', function () { @@ -79,13 +75,11 @@ describe('AdapterManager', function () { adapterManager.registerAdapter('mail', BaseMailAdapter); - try { + assert.throws(() => { adapterManager.getAdapter('mail', 'some-node-module-adapter'); - } catch (err) { - should.ok(err); // We don't care about the error, we just want to check `loadAdapterFromPath` - } + }); - loadAdapterFromPath.calledWith('some-node-module-adapter').should.equal(true); + sinon.assert.calledWith(loadAdapterFromPath, 'some-node-module-adapter'); }); it('Loads registered adapters in the order defined by the paths', function () { @@ -112,40 +106,24 @@ describe('AdapterManager', function () { adapterManager.registerAdapter('mail', BaseMailAdapter); - try { - const customAdapter = adapterManager.getAdapter('mail', 'custom', {}); - - should.ok(customAdapter instanceof BaseMailAdapter); - should.ok(customAdapter instanceof CustomMailAdapter); - } catch (err) { - should.fail(err, null, 'Should not have errored'); - } - - try { - const incompleteAdapter = adapterManager.getAdapter('mail', 'incomplete', {}); - should.fail(incompleteAdapter, null, 'Should not have created'); - } catch (err) { - should.exist(err); - should.equal(err.errorType, 'IncorrectUsageError'); - should.equal(err.message, 'mail adapter incomplete is missing the someMethod method.'); - } - - try { - const defaultAdapter = adapterManager.getAdapter('mail', 'default', {}); - - should.ok(defaultAdapter instanceof BaseMailAdapter); - should.ok(defaultAdapter instanceof DefaultMailAdapter); - } catch (err) { - should.fail(err, null, 'Should not have errored'); - } - - try { - const brokenAdapter = adapterManager.getAdapter('mail', 'broken', {}); - should.fail(brokenAdapter, null, 'Should not have created'); - } catch (err) { - should.exist(err); - should.equal(err.errorType, 'IncorrectUsageError'); - } + const customAdapter = adapterManager.getAdapter('mail', 'custom', {}); + assert(customAdapter instanceof BaseMailAdapter); + assert(customAdapter instanceof CustomMailAdapter); + + assert.throws(() => { + adapterManager.getAdapter('mail', 'incomplete', {}); + }, { + errorType: 'IncorrectUsageError', + message: 'mail adapter incomplete is missing the someMethod method.' + }); + + const defaultAdapter = adapterManager.getAdapter('mail', 'default', {}); + assert(defaultAdapter instanceof BaseMailAdapter); + assert(defaultAdapter instanceof DefaultMailAdapter); + + assert.throws(() => { + adapterManager.getAdapter('mail', 'broken', {}); + }, {errorType: 'IncorrectUsageError'}); }); it('Reads adapter type from the adapter name divided with a colon (adapter:feature syntax)', function () { @@ -166,14 +144,9 @@ describe('AdapterManager', function () { }); adapterManager.registerAdapter('mail', BaseMailAdapter); - try { - const customAdapter = adapterManager.getAdapter('mail:newsletters', 'custom'); - - should.ok(customAdapter instanceof BaseMailAdapter); - should.ok(customAdapter instanceof CustomMailAdapter); - } catch (err) { - should.fail(err, null, 'Should not have errored'); - } + const customAdapter = adapterManager.getAdapter('mail:newsletters', 'custom'); + assert(customAdapter instanceof BaseMailAdapter); + assert(customAdapter instanceof CustomMailAdapter); }); it('Creates separate class instances for adapters requested for different features', function () { @@ -194,29 +167,17 @@ describe('AdapterManager', function () { }); adapterManager.registerAdapter('mail', BaseMailAdapter); - let mailNewslettersAdapter; - try { - mailNewslettersAdapter = adapterManager.getAdapter('mail:newsletters', 'custom'); - - should.ok(mailNewslettersAdapter instanceof BaseMailAdapter); - should.ok(mailNewslettersAdapter instanceof CustomMailAdapter); - } catch (err) { - should.fail(err, null, 'Should not have errored'); - } - - let mailNotificationsAdapter; - try { - mailNotificationsAdapter = adapterManager.getAdapter('mail:notifications', 'custom'); + const mailNewslettersAdapter = adapterManager.getAdapter('mail:newsletters', 'custom'); + assert(mailNewslettersAdapter instanceof BaseMailAdapter); + assert(mailNewslettersAdapter instanceof CustomMailAdapter); - should.ok(mailNotificationsAdapter instanceof BaseMailAdapter); - should.ok(mailNotificationsAdapter instanceof CustomMailAdapter); - } catch (err) { - should.fail(err, null, 'Should not have errored'); - } + const mailNotificationsAdapter = adapterManager.getAdapter('mail:notifications', 'custom'); + assert(mailNotificationsAdapter instanceof BaseMailAdapter); + assert(mailNotificationsAdapter instanceof CustomMailAdapter); - should.notEqual(mailNewslettersAdapter, mailNotificationsAdapter); + assert.notEqual(mailNewslettersAdapter, mailNotificationsAdapter); const secondMailNewslettersAdapter = adapterManager.getAdapter('mail:newsletters', 'custom'); - should.equal(mailNewslettersAdapter, secondMailNewslettersAdapter); + assert.equal(mailNewslettersAdapter, secondMailNewslettersAdapter); }); }); diff --git a/ghost/core/test/unit/server/services/custom-redirects/validation.test.js b/ghost/core/test/unit/server/services/custom-redirects/validation.test.js index a3a6f35b7a3..8e3775f3ef7 100644 --- a/ghost/core/test/unit/server/services/custom-redirects/validation.test.js +++ b/ghost/core/test/unit/server/services/custom-redirects/validation.test.js @@ -1,4 +1,4 @@ -const should = require('should'); +const assert = require('node:assert/strict'); const {validate} = require('../../../../../core/server/services/custom-redirects/validation'); @@ -19,12 +19,9 @@ describe('UNIT: custom redirects validation', function () { to: '/' }]; - try { - validate(config); - should.fail('should have thrown'); - } catch (err) { - err.message.should.equal('Incorrect redirects file format.'); - } + assert.throws(() => { + validate(/** @type {any} */ (config)); + }, {message: 'Incorrect redirects file format.'}); }); it('throws for an invalid redirects config having invalid RegExp in from field', function () { @@ -34,12 +31,9 @@ describe('UNIT: custom redirects validation', function () { to: '/' }]; - try { + assert.throws(() => { validate(config); - should.fail('should have thrown'); - } catch (err) { - err.message.should.equal('Incorrect RegEx in redirects file.'); - } + }, {message: 'Incorrect RegEx in redirects file.'}); }); it('throws when setting a file with wrong format: no array', function () { @@ -48,11 +42,8 @@ describe('UNIT: custom redirects validation', function () { to: 'd' }; - try { - validate(config); - should.fail('should have thrown'); - } catch (err) { - err.message.should.equal('Incorrect redirects file format.'); - } + assert.throws(() => { + validate(/** @type {any} */ (config)); + }, {message: 'Incorrect redirects file format.'}); }); }); diff --git a/ghost/core/test/unit/server/services/members-events/event-storage.test.js b/ghost/core/test/unit/server/services/members-events/event-storage.test.js index c33c6d30444..efdca7b51f9 100644 --- a/ghost/core/test/unit/server/services/members-events/event-storage.test.js +++ b/ghost/core/test/unit/server/services/members-events/event-storage.test.js @@ -1,4 +1,3 @@ -require('should'); const sinon = require('sinon'); const {MemberCreatedEvent, SubscriptionCreatedEvent} = require('../../../../../core/shared/events');