Commit Graph

885 Commits

Author SHA1 Message Date
Nazar Gargol
4fe9d1536c Added error handling for failed member imports
no issue

- When bulk insert fails there is no transactional logic to revert
related records form being inserted. Also, previously there were no
attempts to "retry" the insert.
- To avoid complex retry logic, an iterative one-by-one insert retry
approach was taken. If this becomes a bottleneck in the future, the
retry algorithm could be improved.
- To avoid a lot of code duplication refactored model's `bulkAdd` & `bulkDestroy`
methods to use 'bulk-operations' module.
- Updated error handling and logging for bulk delete operations. It's very
unlikely for error to happen here,  but still need to make sure there is
a proper logging in place to trace back the failure.
- Added debug logs. This should improve debugging experience and
performance measurements.
- Added handling for unrecognized errors. Handling inspired by current unrecognized
error handling by ghost importer -10e5d5f3d4/core/server/data/importer/importers/data/base.js (L148-L154)
2021-07-22 01:53:21 +12:00
Nazar Gargol
4feaf49ca7 Improved error handling for batch deleted records
no issue

- Similar handling to one introduced in 31db3c86800b3268da5485417b16e0fcd8e6579a
- Having granular tracking for failed to remove id's would make it possible to return more specific errors to the client
2021-07-22 01:53:21 +12:00
Nazar Gargol
e32ce37a89 Improved error handling for batch inserted records
no issue

- Similar to 4f9e787119221ecb6b6e9703e020ea0e30a37b76 this adds error handling to batch operations done outside models
2021-07-22 01:53:21 +12:00
Nazar Gargol
b47c151855 Refactored bulk insert/delete operations into separate module
no issue

