Commit Graph

4361 Commits

Author SHA1 Message Date
Nazar Gargol
544382b651 Updated boolean values validations for members uploads
no issue

- When the value is part of `enum` the `null` value is no longer
recognized by `null` type in JSON schema validator.
2020-08-26 17:23:36 +12:00
Nazar Gargol
95044e3ba0 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)
2020-08-26 17:11:35 +12:00
Nazar Gargol
45bca2b104 Fixed linting for await statements
refs 9d65de4b10
2020-08-25 19:30:02 +12:00
Nazar Gargol
9d65de4b10 Removed created import labels when no records were imported
no issue

- When no members are succesfully imported through CSV import process the import label should not be created. Otherwise after multiple failed attempts to import there are orphaned labels in the system
2020-08-25 19:23:05 +12:00
Kevin Ansfield
e07e0a17c6 🐛 Fixed bookmark card display in member emails when using Gmail+Chrome
no issue

- updates `@tryghost/kg-default-cards` which contains two fixes
  - removes email-specific output being added to post html (had no visual impact due to use of conditional comments but keeps rendered html smaller+cleaner)
  - adds a background-url style to the thumbnail container to give two options for styling
- updates member email template styling to hide the `<img>` element in bookmark cards and use a background image instead to get consistent rendering across email clients
2020-08-24 18:21:09 +01:00
Peter Zimon
75f5ab637d Added minor member newsletter customisations via config
no issue
- adds a `members:emailTemplate` config object
  - `showSiteHeader` - defaults to `true`, shows the site title and icon in member emails
  - `showPoweredBy` - defaults to `false`, adds a "Publish with Ghost" button to member email footer
- updates member newsletter email template with hideable site header and "powered by" badge
2020-08-24 10:44:22 +01:00
Rish
297b63edce 🐛 Fixed response for members API update endpoint
closes https://github.com/TryGhost/members.js/issues/87

- The `update` method in members-api package was edited to return Model object instead of JSON directly [here](a28bcc5b2a)
- This caused the update member API on member endpoint to return partial response only as most properties couldn't be fetched
- Fix updates the middleware to correctly call `toJSON` before formatting response
2020-08-24 13:38:21 +05:30
Nazar Gargol
91d1417df2 Fixed email newsletter error when post has no content
closes #12158
2020-08-24 19:46:46 +12:00
Daniel Lockyer
0ab190e83c Merged 3.30.2 into master
* 3.30.2:
  Bumped @tryghost/members-api to 0.28.1 in lockfile
  Bumbed @tryghost/members-api to 0.28.1
  🐛 Fixed unable to delete member when stripe is connected
2020-08-21 12:06:01 +01:00
Rish
87e8298ca7 🐛 Fixed unable to delete member when stripe is connected
closes https://github.com/TryGhost/Ghost/issues/12150

