Commit Graph

96 Commits

Author SHA1 Message Date
Naz
99560ca901 Refactored db dependency out of MembersCSVImporter
refs https://github.com/TryGhost/Team/issues/916

-  The refactor was done follow the DI Constructor pattern and prepare module for extraction
2021-07-22 01:53:21 +12:00
Naz
fdfd185835 Refactored jobs service out of MembersCSVImporter
refs https://github.com/TryGhost/Team/issues/916

-  The refactor was done follow the DI Constructor pattern and prepare module for extraction
2021-07-22 01:53:21 +12:00
Naz
6a8384cafb Refactored labs dependency out of MembersCSVImporter
refs https://github.com/TryGhost/Team/issues/916

-  The refactor was done follow the DI Constructor pattern and prepare module for extraction
2021-07-22 01:53:21 +12:00
Naz
c3b60a5628 Refactored Ghost mailer dependency out of MembersCSVImporter
refs https://github.com/TryGhost/Team/issues/916

-  The refactor was done follow the DI Constructor pattern and prepare module for extraction
2021-07-22 01:53:21 +12:00
Naz
8f169720d1 Removed dead code - batch-import module
refs https://github.com/TryGhost/Team/issues/916

- While investigating members importer related codebase this legacy module was spotted. It's not used anywhere and doesn't serve any particular purpose.
2021-07-22 01:53:21 +12:00
Fabien 'egg' O'Carroll
eca3ae1af3 Replaced usage of Error with @tryghost/errors (#13161)
refs 2f1123d6ca

Usage of the raw Error class has been deprecated in favour of our own
errors, which are more descriptive and have built in HTTP status codes.

This also updates the same errors to use @tryghost/tpl for the error
messages, which is the new pattern we are following in order for us to
deprecate the i18n module.
2021-07-22 01:53:21 +12:00
Hannah Wolfe
23748c92fb Moved labs utlity to shared
- This isn't really a "service" - it's a set of utilities for working with labs flags
- It's also required all over the place, and doesn't require anything that isn't shared
- Therefore, it should live in shared
2021-07-22 01:53:21 +12:00
Hannah Wolfe
e52fdbd737 Revert "Moved labs utlity to shared"
This reverts commit 693836322d96c5563671400f0346afa2e69276c4.
2021-07-22 01:53:21 +12:00
Hannah Wolfe
4550f68107 Moved labs utlity to shared
- This isn't really a "service" - it's a set of utilities for working with labs flags
- It's also required all over the place, and doesn't require anything that isn't shared
- Therefore, it should live in shared
2021-07-22 01:53:21 +12:00
Fabien O'Carroll
ee0cf47ce0 Added support for importing products column
refs https://github.com/TryGhost/Team/issues/765

This supercedes the `complimentary_plan` flag, as it is more precise
because it determines _which_ product(s) a member has access to. Because
of this, if the `products` column is present the `complimentary_plan`
column is not used.
2021-07-22 01:53:21 +12:00
Fabien O'Carroll
4141fd1e6a Linked comped members to default product for imports
refs https://github.com/TryGhost/Team/issues/765

As part of the multiple products feature, we're not longer using Stripe
subscriptions to denote Complimentary access, instead we're linking
members directly to products. Here we update the importer to follow
suit, so long as the flag is enabled.
2021-07-22 01:53:21 +12:00
Fabien O'Carroll
775b4a24f9 Moved MembersCSVImporter out of index.js file
no-issue

This cleans up the importer to match the standards of the rest of our
codebase.
2021-07-22 01:53:21 +12:00
Sam Lord
99bf0b29e2 Switch to @tryghost/debug, remove ghost-ignition
no issue
The only pieces of Ghost-Ignition used in Ghost were debug and
logging. Both of these modules have been superceded by the Framework
monorepo, and all usages of Ignition have now been removed, replaced
with @tryghost/debug and @tryghost/logging.
2021-07-22 01:53:21 +12:00
Sam Lord
ddeb980a9f Change to use @tryghost/logging
no issue

Logging is now controlled by a logginrc.js file in the root of the project - and now we can just import @tryghost/logging everywhere
2021-07-22 01:53:21 +12:00
Fabien O'Carroll
6a1d69c471 Made complimentary_plan & stripe_customer_id exclusive
no-issue

When importing Members it is possible to have both the
complimentary_plan and the stripe_customer_id columns set, this can
result in unusual outcomes, for example when importing a customer with a
zero-amount subscription, they would end up with two "comped"
subscriptions, and there would be two "comped" prices in the database.

As we are deprecating the use of "comped" in favour of creating a
subscription with a specific price, we're updating the import to prefer
`stripe_customer_id` column, only using the `complimentary_plan` column
when it is the only of the two columns passed.
2021-07-22 01:53:21 +12:00
Hannah Wolfe
a6d8f408b3 Moved i18n to shared
refs b51bba7b0e

- i18n is used everywhere but only requires shared or external packages, therefore it's a good candidate for living in shared
- this reduces invalid requires across frontend and server, and lets us use it everywhere until we come up with a better option
2021-07-22 01:53:21 +12:00
Hannah Wolfe
b8d64bfb28 Expanded requires of lib/common i18n and events
- Having these as destructured from the same package is hindering refactoring now
- Events should really only ever be used server-side
- i18n should be a shared module for now so it can be used everywhere until we figure out something better
- Having them seperate also allows us to lint them properly
2021-07-22 01:53:21 +12:00
Daniel Lockyer
119c013229 Updated bson-objectid calls to match API change
refs c873899e49

- as of `bson-objectid` v2.0.0, this library exports the function
  to generate an ObjectID directly, and then you need to use `.toHexString()`
  to get the 24 character hex string - 6696f27d82
- this commit removes all uses of `.generate()` and replaces with this
  change
2021-07-22 01:53:21 +12:00
Peter Zimon
ddedbd4225 Fixed member import email heading spacing 2021-07-22 01:53:21 +12:00
Peter Zimon
3956f93e17 New logo in Admin (#12768)
refs https://github.com/TryGhost/Team/issues/547

- replaced link to static/squircle to orb
2021-07-22 01:53:21 +12:00
Peter Zimon
251ebeaf30 Updated squircle ref to 4.0 repo 2021-07-22 01:53:21 +12:00
Fabien O'Carroll
58b8101781 Fixed Members importer usage of linkStripeCustomer
no-issue

The method signature was updated in the refactor and this was missed
2021-07-22 01:53:21 +12:00
Naz
bfdb6b95fe Extracted members controller's import method
refs #12537

- Moved code related to the importer into the MembersImporter class to  keep the controller code light
2021-07-22 01:53:21 +12:00
Peter Zimon
d12206a5f9 Import email refinements (#12473)
* Updated import email subject

* Updated import email title

* Fixed copy in import email to reflect if there was at least one member added
2021-07-22 01:53:21 +12:00
Fabien O'Carroll
085820bfa8 Added new members importer module
no-issue

This module encapsulates the work around performing imports, it
currently uses the concept of a "Job" which at the moment is not
persisted to the database, however when we want to look at resuming
imports after a server restart, this should give us the flexibility to
do it.
2021-07-22 01:53:21 +12:00
Fabien O'Carroll
0b82f2a9d6 Added email template for completion of background jobs
no-issue

This will be used to generate email content used for notifying users
that their import has been completed.
2021-07-22 01:53:21 +12:00
Fabien O'Carroll
c17c0ec763 Removed support for batched CSV importer
no-issue

We are rewriting the Members CSV importer to use background jobs, the
batched importer will no longer be used locally.
2021-07-22 01:53:21 +12:00
Scott Cabot
1d65c9367c Fixed user import with no created_at date breaking graph
- users imported from CSV with no created_at date where having their created_at date being stored as an int rather than a datetime.
- this was causing parsing issues with the graph so this commit fixes the formatting
2021-07-22 01:53:21 +12:00
Talha
574f2ca9c1 🐛 Fixed table constraint error when updating member's email with an already existing email (#12178)
closes #12045

- When member's email is updated to an already existing email of different member it caused table's unique constraint error, which was not handled properly. 
- Added handling for this error similar to one in members `add` method.
2021-07-22 01:53:21 +12:00
Nazar Gargol
8ea9896622 Refactored members imporeter in preparation for jobs
no issue

- This refactor extracts labels related code into a separate module for easier reuse by the "job-aware" batched importer
2021-07-22 01:53:21 +12:00
Nazar Gargol
8f1691b052 Improved import_label creation logic
no issue

- When an import was done and there were no "global labels" present Ghost created generic `import-[data]` label which later helped to find a specific batch of imported data
- It did not make sense to create such generic label when user provided their own unique label
- The rules that work now are:
1. When there is no global provided Ghost generates on and removes it in case there are no imported records
2. When there is a unique new global label provided no new label is generated, but the label stays even if there are no imported records
2021-07-22 01:53:21 +12:00
Nazar Gargol
3fa94b579d Removed date handling validation in members importer logic
no issue

- This is handled on input sanitization layer with date
format check in JSON schema validation, so there's no need to do this
check again in the importer.
2021-07-22 01:53:21 +12:00
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