- Moved bulk db operations outside of importer module to create clearer separation of responsibilities
2021-07-22 01:53:21 +12:00
naz
29f9d60d3b Improved error handling for batch inserted member records (#12146)
no issue

- When batch insert fails handling should be more granular and aim to retry and insert as many records from the batch as possible.  
- Added retry logic for failed member's batch inserts. It's a sequential insert for each record in the batch. This implementation was chosen to keep it as simple as possible
- Added filtering of "toCreate" records when member fails to insert. We should not try inserting related members_labels/members_stripe_customers/members_stripe_customer_subscriptions records because they would definitely fail insertion without associated member record
2021-07-22 01:53:21 +12:00
Nazar Gargol
0756d26d22 Fixed handling for Stripe connected members import
no issue

- When stripe is disconnected and there are Stripe-connected records present in imported set they should not be processed and proper error should be thrown
2021-07-22 01:53:21 +12:00
Nazar Gargol
f928b68ee3 Fixed label serialization in members bulk importer
no issue

- After a refactor logic was missing trimming logic and handling for empty labels
2021-07-22 01:53:21 +12:00
Nazar Gargol
7dc70f5607 Fixed default subscribed value for member model in the importer
no issue

- The default value should not be a string but rather a boolean
2021-07-22 01:53:21 +12:00
Fabien 'egg' O'Carroll
0106138fd5 Updated bulk importer to improve performance (#12128)
no-issue

* Added bulkAdd method to Member,Customer&Subscription model
  This allows us to keep the db access in the model layer
* Updated @tryghost/members-api to 0.27.2
  This includes fixes for rate-limiting of requests, and exposes necessary
  Stripe methods for creating customers and complimentary subscriptions,
  without affecting the database.
* Refactored importer to parallelise tasks where possible
  By parallelising our tasks we are able to improve the speed at which the
  entire import completes.
2021-07-22 01:53:21 +12:00
Nazar Gargol
320cf58ff3 Fixed parameter naming for members importer 2021-07-22 01:53:21 +12:00
Nazar Gargol
ccb3068609 Update Stripe concurrency in members importer
no issue

- There were many failed import records due to rate-limit errors. With concurrency of 9 imports go through with 100% success
- Would need to verify these limits with live API to make the most of it
2021-07-22 01:53:21 +12:00
Nazar Gargol
a6ab9a6db2 Moved batching logic inside the members importer module
no issue

- This way importer is more self contained and controller logic doesn't have to know about batch sizes and other unecessary variables
2021-07-22 01:53:21 +12:00
Fabien 'egg' O'Carroll
39aab8ae28 Replaced all usage of member models with members-api (#12117)
no-issue

* Added stripeSubscriptions relation to member model

This allows us to fetch the subscriptions for a member via standard
model usage, e.g. `withRelated: ['stripeSubscriptions']` rather than
offloading to loops and `decorateWithSubscriptions` functions, this is
more performant and less non-standard than the existing method.

* Updated serialize methods to match existing format

The current usage of `decorateWithSubscriptions` and the usage of
members throughout the codebase has a subscriptions array on a stripe
object on the member, this ensures that when we serialize members to
JSON that we are using the same format.

There is definitely room to change this in future, but this is an
attempt to create as few breaking changes as possible.

* Installed @tryghost/members-api@0.26.0

This includes the required API changes so that everywhere can use
members-api directly rather than models and/or helper methods
2021-07-22 01:53:21 +12:00
Nazar Gargol
c5c57cbd85 Extracted batched member import into separate module
no issue

- The code in controller was becoming hard to reason about.
- Having a single module shows exactly how many dependencies are there to do an import for single batch.
- Having a separate module would make it easier to extract into it's own package in Members monorepo
2021-07-22 01:53:21 +12:00
Fabien O'Carroll
d0f587a6cc Published new versions
- @tryghost/members-api@1.22.1
2021-07-20 12:37:36 +01:00
Fabien O'Carroll
6693d470d0 Removed superfluous benefits relation fetch
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.*`.
2021-07-20 12:36:41 +01:00
Fabien O'Carroll
19ae16590d Published new versions
- @tryghost/members-ssr@1.0.8
2021-07-20 10:24:31 +01:00
Fabien O'Carroll
ae9bad957b Fixed _removeSessionCookie method use of cookies
refs https://github.com/TryGhost/Team/issues/908

The `cookies` module will unset a cookie if `null` or `undefined` is
passed as the value, or if the value is not passed. The previous call
was passing the options, which were being read as the value, and
resulting in `'[Object object]'` being stored as a cookie.

Explicitly passing `null` as the value makes this code correct and
easier to maintain.
2021-07-20 10:19:39 +01:00
Fabien O'Carroll
5fc0646d5e Published new versions
- @tryghost/members-api@1.22.0
2021-07-19 13:58:43 +01:00
Fabien O'Carroll
caf059cd7e Added WellKnownController and exposed jwks.json
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
2021-07-19 13:51:58 +01:00
Rishabh
97be7ca3db Published new versions
- @tryghost/members-api@1.21.0
2021-07-19 17:23:50 +05:30
Rishabh
069accdbe8 Fixed stripe migration to avoid complimentary prices as monthly/yearly price
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
2021-07-19 16:20:12 +05:30
Fabien O'Carroll
ceec3efe6a Published new versions
- @tryghost/magic-link@1.0.7
 - @tryghost/members-api@1.20.3
 - @tryghost/members-ssr@1.0.7
2021-07-15 18:19:11 +01:00
Fabien O'Carroll
ac3177e0bb Fixed require paths and top level js files
no-issue

We only include index.js and lib in npm packages.
2021-07-15 18:15:49 +01:00
Fabien O'Carroll
128ce5db9a Published new versions
- @tryghost/members-api@1.20.2
2021-07-15 18:08:17 +01:00
Fabien O'Carroll
15915a6040 Fixed index.js require
no-issue
2021-07-15 18:06:45 +01:00
Fabien O'Carroll
fe668113b1 Published new versions
- @tryghost/members-api@1.20.1
2021-07-15 18:03:59 +01:00
Fabien O'Carroll
02766afedd Moved MembersAPI.js into lib
no-issue

The previous published version was broken as we only include index.js
and the lib directory in the npm package
2021-07-15 18:01:53 +01:00
Fabien O'Carroll
d59051c104 Published new versions
- @tryghost/magic-link@1.0.6
 - @tryghost/members-api@1.20.0
 - @tryghost/members-csv@1.1.2
 - @tryghost/members-ssr@1.0.6
2021-07-15 16:34:02 +01:00
Fabien O'Carroll
d427e72b1c Fixed created_at dates for member event objects
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.
2021-07-15 15:20:16 +01:00
Rishabh
aad662267c Added migration to remove invalid subscriptions
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.
2021-07-14 20:12:39 +05:30
Rishabh
aa19008651 Fixed incorrect import path
no refs
2021-07-14 20:01:29 +05:30
Fabien O'Carroll
9102b7527f Moved MagicLink out of index.js
refs https://github.com/TryGhost/Team/issues/879
2021-07-14 14:17:38 +01:00
Fabien O'Carroll
3e1084905e Removed usage of raw Error class
refs https://github.com/TryGhost/Team/issues/879
2021-07-14 14:17:38 +01:00
Fabien O'Carroll
d51fdc3f4a Moved code out of index.js in directories
refs https://github.com/TryGhost/Team/issues/879
2021-07-14 14:17:38 +01:00
Fabien O'Carroll
13943e9746 Moved MembersAPI out of index.js
refs https://github.com/TryGhost/Team/issues/879
2021-07-14 14:17:38 +01:00
Fabien O'Carroll
004c7d2739 Moved MembersSSR out of index.js
refs https://github.com/TryGhost/Team/issues/879
2021-07-14 14:17:38 +01:00
Renovate Bot
223ee56647 Update Test & linting packages 2021-07-14 14:12:12 +01:00
Renovate Bot
4013642bb1 Update dependency mocha to v8.4.0 2021-07-14 11:35:50 +00:00
Fabien O'Carroll
4f660554eb Fixed linting issues for members-ssr example
refs https://github.com/TryGhost/Team/issues/879
2021-07-14 12:05:10 +01:00
Fabien O'Carroll
e39016423e Removed calls to console.log
refs https://github.com/TryGhost/Team/issues/879
2021-07-14 12:05:07 +01:00
Fabien O'Carroll
6083e4825f Removed trailing commas from .eslintrc.js
refs https://github.com/TryGhost/Team/issues/879
2021-07-14 12:04:46 +01:00
Fabien O'Carroll
8af799bdc9 Published new versions
- @tryghost/members-api@1.19.0
2021-07-06 13:47:38 +01:00
Fabien O'Carroll
dd8376dd90 Restricted Stripe Checkout to members without products
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.
2021-07-06 12:59:32 +01:00
Fabien O'Carroll
9841a33c83 Published new versions
- @tryghost/members-api@1.18.1
2021-07-06 11:54:12 +01:00
Fabien O'Carroll
12a3ae77cf Fixed creating members without products
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
2021-07-06 11:52:08 +01:00
Fabien O'Carroll
7545e67f2c Published new versions
- @tryghost/magic-link@1.0.5
 - @tryghost/members-api@1.18.0
 - @tryghost/members-csv@1.1.1
 - @tryghost/members-ssr@1.0.5
2021-07-06 11:18:01 +01:00
Fabien O'Carroll
631e78631f Updated linkSubscription to handle comped status
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.
2021-07-06 11:14:49 +01:00
Fabien O'Carroll
8d8d886705 Supported comped status for create/update methods
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.
2021-07-06 11:14:49 +01:00
Renovate Bot
7f71e28392 Update dependency nock to v13.1.0 2021-06-29 21:43:45 +01:00