no-issue
We were incorrectly mixing transactional and non-transactional
operations. An e2e test in Ghost will be merged shortly which caught
this problem.
refs https://github.com/TryGhost/Team/issues/1299
- Convert `created_at` to the right column in each function query
- Renamed the misspelled `getEmailDeliveredEvents` function
- Updated existing unit tests to cover the order
refs: https://github.com/TryGhost/Toolbox/issues/166
New package handles the email verification workflow to prevent spammers. It currently handles MembersSubscribeEvent to detect potential abuse of the API to add members, and exposes methods for checking the threshold / starting the verification process for use by other areas of the code (at the moment - just member imports).
The import package no longer needs to handle anything related to verification since it can be handled in the wrapper function in Ghost, and the API package doesn't need to do anything other than dispatch the new event.
refs https://github.com/TryGhost/Team/issues/1277
- When a user signs-up, two events are created, the code was only keeping one of these events.
- This was introduce in commit 58c9c1c649 when the only usage of the function was to extract the 5 most recent events. Any duplication was creating too much noise.
- This was creating issues now that we introduce event filtering. Some `newsletter_event` events would appear from nowhere we we were filtering-out `signup_event` events.
- We removed the deduplication when the `membersActivityFeed` flag is enabled.
refs https://github.com/TryGhost/Team/issues/1277
- In `getEventTimeline` we filter to only perform the relevant queries, passing to each query function the filters (subset of NQL)
- In each query function, we rewrite the filters to adapt them to the internal data shape.
- We need to do this rewrite to allow API consumers to create filters based on the output on the API instead of the internal data structure.
- Added partial unit tests as there is a lot of repetition between the query functions.
refs https://github.com/TryGhost/Team/issues/1277
- This will allow to filter events within `getEventTimeline`
- The subset of NQL has the following rules:
- Only one level of filters, now parenthesis allowed
- Only three filter keys allowed
- No `or` allowed outside of the bracket notation (this is allowed: `type:-[email_opened_event,email_failed_event]` but this isn't: `type:1,data.created_at:1`)
- The return is an object with a NQL filter by allowed filter key
refs https://github.com/TryGhost/Team/issues/1252
Although we filter out archived tiers from being shown in Portal - we
must also persist this information, so that when they are unarchived,
they continue to not be shown in Portal.
Unfortunately this information is stored in a setting, rather than on
the Tier object itself, two things to consider for the future are:
1. Representing the display value on the Tier object in the API
2. Updating the DB tables to allow storing the display value on the Tier
refs https://github.com/TryGhost/Team/issues/1277
- Doing a filtered DB query wasn't a good solution because we query several tables with different shapes.
- Filtering in memory is done with the NQL library
- Removed the side-effect of changing options.filter for each query, now that options.filter will already contain NQL
- The filtering is done in the reduce function to avoid looping one more time over the in-memory events
refs https://github.com/TryGhost/Team/issues/1277
- Added the pre-existing framework filters to each event query.
- Made sure we aren't modifying the original `options.filter` to avoid side-effects between event queries.
refs https://github.com/TryGhost/Team/issues/1252
This will be used to archive and unarchive Tiers. There is a restriction
on archiving "free" Tiers because our current system expects only one,
and it should always be active.
refs https://github.com/TryGhost/Team/issues/1277
- The members weren't included in the serialized version of the new events
- Some properties weren't using the bookshelf `get` method as they should have
refs https://github.com/TryGhost/Team/issues/1277
- Adds 3 new requests to the `email_recipients` table in the `getEventTimeline` method
- This allows to extract new member events from the table: `email_delivered_event`, `email_opened_event`, `email_failed_event`
refs https://github.com/TryGhost/Team/issues/1037
Tiers have a new `type` column to differentiate between `free` and `paid` tiers. This change -
- sets type as paid for all new tiers created, as `free` tier is created by default
- excludes any price/stripe data change for free tier
- updates all usages of default product to fetch the first paid product from the products list in DB instead of just the first product it finds.
refs https://github.com/TryGhost/Team/issues/1275
We want to be able to track where member subscriptions came from, so
that we can use the information to reduce spam imports of members.
We were missing information when members were uploaded via the Admin
API, and setting the source to 'member' be default - this fixes that
both when creating members and when updating their subscription status.
no-issue
Running these in a transaction ensures that they do not partially execute or run
into race conditions with simultaneous operations via the API.
refs https://github.com/TryGhost/Team/issues/1257
This gets us closer to not having to reload the MembersAPI when config
is changed which will help stop bugs arising from multiple instances of
the MembersAPI being created.
refs https://github.com/TryGhost/Team/issues/1259
These errors are thrown by nodemailer and can occur when an invalid
email address is used. Without special handling these cause a 500 error.
refs https://github.com/TryGhost/Team/issues/789
We are still having issues with duplicate subscriptions being inserted,
despite running our code in transactions. For now we will catch these
errors and response ot Stripe with a 409 so that it'll retry later - and
it stops us from throwing 500's
closes https://github.com/TryGhost/Team/issues/1238
- previously returned 500 errors when a subscription had multiple prices due to external tampering on Stripe directly
- instead now returns 400 Bad Request error when subscriptions don't have right number of prices
refs https://github.com/TryGhost/Team/issues/1067
This decouples the contents/type of email from the webhooks service,
allowing us to easily make changes to the type of email sent, without
having to make changes to the webhooks service.
refs https://github.com/TryGhost/Team/issues/1243
It's possible to get into strange states where a subscription in Ghost
doesn't have an associated Price. This then has knock on effects because
we're dealing with data in an undefined state. Rather than add guards
against this throughout the entire stack, we stop returning it from the
BREAD API. It might be worth considering removing these subscriptions
from the response of the repository, but for now this is the most
minimal change that fixes the problem.
refs https://github.com/TryGhost/Team/issues/1092
refs https://github.com/TryGhost/Team/issues/1135
This was missed in the initial due to the issue tracking the task being
superceded, and the task not being copied across to the superceding
issue.
A new method to remove coupons has been added, as opposed to updating
the existing change subscription price method, because the removal of a
coupon is not the concern of an auxillary stripe service, but a busines
concern that should be explicit in the members-api codebase.
refs https://github.com/TryGhost/Team/issues/1067
The auto-login behaviour obliterates the concept of a success URL,
because the Member is redirected in a logged in state, to the welcome
URL - rather than a logged out state to the success URL.
In order to not disrupt existing flows, we disable auto login if a
success URL is provided.
refs https://github.com/TryGhost/Team/issues/1067
This is the MVP for auto-login of Members, it does not support custom
redirects, and will always just redirect to the same place that the
signin & signup links do. Behind a feature flag whilst we iron out the
functionality.
refs https://github.com/TryGhost/Team/issues/887
Our invoice webhook handling code assumed that every invoice would be
for a subscription, but that is not the case. There are valid use-cases
of using the same Stripe account in order to sell items with a one-off
purchase. Here we update the handling to ignore all invoices which are
not for subscriptions.
no-issue
When adding a new member, we allow an email to be sent, and the type of
email to be chosen. This choice was being overriden by our signup email
logic - here we allow the BREAD API to have full control over which
email is sent.
refs https://github.com/TryGhost/Team/issues/885
This webhook isn't used and can cause issues when Checkout Sessions are
completed but with a failed payment. Removing it will remove those
errors.
refs https://github.com/TryGhost/Team/issues/1135
We use the OffersAPI to fetch Offers, so that we can be using the same
format for Offers in all of our APIs.
We will not attach the Offer to the Subscription if either the Tier or
the Cadence do not match. This is because the Offer would no longer
apply to this Subscription.
We do however retain the data, so that a Member can still be filtered on
the Offers which they've redeemed.
refs https://github.com/TryGhost/Team/issues/1166
By using the PaymentsService to fetch coupon information - we ensure
that the coupons are created if they're missing. Like in the case of a
Stripe disconnect/connect cycle.
refs linear.app/tryghost/issue/CORE-74/improve-the-test-situation
- this commit adds the codecov GitHub Action into CI so we can upload
coverage reports
- the coverage files need to be in XML for them to work with
codecov, so this commit also adds cobertura (XML) as a reporter
no-issue
Without forcing linkSubscription to run inside a transaction - it's
possible to have race conditions where it is called twice, and attempt
to insert duplicate rows into the database.
refs https://github.com/TryGhost/Team/issues/1132
We have to include the Offer on the metadata for the Stripe Checkout -
as Offers with a duration of 'once' will not always be present on the
Subscription after fetching it.
Once we receive the Stripe Checkout webhook we emit an event for
subscription created - the reason we use an event is because this logic
should eventually live in a Payments/Stripe module - and we'd want to
decouple it from the Members module.
The Members module is in charge of writing Offer Redemptions - rather
than the Offers module - because Offer Redemptions are "owned" by a
Member - and merely reference and Offer. Eventually Offer Redemptions
could be replaced by Subscriptions.
refs https://github.com/TryGhost/Team/issues/1156
Because we were only attempting to add the product to the members if the
subscription was new AND active - we would not add it for incomplete
subscriptions transitioning to active.
Instead we always attempt to add the product to a member for an active
subscription - it doesn't matter if it's a new one. We later have logic
to filter out duplicate products if the member already has access to the
product.
refs https://github.com/TryGhost/Members/commit/5172e40646
When we updated to use the OffersAPI instead of OfferRepository this was
missed, and we were passing blank coupon to Stripe Checkout. This should
eventually be replaced with a call like `getCoupon(offerId)` from a
payments service.
refs https://github.com/TryGhost/Team/issues/1133
An archived Offer is intended to be disabled from a redemption point of
view. This ensures that we do not allow Stripe Checkout Sessions to be
created for them.
no-issue
The OfferRepository deals with domain objects in the Offers module, and
as such is not suitable for use with "external" services. This update
means that MembersAPI can deal with POJO DTOs so that there is not a
dependency on the internals of the Offers module. Just on the contract
it holds with the outside world.
refs https://github.com/TryGhost/Team/issues/1090
Instead of the hardcoded 1-day version for Offers, we can now talk
directly to the Offers repository and use the real values for Stripe
Checkout.
refs https://github.com/TryGhost/Team/issues/1083
The Offers service is going to need access to the StripeAPIService too,
so we must pull its initialisation out of this module up to the Ghost
application layer, which will allow us to pass a reference of the
StripeAPIService to wherever needs it.
refs https://github.com/TryGhost/Team/issues/1090
This 1-day version of Offers allows us to test the full flow of the
Offers feature without having to implement all of it. The focus here is
that we can pass an Offer ID when creating a Stripe Checkout session and
have it apply. Here we use hardcoded Stripe Coupons as we haven't yet
got persistence implemented for Offers & their related Stripe Coupons
no-issue
Without a return after ending the response, the code will continue to
attempt to send emails and then send another response which results in
an uncaught error.
refs https://github.com/TryGhost/Ghost/security/advisories/GHSA-65p7-pjj8-ggmr
The email address change flow was built on top of the unauthenticated
signin/signup flow. This meant that ownership of the email being changed
wasn't verified and allowed a malicious actore to change the email
address of arbitrary accounts to an email address which they controlled.
We remove the ability to change email addresses from the signin/signup
flow and instead create a dedicated, authenticated flow for changing
email address.
refs https://github.com/TryGhost/Team/issues/1054
We need to instantiate the MemberAnalyticsService so that we can start
listening to events and storing them, this is the minium glue code
required to get us going.
refs https://github.com/TryGhost/Team/issues/1057
This method will validate a token, and then return the member associated
with it. Rather than exposing token validation and coupling consumers to
the structure of the token response data.
refs https://github.com/TryGhost/Team/issues/873
This ensures that all requests to the API will include the mock
subscriptions for comped members. Allowing the Admin to correctly show
the subscription information after adding and editing members. As well
as having the correct information when navigating from the list of
members to an individual member.
no-issue
Previously we would not create an instance of the StripeAPIService if
Stripe was not configured, but that is not the case any more, instead we
have a configured flag on the service. The webhook route handler was not
updated to use this flag and so would attempt to handle webhooks without
having any of the required data. This would result in an uncaught error.
refs https://github.com/TryGhost/Team/issues/1006
When disconnecting from Stripe, we currently do not remove the webhooks,
this will result in the webhooks from Stripe failing, and tending toward
a 100% error rate, which will ultimately result in emails from Stripe
about the failing webhook.
In order to stop all of that from happening, we should make sure that we
actively remove the webhook from Stripe when disconnecting.
refs https://github.com/TryGhost/Team/issues/1006
As part of the work to handle cleaning up webhooks when we disconnect
from Stripe, I'm moving the logic to clear out the Stripe related data
from the database into a disconnectStripe method. This then allows us to
start handling the cleanup of webhooks via the Stripe API.
refs https://github.com/TryGhost/Team/issues/995
Since we reintroduced the comped status, we did not update the
subscription handling to correctly set members to a status of comped
when they were on a 'Complimentary' plan.
no-issue
Since updating the product repository to force transactions, the options
parameter was used in every call, meaning it wasn't optional any more,
which broke usage. This updates the parameter to have a default so that
existing usage still works.
no-issue
Since we run our product repository methods in transactions now we must
ensure that all database interations in the method use the transaction.
This adds the missing options to the reading of existing prices so that
they happen inside of the transaction.
refs https://github.com/TryGhost/Team/issues/982
These calls to the edit method were missing the transaction option from
the parent which meant that they ran outside of the transaction and
would cause the method to timeout.
no-issue
As subscriptions are a default relation, and we now require products to
populate subscriptions for comped members, we need to include products
by default when reading members.
refs https://github.com/TryGhost/Team/issues/986
The getMemberIdentityData is a relic of time past. Originally it was
used before we had anything like the member repository or bread
controller as a way for things inside of the Members ecosystem to get
access to member data.
This updates it to use the same interface as everything else for
fetching members so that we can rely on the shape of the data that we
consider a member.
This update will ensure that themes have access to the dummy
subscriptions created by the `read` method of the MemberBREADService.
refs https://github.com/TryGhost/Team/issues/979
This correctly handles updates to subscriptions so that if the product a
subscription is for has changed, we will remove the previous product, if
and only if there is not another subscription which gives access to it.
refs https://github.com/TryGhost/Team/issues/873
The `get` method of the member repository will return null when no
member is found - we must ensure that we don't attempt to call toJSON!
It is also possible for a member to not have any products, in which case
we should not attempt to iterate over them, and we can return early.
refs https://github.com/TryGhost/Team/issues/873
This is an unexpected state, but possible if the alpha version of tier
has been enabled previous to the events being added.
refs https://github.com/TryGhost/Team/issues/873
This adds a dummy subscription for each product that a member has
without an associated stripe subscription. It allows clients to deal
with things like a created date for comped members.
no-issue
The idea of this service is to sit infront of the repository and handle
application logic which does not belong at the data layer. The exact
naming and structure is TBC but this gives us a place to start pulling
logic out of the controllers, without having to mash it all into the
repository.
Also important to note is that is does not return instances of bookshelf
models, but a JSON representation of the model, this allows us to not
leak internal implementation to consumers.
refs https://github.com/TryGhost/Team/issues/873
This handles the creation of product events when a members access to
products is changed. This can happen on creation, update, and any
changes to stripe subscriptions.
We manually workout the difference between the current products and the
new products, and add the events accordingly.
no-issue
Calling ObjectId doesn't return a string a but an ObjectId object.
Whilst this object is cast to a string via the toJSON and toString
methods, this is not enough for MySQL. Instead we should explicitly cast
this to a string ourselves and the application level.
refs https://github.com/TryGhost/Team/issues/870
- using `c8` allows us to see test coverage for all packages in the repo
- this commit adds `c8` as a dev dependency and prepends the `mocha`
command with `c8` so it runs on all tests
refs https://github.com/TryGhost/Team/issues/946
This adds the bulk edit method which handles bulk edit operations to members
to be used by the filtering feature. They have been combined into a single method
as that is how they are exposed to the API. This is definitely a candidate for a
refactor in the form of a service in front of the repository.
refs https://github.com/TryGhost/Ghost/commit/1dd52075
- Fixes bulkDestroy being passed the context
- Fixes passing options.search to the model layer
- Updates return value since the changes in referenced commit
no-issue
The logic for bulk destroy is currently incorrectly inside of the
members api controller in Ghost core. Moving it out to here allows us to
simplify the controller to rely on the service, rather than implement
the logic.
refs https://github.com/TryGhost/Team/issues/959
Because we were using the pre-existing products to determine a members
status, instead of the products _after_ we have handled the updates to
subscriptions, members with a paid subscription which was later canceled
were changed to 'comped' rather than 'free'. This adds a final check to
set a member to 'free' if their new set of products is empty.
refs https://github.com/TryGhost/Team/issues/909
The member identity data currently attaches several extra data points to member information which is not used/needed, and causes multiple DB queries on each page load when Portal requests for member via `/members/api/member` endpoint. This change removes all the unused data points on member - `labels`, `stripe_customer`, products`, `stripe_product` cutting DB queries in half.
refs https://github.com/TryGhost/Team/issues/919
As we pass the `benefits` to the Product model on creation, we do not
need to manually fetch them again. In fact doing so causes a strange SQL
error, where we attempt to run `SELECT undefined.*`.
refs https://github.com/TryGhost/Team/issues/664
The well known controller is designed to handle any requests to the
/.well-known endpoint where the members app is mounted. The first and
only requirement so far is that we expose a JSON Web Key Set so that
external services are able to validate Members JWT's
closes https://github.com/TryGhost/Team/issues/778
- cleans up the stripe migration to add default monthly/yearly prices for sites, which had a possibility of using complimentary (0 amount prices) in edge cases
- adds missing return in the same migration for an unlikely failure to parse stripe plans
refs https://github.com/TryGhost/Team/issues/542
Importing members with a created_at date will incorrectly create events
for the member for the date of the import. This updates our event
handling to use either the passed created_at date, or in the case of
subscriptions the start_date of the subscription. We're using start_date
for subscriptions rather than created, as this is more accurate because
start_date works correctly for backdated subscriptions in Stripe.
closes https://github.com/TryGhost/Team/issues/660
All subscriptions in Ghost are expected to have a corresponding price details in `stripe_price` table, which is used to determine the Stripe price a subscription is on. In some edge cases, specially before we started deleted old Stripe data during Stripe disconnect, it's possible that a subscription exists in DB without having a corresponding Stripe price in the DB. These subscriptions are not active for the connected Stripe account, and are save to remove. Going forward, all existing subscriptions with connected account will be removed when disconnecting stripe so we shouldn't have invalid subscriptions in DB in future.
The goal of this migration is to clean all such subscriptions from the DB to avoid any issues around missing price with invalid subscriptions.
refs https://github.com/TryGhost/Team/issues/858
Replacing the check for subscriptions with products ensures that Stripe
Checkout is not able to be opened by comped members.
refs https://github.com/TryGhost/Team/issues/790
We were missing a check for the existence of memberData.products before
attempting to read the length property from it, which can result in an
Uncaught TypeError
refs https://github.com/TryGhost/Team/issues/790
When linking a subscription to a member we must also update their
status. We could be in one of many states, so we start with the initial
values of either 'free' or 'comped' based on whether the member has any
products. We then make sure to updated the status to 'paid' if we find
any active subscriptions associated with the member, otherwise we leave
it as the initial value.
refs https://github.com/TryGhost/Team/issues/790
Both creating and updating members only ever need to explicitly set
either the 'comped' or 'free' status as these methods do not deal with
Stripe. When updating a member if the products are not changing, we do
not attempt to change the status either.
no issue
- we're killing off `ghost-ignition` in favor of explicit packages
containing its individual components
- this commit switches the Members repo to using the
`@tryghost/ignition-errors` and `@tryghost/debug` dependencies,
updates the code with relevant changes and removes the `ghost-ignition`
dependency
refs https://github.com/TryGhost/Team/issues/748
This ensures that you cannot add more than one product to a Member.
However it does allow a Member which already exists with more than one
Product to continue using the API. This is to account for edgecases such
as a Member going through the Stripe flow twice and ending up with
multiple subscriptions for multiple products