- The `destroy` method on members-api method was called incorrectly with not enough params
- It expects both `data` and `options` passed down [here](https://github.com/TryGhost/Members/blob/master/packages/members-api/lib/users.js#L14)
- Missing `options` in method call throws error as we read `cancelStripeSubscriptions` property on options if stripe is connected
- Fix passes both data and options to the destroy method
2020-08-21 11:13:07 +01:00
Rishabh Garg
c7f3837c54
Added admin endpoint for editing member subscription (#12145)
refs #12127

- Adds new `editSubscription` endpoint for members admin API which allows updating individual subscription for a member - `PUT /members/:id/subscriptions/:subscription_id/`
- `editSubscription` has same permissions as member's `edit` endpoint
- Currently allows toggling of cancellation at period end for an active subscription
2020-08-20 17:28:11 +05:30
Nazar Gargol
5670d99f46 Improved error handling for batch deleted records
no issue

- Similar handling to one introduced in 8418c829de
- Having granular tracking for failed to remove id's would make it possible to return more specific errors to the client
2020-08-20 19:11:41 +12:00
Nazar Gargol
8418c829de Improved error handling for batch inserted records
no issue

- Similar to 3a594ce22e this adds error handling to batch operations done outside models
2020-08-20 19:01:17 +12:00
Nazar Gargol
f8adb4a672 Refactored bulk insert/delete operations into separate module
no issue

- Moved bulk db operations outside of importer module to create clearer separation of responsibilities
2020-08-20 18:42:38 +12:00
naz
3a594ce22e
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
2020-08-20 17:41:47 +12:00
Nazar Gargol
2e769e3122 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
2020-08-20 17:08:19 +12:00
Nazar Gargol
4498b4624a Fixed label serialization in members bulk importer
no issue

- After a refactor logic was missing trimming logic and handling for empty labels
2020-08-20 16:43:42 +12:00
Nazar Gargol
32fd913ac3 Fixed default subscribed value for member model in the importer
no issue

- The default value should not be a string but rather a boolean
2020-08-19 23:42:48 +12:00
Kevin Ansfield
d8e319af88 🐛 Fixed card spacing and caption styling for member emails in Outlook
closes https://github.com/TryGhost/Ghost/issues/12139

- once the email content has been rendered in the post serializer, perform some whole-content transformation of `figure` and `figcaption` to `div` using cheerio
- juiced will have already inlined the elements styles so there's no need to adjust the template's stylesheet
2020-08-19 12:34:14 +01:00
Nazar Gargol
616767d0ec Fixed members bulk import's createdBy field assignment
no issue

- Method signature has been changed with cada88975a, the call needed an update as well
2020-08-19 17:14:45 +12:00
Jeremy Davidson
681c96b5e9
🐛 Fixed bookmark card author/publisher spacing in member emails (#12134)
closes #12078

- Root cause was that pseudo class .kg-bookmark-author:after was not getting inlined to email newsletter or its preview.
  - That is where the margin and bullet-point content are added between author and publisher.
- Dependency juice supports an option inlinePseudoElements which is false by default.
- Fix was to set inlinePseudoElements to true when serializing post email.

Co-authored-by: Jeremy Davidson <jeremy@crossingcontour.com>
2020-08-18 16:18:44 +01:00
Hannah Wolfe
9f291b2d10 Improved testmode logging of shutdown status
- testmode allows us to test Ghost's behaviour with long running requests and jobs
- moved all the testmode logging to only happen if Ghost starts successfully to make it clearer what is happening
- added a (currently very dirty) bit of code to fetch the jobService and output the queue status giving us a similar
     output for both open connections and jobs
- all of this allows for debugging exactly what Ghost is doing whilst it is processing a shutdown request
2020-08-18 15:48:42 +01:00
Kevin Ansfield
cefcdad090 🐛 Fixed emails appearing very wide in Outlook and improved email image sizes
closes https://github.com/TryGhost/Ghost/issues/11536

- bumps `@tryghost/kg-default-cards`
  - image and gallery cards now output `width/height` attributes on `img` elements with a max width of 600px
  - uses resized images where possible to keep email weight down
- adds `height: auto` style to image card images so that the `height` attribute does not cause distortion at smaller screen widths
2020-08-18 15:33:54 +01:00
Fabien 'egg' O'Carroll
cada88975a
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.
2020-08-18 13:39:45 +01:00
Rishabh Garg
2bac2c67cc
Granted Admin users ability to impersonate member (#12132)
refs #12126

- Adds migration to add impersonation permission to administrators
- Adds default permission fixture to allow administrators to read member impersonation urls
- Allows administrators to create member impersonation magic links
2020-08-18 14:17:56 +05:30
Nazar Gargol
6ca51eae29 Fixed member import csv validator for labels
no issue

- Labels can have a null value when there is none defined in the imported file. Such records fails for "string" type in JSON schema
2020-08-18 15:25:04 +12:00
Kevin Ansfield
752c1022f9 🐛 Fixed failing migration from <2.34 to 3.29
closes https://github.com/TryGhost/Ghost/issues/12129

Migrations were failing when upgrading from a version that didn't include stripe member subscriptions table. The `up` migration was checking for the existence of a unique index that was added to the schema in 3.29 but it was using the wrong variable name which meant that it would never return true for an existing index. For most migrations this worked because the index existed but when through 2.34 the table was created from scratch and did included the index at that point.

- fixed variable name and re-ordered variable assignment for better code locality that would have made the typo more visible
2020-08-17 18:31:00 +01:00
naz
51c9a50c4f
Added validation layer to members import endpoint (#12131)
no issue

- Additional validation is needed for imported data because in case of bulk insertions (through knex) we bypass model layer validation - this could lead to invalid data in the database, which would be hard to fix.
- Chose validation method we use for other endpoints - through JSON Schema. It proved to be very performant (200ms overhead for 50k records). When comparing it with iterative method (validating each record separately) this was adding about 17s of overhead.
- Refactored returned values from "sanitizeInput" method to encapsulate more logic so that the caller doesn't have to calculate amount of invalid records and deal with error types
- Whole sanitizeInput method could now be easily extracted into separate module (somewhere close to members importer)
- Bumped members-csv package. It is meant to handle empty string values - '' and null, which should allow validating member records more consistently!
2020-08-17 23:28:57 +12:00
Daniel Lockyer
046b5540fe Removed workaround for intermittent ghost-server logging
- upon further testing, this started to not work again so I'm removing
  this horrible hack
- it could be Bunyan playing around with us, and the server is actually
  shutting down
2020-08-13 18:45:51 +01:00
Daniel Lockyer
d4f1274ba5 Returned empty promises in ghost-server
- I have no idea why this is required and I really don't like it
- this change looks superfluous but I had problems with Ghost not
  displaying shutting down messages properly and this is the only
  thing that got it working
- the async-awaits were getting stuck in `await this._cleanup()`,
  and then wouldn't enter into the `finally` block
- oddly, this was only reproducible when a job was running, it would
  shut down normally otherwise
- if I find the solution in the mean time, I'll happily get rid of this
2020-08-13 17:26:33 +01:00
Nazar Gargol
ef41c57974 Added test coverage for invalid member import emails
no issue

- This test should serve as a control for new batched importer
2020-08-13 22:14:52 +12:00
Nazar Gargol
a1e6b8f898 Updated members import sanitization
no issue

- Updated sanitization logic to be self contained and return sanitieze input along with error stats
- This should give a nice place for validations to fit in
2020-08-13 22:12:20 +12:00
Nazar Gargol
4126187df3 Fixed parameter naming for members importer 2020-08-13 21:08:34 +12:00
Nazar Gargol
d2fbe327e4 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
2020-08-13 21:04:57 +12:00
Nazar Gargol
78b4dff656 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
2020-08-13 20:31:11 +12:00
Kevin Ansfield
c434666ba2 🐛 Removed [http://url/] output in member email preview text
no issue

- we output the post excerpt in a hidden div in the email template so that email clients pick it up as the "preview" text when listing emails
- when no custom excerpt is provided the preview text is grabbed from post.excerpt which is the first 500 chars of the post.plaintext value
- post.plaintext formats links as "Link [http://url/]" which is unwanted in html email previews

- add a basic replacement to the post email serializer to remove any `[http://url/]` occurrences from the post excerpt before rendering the email content
2020-08-12 20:14:06 +01:00
Kevin Ansfield
163092f377 🐛 Fixed apostrophes not displaying correctly in Outlook for member emails
refs https://github.com/TryGhost/Ghost/issues/11536

- Outlook supports `&#39;` as a special char for apostrophes but not `&#apos;` which is what cheerio/juiced render
- adds a basic string placement to the email serializer to switch to the older style of special char
2020-08-12 20:01:42 +01:00
Kevin Ansfield
c7ff4c9e93 Moved email sending to the background job queue
no issue

- moves the meat of `pendingEmailHandler()` code into a new function `sendEmailJob()` that is passed over to the new job service
- lets the server keep processing email generation and sending when it receives a shutdown request rather than halting processing mid-send and ending up in a partial state
2020-08-12 17:02:14 +01:00
Fabien 'egg' O'Carroll
1294e3f92c
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
2020-08-12 14:17:44 +01:00
Nazar Gargol
c696d715c1 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
2020-08-12 20:18:30 +12:00
Nazar Gargol
7d3f6e32ca Addes sort order support for imported members
no issue

- Member's labels have to have sort_order assigned when added/edited. This was lacking from batched importer.
- Implementation is based on logic used in model's base - e484709e73/core/server/models/base/index.js (L81-L86)
2020-08-12 16:34:15 +12:00
Hannah Wolfe
bbbd011074 Added job-manager & wired up shutdown and testmode
- Bottom line - we need to manage shutting down gracefully when doing long-running tasks
- To achieve that, we're going to use job queues

In this commit:
- added new @tryghost/job-manager dependency
- added a minimal job service, that handles in passing things like logging and (maybe later) config
- job service is wired up to server shutdown, so that the queue finishes before the server exits
- also added a new job endpoint to testmode so that it's easy to test job behaviour without needing to do real work
2020-08-11 21:31:34 +01:00
Hannah Wolfe
624206b6d7 Improved ghost-server with async/await
- Using consistent patterns for shutdown and stop
- Make it clear what each method does
- Use async/await to make the code more readable and simple
- This lays groundwork for having more cleanup tasks in stop than just server.stop()
2020-08-11 19:42:37 +01:00
Daniel Lockyer
5b471e1bbe Extracted promise libs and history into @tryghost/promise
- deleted files under `core/server/lib/promise` and related test files
- added `@tryghost/promise` as a dependency
- fixed all local requires to point to the new package
2020-08-11 18:44:21 +01:00
Daniel Lockyer
c9a5b28669 Extracted core/server/lib/security to @tryghost/security package
- code and tests were extracted out to this package
- deletes these files
- replaces all local requires, and adds it as a dependency
2020-08-11 14:06:50 +01:00
Daniel Lockyer
8799feb801 Replaced constants file with @tryghost/constants
- extracted constants file into a new package
- replaced all local requires of the file with new package
2020-08-11 12:51:16 +01:00
Hannah Wolfe
e84621d6ef 🐛 Readded missing server.start event
closes #12118

- server.start was mistakenly removed in 71f02d25e9
- it is used for loading themes (and other things) and is critical
- added tests to prevent this regressing again in future
2020-08-11 10:09:24 +01:00
Nazar Gargol
8a7e00c413 Enabled batched members import method through enableDeveloperExperiments flag
no issue

- Allows early testing of batched import method
2020-08-11 18:52:37 +12:00
Nazar Gargol
bbcc0f5178 Added batched members import API method
no issue

- New Member API batched import is meant to be a substitution to current import
with improved performance while keeping same behaviore. Current
import processes 1 record at a time using internal API calls and times
out consistently when large number of members has to be imported (~10k
records without Stripe).
- New import's aim is to improve performance and process >50K
records without timing out both with and without Stripe connected
members
- Batched import can be conceptually devided into 3 stages which have
their own ways to improve performance:
  1. labels - can be at current performance as number of
labels is usually small, but could also be improved through batching
  2. member records + member<->labels relations - these could
be performed as batched inserts into the database
  3. Stripe connections - most challanging bottleneck to solve because
API request are slow by it's nature and have to deal with rate limits of
Stripe's API itself
- It's a heavy WIP, with lots of known pitfalls which are marked with
TODOs. Will be solved iteratively through time untill the method can be
declared stable
- The new batched import method will be hidden behind 'enableDeveloperExperiments' flag to
allow early testing
2020-08-11 18:31:31 +12:00
Hannah Wolfe
b0512f2c25 Added a server testmode to help test behaviour
- A simple way to test behaviours without having to do complex interactions to e.g. generate errors or slow requests
- Makes it easier to test the new shutdown behaviour, among other things
2020-08-10 16:38:49 +01:00