closes https://github.com/TryGhost/Toolbox/issues/399
- The MemberCreatedEvent event is more accurate representation of the limit nature - counting the number of members created. The previous MemberSubscribeEvent was slightly hacky solution because a member could be subscribed/unsubscribed multiple times and distorting the limit counts.
fixes https://github.com/TryGhost/Ghost/issues/14508
This change requires the frontend to send an explicit `emailType` when sending a magic link. We default to `subscribe` (`signin` for invite only sites) for now to remain compatible with the existing behaviour.
**Problem:**
When a member tries to login and that member doesn't exist, we created a new member in the past.
- This caused the creation of duplicate accounts when members were guessing the email address they used.
- This caused the creation of new accounts when using an old impersonation token, login link or email change link that was sent before member deletion.
**Fixed:**
- Trying to login with an email address that doesn't exist will throw an error now.
- Added new and separate rate limiting to login (to prevent user enumeration). This rate limiting has a higher default limit of 8. I think it needs a higher default limit (because it is rate limited on every call instead of per email address. And it should be configurable independent from administrator rate limiting. It also needs a lower lifetime value because it is never reset.
- Updated error responses in the `sendMagicLink` endpoint to use the default error encoding middleware.
- The type (`signin`, `signup`, `updateEmail` or `subscribe`) is now stored in the magic link. This is used to prevent signups with a sign in token.
**Notes:**
- Between tests, we truncate the database, but this is not enough for the rate limits to be truly reset. I had to add a method to the spam prevention service to reset all the instances between tests. Not resetting them caused random failures because every login in every test was hitting those spam prevention middlewares and somehow left a trace of that in those instances (even when the brute table is reset). Maybe those instances were doing some in memory caching.
We're planning to change this from a warning to an error and need to
clean the codebase up before we do so.
In all of these cases the shadowing was known about and was not
causing unexpected behaviour, so the refactor consists entirely of
renaming, rather than refactoring/bug fixes.
- I want to upgrade no-shadow to an error, but to do this I need to resolve the outstanding warnings
- This is not all the warnings, just the ones that were easy to fix
closesTryGhost/Team#2007
- uses request context to add referrer source and medium for a new member
- uses integration name as referrer medium if exists
- renames `refSource`, `refMedium` and `refUrl` to `referrerSource`, `referrerMedium` and `referrerUrl` respectively for consistent naming across files and usages
closes https://github.com/TryGhost/Team/issues/1933
- Added click_events to activity feed
- Added support for parsing click_events in the frontend
- Moved url parsing (transform ready) to model layer of LinkRedirect
- Moved `getEventTimeline` method to the top of the event repository
- Added description field to parsed events in the frontend (because we need a second line)
- Fixed: member email not returned in comment_event
refs https://github.com/TryGhost/Team/issues/1899
- Added `addEmailAttributionToUrl` method to MemberAttributionService. This adds both the source attribution (`rel=newsletter`) and member attribution (`?attribution_id=123&attribution_type=post`) to a URL.
- The URLHistory can now contain a new sort of items: `{type: 'post', id: 'post-id', time: 123}`.
- Updated frontend script to read `?attribution_id=123&attribution_type=post` from the URL and add it to the URLHistory + clear it from the URL.
- Wired up some external dependencies to LinkReplacementService and added some dummy code.
- Increased test coverage of attribution service
- Moved all logic that removes the subdirectory from a URL to the UrlTranslator instead of the AttributionBuilder
- The UrlTranslator now parses a URLHistoryItem to an object that can be used to build an Attribution instance
- Excluded sites with different domain from member id and attribution tracking
closes https://github.com/TryGhost/Team/issues/1864
refs https://github.com/TryGhost/Team/issues/1881
- triggers free member email alert via event dispatch from member create method
- passes subscription/stripe data to member creation for paid members so free member alert can be ignored for them
- moves subscription created event being called from webhook controller to `linkSubscription`, allows creating subscription events for all new subscriptions instead of ones just via webhooks
refs https://github.com/TryGhost/Team/issues/1865
- refactors staff service to listen to member and subscription events
- triggers email alerts based on events instead of directly calling the service
- removes staff service dependency for members api
closes https://github.com/TryGhost/Team/issues/1772
- The user facing side of comments recently replaced `bio` with `expertise`.
- To remain consistent we replaced all the references of `bio` with `expertise` throughout the codebase.
- This includes a database column name changing migration, within the `members` table.
- Bumped up the comments-ui version to a new minor (0.10.x) as its a breaking change.
fixes https://github.com/TryGhost/Team/issues/1859
**Problem:**
When for some reason a member has an active subscription (or legacy comped subscription) for product A, and a comped subscription for product B. You cannot remove comped subscription B.
**Fixed by:**
Updating the API to allow more flexible product changes on members.
- Allow the removal of (comped) products on a member, as long as that product doesn't have a related subscription
- (still) allow the addition of comped products to a member, as long as that member doesn't have other active subscriptions. This matches the existing behaviour, but now this is only checked for added products.
- Includes tests for these edge cases
no issue
- Return was missing for `res.end` if an invalid subscription_id was passed
- Added explicit `text/plain` `Content-Type` headers to error messages to avoid MIME sniffing
Signed-off-by: Elijah Conners <business@elijahpepe.com>
Co-authored-by: Simon Backx <simon@ghost.org>
refs https://github.com/TryGhost/Team/issues/1826
Geolocation was prev. loaded after member was created and updated on existing member. this was mostly due to historical context where we couldn't store data on magic link token.
Since email alerts go out at the time of member creation, this flow missed out on attaching member's location to email.
This change -
- stores request ip when a member asks for magic link in the token
- loads request ip from token when member uses magic link, and for new members loads their geolocation and stores it with member creation
refs TryGhost/Team#1826
- triggers paid subscription cancellation alert for staff users
- passes tier and subscription information for the email - loads tier info from DB for the subscription tier
refs https://github.com/TryGhost/Toolbox/issues/387
- There will three distinct verification limits soon. To keep the naming clear "configThreshold" would be too generic/confusing to use.
- Introduced jsdoc descriptions for the "source" parameter, which will be corelating with each new config parameter ("apiTriggerThreshold", "importTriggerThreshold", "adminTriggerThreshold", etc.). This should give a better visibility into parameters we are dealing in this area.
refs https://github.com/TryGhost/Team/issues/1833
refs https://github.com/TryGhost/Team/issues/1834
We've added the attribution property to subscription and signup events when the
flag is enabled. The attributions resource is fetched by creating multiple relations
on the model, rather than polymorphic as we ran into issues with that as they can't
be nullable/optional.
The parse-member-event structure has been updated to make it easier to work with,
specifically `getObject` is only used when the event is clickable, and there is now a
join property which makes it easier to join the action and the object.
refs https://github.com/TryGhost/Team/issues/1833
refs https://github.com/TryGhost/Team/issues/1834
We've added the attribution property to subscription and signup events when the
flag is enabled. The attributions resource is fetched by creating multiple relations
on the model, rather than polymorphic as we ran into issues with that as they can't
be nullable/optional.
The parse-member-event structure has been updated to make it easier to work with,
specifically `getObject` is only used when the event is clickable, and there is now a
join property which makes it easier to join the action and the object.
closes https://github.com/TryGhost/Toolbox/issues/386
- When the API request was made using staff token the source attribution was "user" instead of "api". Misattribution caused ripple effects in limit service.
- The fix also adds a new combination of data available on the `req` object - both `user` and `api_key` can be present when the request is done using a staff (user) token. Having both pieces of data on the request object gives more context for business logic, did not find a good reason to keep it "pure" with either `api_key` or `user` property.
refs https://github.com/TryGhost/Toolbox/issues/386
- Reusing tontext mapping logic to improve maintainability. It seems like the `update` method was not updated properly or intentionally was left out from 'import' source as that should not ever happen theoretically. Probably the latter is most likely.
- My reasoning on reusing same context to source mapping is: it is better to attribute an appropriate "import" source here. Who knows, maybe we'll have logic in the future where the importer updates instead of skipping existing members. It would not make sense to attribute the source to 'member' in that case, amirite?
- This refactor also makes maintainability of this code way easier
refs https://github.com/TryGhost/Team/issues/1728
- previously, we allowed a member to be mapped to multiple tiers simultaneously as an edge case, in case they managed to signup via another subscription
- since this was always an edge case and not supported, to simplify the flows going forward now that complimentary members can also upgrade, in case of an active subscription we'll always just attach the associated tier to member and remove all other tiers mapped to it
refs https://github.com/TryGhost/Team/issues/1808
refs https://github.com/TryGhost/Team/issues/1809
refs https://github.com/TryGhost/Team/issues/1820
refs https://github.com/TryGhost/Team/issues/1814
### Changes in `member-events` package
- Added MemberCreatedEvent (event, not model)
- Added SubscriptionCreatedEvent (event, not model)
### Added `member-attribution` package (new)
- Added the AttributionBuilder class which is able to convert a url history to an attribution object (exposed as getAttribution on the service itself, which handles the dependencies)
```
[{
"path": "/",
"time": 123
}]
```
to
```
{
"url": "/",
"id": null,
"type": "url"
}
```
- event handler listens for MemberCreatedEvent and SubscriptionCreatedEvent and creates the corresponding models in the database.
### Changes in `members-api` package
- Added urlHistory to `sendMagicLink` endpoint body + convert the urlHistory to an attribution object that is stored in the tokenData of the magic link (sent by Portal in this PR: https://github.com/TryGhost/Portal/pull/256).
- Added urlHistory to `createCheckoutSession` endpoint + convert the urlHistory to attribution keys that are saved in the Stripe Session metadata (sent by Portal in this PR: https://github.com/TryGhost/Portal/pull/256).
- Added attribution data property to member repository's create method (when a member is created)
- Dispatch MemberCreatedEvent with attribution
### Changes in `members-stripe-service` package (`ghost/stripe`)
- Dispatch SubscriptionCreatedEvent in WebhookController on subscription checkout (with attribution from session metadata)
refs https://github.com/TryGhost/Team/issues/1726
Free trial offers don't have a Stripe coupon created for them, as the trial is directly added to checkout session. So for mapping a subscription to offer, we pass the offer id directly from checkout metadata to link the subscription in backend with right offer data. This also handles the case where the offer id against a subscription can get overwritten for a subsequent subscription event, as the sub event from Stripe doesn't has the trial offer info.
- handles storing an offer id for a subscription
- updates member detail in Admin to show the offer info for a subscription
refs https://github.com/TryGhost/Team/issues/1726
- free trial offers don't need a stripe coupon created for them
- checkout sessions for free trial offers ignore stripe coupon and directly pass the trial days value
- trial days of an offer take precedence over trial days added as default to a tier
Without this check, an inactive price in our database will just be
reactivated each time it is required. This can cause issues when
prices have been deleted.
By adding this constraint to the query, we will create a new price in
Stripe and our database when attempting to use an inactive price, this
is particularly useful when trying to fix problems caused by Stripe
prices being deleted.
refs https://github.com/TryGhost/Team/issues/1724
With free trials, members can start subscriptions with a trial period. This change stores the information about trial start and end date for every subscription so it can be shown on Admin/Portal for member.
- adds new `trial_start_at` column for storing trial start date on Stripe subscription. Will in most cases match the start of subscription date.
- adds new `trial_end_at` column for storing trial end date on Stripe subscription.
- wires storing trial start and end values on stripe subscription
refs https://github.com/TryGhost/Team/issues/1724
- wires trial days stored on a tier to stripe checkout session creation
- removes deprecated `trial_from_plan` if trial days is set
refs https://github.com/TryGhost/Team/issues/1716
- Adds the bio field to the API output
- Allow setting bio when updating the member
- Includes new E2E tests for the members API that were missing
refs https://github.com/TryGhost/Team/issues/1709
- New event type `comment_event` (comments and replies of a member in the activity feed)
- Includes member, post and parent relation by default
- Added new output mapper for ActivityFeed events
**Changes to `Comment` model:**
* **Only limit comment fetched to root comments when not authenticated as a user:**
`enforcedFilters` is applied to all queries, which is a problem because for the activity feed we also need to fetch comments which have a parent_id that is not null (`Member x replied to a comment`). The current filter in the model is specifically for the members API, not the admin API (so checking the user should fix that, not sure if that is a good pattern but couldn’t find a better alternative).
* **Only set default relations for comments when withRelated is empty or not set:**
`defaultRelations`: Right now, for every fetch it would force all these relations. But we don’t need all those relations for the activity feed; So I updated the pattern to only set the default relations when it is empty (which we also do on a couple of other places and seems like a good pattern). I also updated the comments-ui frontend to not send ?include
refs https://github.com/TryGhost/Team/issues/1717
- Updates last_commented_at and last_seen_at (only once a day)
- Used the LastSeenAtUpdater, so we can combine updating last_commented_at and last_seen_at in one query + used same pattern
- Updated comments service to await emails in order to make E2E tests more stable (as we don't have any method to await emails and test emails otherwise). This removed the email sending logic from the `onCreated` hook of the model.
refs https://github.com/TryGhost/Team/issues/1174
This paves the way for Ghost to be able to redirect to the referrer
page when dealign with signup magic links. We pass the referrer for
all types of magic links however, to allow extension of this
functionality in the future.
We've also removed the concept of `requestSrc` which has been unused
for a while now.
refs https://github.com/TryGhost/Team/issues/1674
- While preparing the changes had a look around and made small refactors to understand the codebase a little better. In general it's best to keep the method parameters as small and precise as possible instead of passing around a "bag-of-all-the-things" like "data" around
refs https://github.com/TryGhost/Team/issues/1674
- While preparing the changes had a look around and made small refactors to understand the codebase a little better. In general it's best to keep the method parameters as small and precise as possible instead of passing around a "bag-of-all-the-things" like "data" around
refs https://github.com/TryGhost/Team/issues/1526
We weren't using the `req.body.customerEmail` to load a member and
check their existing tiers, this meant that existing members which
were signed out and attempted to create a stripe checkout session were
able to.
refs https://ghost.slack.com/archives/C02G9E68C/p1652980792270029
When bulk unsubscribing members, the number of deleted newsletter relations are returned instead of the number of members with newsletters that were cleared. This update deletes newsletter relations on member_id, so we can return the count of members instead of newsletter relations that were deleted.
Tests in https://github.com/TryGhost/Ghost/pull/14871
- In case of an offer id present in stripe checkout session, the cadence and tier id values can be null
- this fixes the invalid 400 thrown on missing cadence value while creating stripe checkout session for offer
no issue
- Setting a dedicated source for imported members was already done with 455778662c
- This unifies the same source but keeps the usage of context setting on init alive
refs https://github.com/TryGhost/Team/issues/1577
The call to `edit` was not loading the newsletter relations which is needed
by the serializer used by the webhooks service.
Co-authored-by: Fabien "egg" O'Carroll <fabien@allou.is>
no issue
Using a designated source for members being added within the `importer` context will better describe the source in our members event table. It's more consistent with the usage of the other possible sources.
no issue
When importing members, the members-importer isn't aware of the context and therefore falls back to use `member` as a source in members event table. This makes it impossible to determine imported members from others.
- Added an `context` property to the options in the members-importer constructor
- Checked for `importer` context when creating member events and assigned the source `admin` to it
refs https://github.com/TryGhost/Team/issues/1478
- When creating or editing a member, we'll add the newsletter ids to the subscribe events
- When susbcribing to multiple newsletters, we'll create multiple subscribe events
- When removing newsletters from a member, we'll create one or more subscribe events
- Tests in https://github.com/TryGhost/Ghost/pull/14579
refs https://github.com/TryGhost/Team/issues/1520
- Instead of doing the matching of the offers and subscriptions by looking at the offer redemptions, we can now look at the offer_id from subscriptions.
- This also fixes an issue where we don't attach the offer object to subscriptions in the members' browse method
- Updated browse behaviour to match the read behaviour of members (product relation needs to get loaded because it is missing in member.products if the subscription is expired).
Tests in https://github.com/TryGhost/Ghost/pull/14515
refs https://github.com/TryGhost/Team/issues/1519
- Removed offer metadata again from offer (added in one of the previous commits)
- Added `getByStripeCouponId` method in offer repository (required to find an offer based on the stripe_coupon_id)
- Match discounts from Stripe based on the stripe_coupon_id instead of metadata
refs https://github.com/TryGhost/Team/issues/1519
- Added offer repository dependency to member repository (offerAPI didn't work because it creates a new transaction that resulted in a deadlock during tests)
- Store the offer id from the Stripe subscription metadata in the subscription (only if the discount is still active)
- Also added the offer id to the metadata for a Stripe coupon, this will make adding and removing coupons a bit more foolproof
- Prefer the usage of the offer metadata from a coupon if it is present
- When no discount is applied to a subscription, it always sets the offer id to null, even when the metadata still contains the offer
- The offer_id remains stored when a subscription is canceled/expired
refs https://github.com/TryGhost/Team/issues/1469
Previously, members were subscribed to all available newsletters by default when added. This change updates the default newsletters subscription for member to take into account newsletter preferences for auto opt-in as well as visibility.
refs: https://github.com/TryGhost/Toolbox/issues/293
Things needed to create this:
* MemberSubscriptionEvent now has an import source
* Importer now creates events with this type
* Verification trigger logic changed to use 30 day window of imports
* Updated member mrr_delta calculation to use stored mrr
refs https://github.com/TryGhost/Team/issues/1456
Simplifies the calculation of MRR deltas, which will make it easier to update MRR to include offers and cancellations in the future.
* Adjusted MRR and MRR delta calculation to consider "forever" duration offers
refs https://github.com/TryGhost/Team/issues/1451
Uses the discount information from Stripe to calculate the MRR (this was the easiest way to include it + also supports manually created discounts from users)
refs https://github.com/TryGhost/Team/issues/1490
With multiple newsletters, free members can choose their newsletter subscription preference while signing up.
This change -
- includes newsletters data in magic link token creation
- attaches newsletter data to new members created via magic link
refs https://github.com/TryGhost/Team/issues/1456
- Sets the `mrr` column correctly when updating a subscription
- We can use this value in the future to have an improved calculation of mrr_delta, but we first need to have the migration in place
- Updated missing cancelled -> canceled
refs https://github.com/TryGhost/Team/issues/1469
When a new member signs up on the site, by default they should get subscribed to all newsletters. A new member can get added via different ways (stripe webhook, admin add, free signup), so this change updates the base member repository which gets used irrespective of how member is added to ensure default newsletters are always included for new member.
If a member already has a custom newsletters list attached, we don't change anything.
refs https://github.com/TryGhost/Team/issues/1469
With multiple newsletters, members can now have one or more newsletter subscriptions that is attached to them. This change updates the member BREAD service to handle attaching newsletter data to member, based on the newsletter flag.
- if newsletter flag is disabled, add/edit methods delete any newsletter data attached to member
- sets `newsletters` as a default relation for read/browse methods so a member always has newsletter data attached to them
no issue
- When a new member is added via the API, with a stripe_customer_id, the member bread service is passed also with a default option: `withRelated: ['labels']`.
- This option is passed along to the member repository, and further and is also passed when loading relations that don't have a relationship called 'labels'.
- This results in a `labels is not defined on the model` error when you try to create a new member with a stripe customer id.
- Test that found this issue will be added to the Ghost repo.
refs https://github.com/TryGhost/Team/issues/1141
- when a member had canceled subscriptions the check we have to match products to subscriptions to determine whether to insert the hardcoded complimentary subscription was incorrectly matching against the canceled subscriptions
- updated to match only active subscriptions
refs https://github.com/TryGhost/Team/issues/1141
- The goal is to retrieve the product name for canceled subscriptions
- The property is located at `member.subscriptions.price.product.name`
- We can't easily get the full product as products are retrieved using a join through `members_products`, and there is only a `members_products` row when the subscription is active
refs https://github.com/TryGhost/Team/issues/1387
First, this default value didn't make sense as it was written when we
had a boolean visible column, second, we don't need to add defaults here
- this implementation is super tied to the models anyway, so we may as
well rely on the model defaults - if we do pull it out similar to
Offers, then it may make sense to add a default there.
We also update the JSDoc to match the updated data structure.
refs: 23b383bedf
- @tryghost/error constructors take an object, not a string - the expectation is that message, context & help should all be set
- This does the bare minimum and just ensures message is set correctly
refs https://github.com/TryGhost/Team/issues/1322
We no longer restart the Members service based on the Stripe service
being updated, which meant that if it was initially configured with
missing URL's and later Stripe connected, it would not get the new
config until a server restart. This moves the last of Stripe config into
the Stripe service, so that all things concerning Stripe can be handled
in one place and updated together.
refs https://github.com/TryGhost/Team/issues/1319
Rather than allowing our code to attempt to speak with Stripe and error
there, we opt to fail fast and throw validation errors so that our API
will respond with a 422, the existing pattern for validation errors.
