Commit Graph

1048 Commits

Author SHA1 Message Date
naz
8ddf83f3c5
Fixed "no-shadow" linting error in server modules (#12287)
refs 143921948d

- Continuation of changes started in referenced commit
2020-10-20 12:02:56 +13:00
Kevin Ansfield
16bfb3fa41 Added basic CRUD admin API for snippets
no issue

- standard browse/read/add/edit/destroy API endpoints for snippets resource
- updates `@tryghost/admin-api-schema` dependency to version that includes snippet definition and schemas
2020-10-16 18:02:58 +01:00
naz
392140cb36
Allowed for comped field when creating a member through Members API (#12278)
closes #12273

- `comped` field has been allowed when editing a member or importing from a CSV. There has been a usecase (Zapier Integration) for API client to create a member with "Complimentary" plan, which made this change necessary
- Previously the logic for comped field was to skip and continue member record creation if Stripe was not connected. Now we throw an error - same as the one we have been throwing before when stripe_customer_id field was passed in. The implication of this change is that we won't be creating any record now if comped === true and Stripe is disabled. 
- Bumped admin-api-schema-package. Contains `comped` schema change so this field gets passed through to controller
2020-10-14 13:24:09 +13:00
Kevin Ansfield
8ad384d7f8 🐛 Fixed scheduled post emails pointing at /404/ for the "view online" link
no issue

When scheduling a post to publish+send the "view online" link was pointing at https://site.com/404/ rather than the published post's url.

The problem occurred because the `/schedules/` endpoint wraps it's post read+edit calls in a transaction. Context:
- when a post is published with with the "send email" option the email record is immediately generated and added to the API response, as part of the email record generation we render the email content including fetching the url for the "view online" link
- urls for all resources are handled by our `url` service, that service updates it's internal cache based upon model events such as the "edited" event triggered when a post is published
- if the posts API controller is given a transaction, the email record is also generated inside of that transaction however at this point the `url` service will not have been updated because the post record hasn't been committed meaning it has no available url for the post

Fix:
- removed the `models.Base.transaction()` wrapper around the post read+update in the `/schedules/` API controllers
- we don't need a transaction here. It was added as protection against another write request coming in between the `/schedules/` controller reading a post and publishing a post but we already have protection against that in the form of collision detection - if a write request comes in and commits between the schedules controller reading the post and updating it, the scheduler's update call will fail with a collision error at which point the scheduler itself should retry the request which could then publish the post successfully if everything else is in order
2020-10-08 10:26:49 +01:00
Kevin Ansfield
9cbeb74db0
🐛 Fixed broken embeds cards when pasting links to Wordpress sites (#12262)
closes https://github.com/TryGhost/Ghost/issues/12260

- if a card type was not explicitly chosen (i.e. a url was pasted into the editor) then abort fetching the oembed endpoint if we detect it's a `wp-json` oembed and return a bookmark card payload instead
- cleaned up an unused argument in the internal `fetchBookmarkData()` method
2020-10-06 08:44:03 +01:00
Naz
39e403d176 🐛 Fixed meta attributes calculation on post preview
closes https://github.com/TryGhost/Ghost/issues/12247

- Internal preview controller was lacking "mapping" call to post object which handled not only missing meta attribute information but lots of other mappings (e.g. users, tags, etc.)
- Have added a regression test to catch issues like this in the future
2020-10-05 17:52:40 +13:00
Rish
58fda5bad2 Allowed accent color setting for site with portal flag
no issue

- We had previously allowed accent_color setting for member site settings behind portal flag, but Ghost Admin also needs the public site setting with accent color to correctly reflect the accent color when flag is switched on
- Removes deletion of accent color setting when behind the Portal flag OR dev experiment flag
2020-10-02 15:30:54 +05:30
Fabien 'egg' O'Carroll
c19d282a51
Ensured Admin API members resource only returns known fields (#12240)
refs #12055

As part of the work in TryGhost/Members#206 we load the stripeCustomers relation on the member model, and we do not want this to be part of the API response. The changes here include a refactor but the main thing is that the serialized object is explicit and does not include unexpected or unknown fields.

* Moved mapMember out of mapper file

This cleans up the serializer a bit by keeping it's functionality all in
one place, rather than a shared mapper file

* Refactored members controller to return models

Previously the controller was calling toJSON, which is serialization,
this updates the controller to only deal with models, leaving all of the
serialization to the serializer!

* Refactored members serializer

This adds typings to all of the methods/functions in the serializer, as
well as making the serializating explicit, rather than returning the
result of toJSON, we explicitly set the properties we expect to be on
the output object. This protects us against accidental API changes in
the future.
2020-09-30 10:22:22 +01:00
naz
071bf8edb5
Extracted Admin API JSON schemas into @tryghost/admin-api-schema (#12237)
closes https://github.com/TryGhost/Ghost/issues/10628

- JSON Schemas were extracted into a separate module to allow other clients to reuse them (for example documentation). Having them in a separate package also slims down the amount of code needed to be maintained in the core.
- Updated canary API input validators to use admin-api-schema module
- Removed canary schemas that moved into admin-api-schema package
- Updated v2 API input validators to use admin-api-schema package
- Removed v2 schemas that moved into admin-api-schema package
- Updated tests to contain needed information in apiConfig to pick up correct validation
- Added @tryghost/admin-api-schema package dependency
2020-09-30 16:25:12 +13:00
Kevin Ansfield
932def7e6d Fixed replacement strings showing when viewing sent emails
refs https://github.com/TryGhost/Ghost/pull/12232

When viewing sent emails in Ghost's admin area, it displays the `html` field directly from the `email` relation loaded with the post. Since the `mega` refactor we now store raw content in that field rather than sanitized "preview" content so it's necessary to modify the API output to match the old behaviour.

- use the API output serializers to parse replacements in email content and replace with the desired fallback or empty string
2020-09-29 17:39:22 +01:00
Kevin Ansfield
474e6c4c45 Refactor mega service to use stored email content and batch/recipient records
no issue

- store raw content in email record
  - keep any replacement strings in the html/plaintext content so that it can be used when sending email rather than needing to re-serialize the post content which may have changed
- split post email serializer into separate serialization and replacement parsing functions
  - serialization now returns any email content that is derived from the post content (subject/html/plaintext) rather than post content plus replacements
  - `parseReplacements` has been split out so that it can be run against email content rather than a post, this allows mega and the email preview service to work with the stored email content
- move mailgun-specific functionality into the mailgun provider
  - previously mailgun-specific behaviour was spread across the post email serializer, mega, and bulk-email service
  - the per-batch `send` functionality was moved from the `bulk-email` service to the mailgun provider and updated to take email content, recipient info, and replacement info so that all mailgun-specific headers and replacement formatting can be handled in one place
  - exposes the `BATCH_SIZE` constant because batch sizes are limited to what the provider allows
- `bulk-email` service split into three methods
  - `send` responsible for taking email content and recipients, parsing replacement info from the email content and using that to collate a recipient data object, and finally coordinating the send via the mailgun provider. Usable directly for use-cases such as test emails
  - `processEmail` takes an email ID, loads it and coordinates sending related batches concurrently
  - `processEmailBatch` takes an email_batch ID, loads it along with associated email_recipient records and passes the data through to the `send` function, updating the batch status as it's processed
  - `processEmail` and `processEmailBatch` take IDs rather than objects ready for future use by job-queues, it's best to keep job parameters as minimal as possible
- refactored `mega` service
  - modified `getEmailData` to collate email content (from/reply-to/subject/html/plaintext) rather than being responsible for dealing with replacements and mailgun-specific replacement formats
    - used for generating email content before storing in the email table, and when sending test emails
    - from/reply-to calculation moved out of the post-email-serializer into mega and extracted into separate functions used by `getEmailData`
  - `sendTestEmail` updated to generate `EmailRecipient`-like objects for each email address so that appropriate data can be supplied to the updated `bulk-email.send` method
  - `sendEmailJob` updated to create `email_batches` and associated `email_recipients` records then hand over processing to the `bulk-email` service
  - member row fetching extracted into a separate function and used by `createEmailBatches`
  - moved updating of email status from `mega` to the `bulk-email` service, keeps concept of Successful/FailedBatch internal to the `bulk-email` service
2020-09-29 17:17:54 +01:00
Fabien 'egg' O'Carroll
854a41e556
🐛 Fixed CSV import json-schema email validation (#12239)
no-issue

By using the "email" validation, we were validating emails in CSV
imports using a different validator to the rest of the API. AJV's built
in email validation was failing on emails with "special" characters,
such as letters with an umlaut above them.

This commit brings the validation for CSV imports in line with the rest
of the API.
2020-09-29 12:51:35 +01:00
Rish
03968219a7 🐛 Fixed email verification mails not sent
no issue

- Email ownership verification emails for support/from address was not using the updated magic link service syntax
2020-09-24 23:39:25 +05:30
Naz
43153ba31e Disabled changing webhook's integration_id through PUT API
refs #12033

- Allowing to change parent integration opens up possible security holes and has no clear usecase at the moment. After a webhook record is created it should not be possible to change parent integration.
- Had do partially duplicate JSON schema definition from webhooks definition as there is no proper composition technique available in current version of JSON Schema.
2020-09-24 16:24:02 +12:00
Naz
6f1abc610a Added check for parent integration_id when creating a webhook
refs https://github.com/TryGhost/Ghost/issues/12033
refs https://github.com/TryGhost/Ghost/issues/10567

- Creating a webhook without valid parent integration leads to orphaned webhook records, which shoult not ever happen
- This scenario is only possible for non-integration authentication,
because in case of integration being authenticated it's id is
automatically assigned to creatd webhook
2020-09-24 15:09:51 +12:00
Fabien O'Carroll
171908cefa Added support for filtering Members CSV exports
no-issue

Adding these options to the endpoint will pass them to the model layer,
allowing us to filter members CSV exports
2020-09-23 14:29:15 +01:00
Fabien O'Carroll
fe1e480e2e Updated getMagicLink call to handle Promise return
no-issue

refs: https://github.com/TryGhost/Members/commit/63942f03

The above commit updated all magic-link methods to be async, this change
ensures that we will handle a Promise return when it's updated in Ghost.
`await` has no effect on non Promise return values so it's safe to add
this now.
2020-09-18 10:49:18 +01:00
Rish
7a3839fb14 Added new portal config flag
no issue

- The new Portal config flag allows switching on Portal conditionally with config
- The dev experiment flag still works for enabling Portal
- The flag currently defaults to `false` as Portal is still a beta feature and switched off by default
- We expose it on the admin api config endpoint so that the Ghost-Admin client can use it to conditionally render Portal settings
2020-09-17 19:40:34 +05:30
naz
cbdc91ce48
Added Location header to API's POST request responses (#12186)
refs #2635

- Adds 'Location' header to endpoints which create new resources and have corresponding `GET` endpoint as speced in JSON API - https://jsonapi.org/format/#crud-creating-responses-201. Specifically:
    /posts/
    /pages/
    /integrations/
    /tags/
    /members/
    /labels/
    /notifications/
    /invites/

- Adding the header should allow for better resource discoverability and improved logging readability
- Added `url` property to the frame constructor. Data in `url` should give enough information  to later build up the `Location` header URL for created resource.
- Added Location header to headers handler. The Location value is built up from a combination of request URL and the id that is present in the response for the resource. The header is automatically added to requests coming to `add` controller methods which return `id` property in the frame result
- Excluded Webhooks API  as there is no "GET" endpoint available to fetch the resource
2020-09-14 22:33:37 +12:00
Talha
afa976066a
🐛 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.
2020-09-10 16:03:57 +12:00
Nazar Gargol
5582d030e3 Added routes.yaml content checksum storage to the db
closes #11999

- When the routes.yaml file changes (manually or through API) we need
to store a checksum to be able to optimize routes reloads in the future
- Added mechanism to detect differences between stored and current routes.yaml hash value
- Added routes.yaml sync on server boot
- Added routes.yaml handling in controllers
- Added routes hash synchronization method in core settings. It lives in core settings
as it needs access to model layer. To avoid coupling with the frontend settings it accepts
a function which has to resolve to a routes hash
- Added note about settings validation side-effect. It mutates input!
- Added async check for currently loaded routes hash
- Extended frontend settings loader with async loader. The default behavior of the loader is
to load settings syncronously for reasons spelled in 0ac19dcf84
To avoid blocking the eventloop added async loading method
- Refactored frontend setting loader for reusability of  settings file path
- Added integrity check test for routes.yaml file
2020-09-10 10:54:57 +12:00
Nazar Gargol
4606c93e4f Refactored headers function to use async/await
no issue

- The async/await syntax makes it easier to reason about the code. Because adding 'Location' header is in the works it's a prep-work in a sense
2020-09-07 15:36:06 +12:00
Rish
cbb3664bdd Added handling for new members support/reply email addresses
no issue

- Updated magic link generation and validation methods for email update API to handle new support address
- Updated importer to ignore the new support address as it can only be updated via verification
- Updated members service to listen on settings edit for new support/reply address fields as well
- Updated tests to include the new settings
2020-08-31 18:09:38 +05:30
Nazar Gargol
eb8c0fcff9 🐛 Fixed Tags API v2 to return posts count
closes #12167

- Tags API v2 was ignoring `count.posts` include parameter.
- Regression was introduced with a3f693b472
- Introduced regression tests across all Content API versions to avoid similar bug in the future
2020-08-31 18:46:35 +12:00
Nazar Gargol
e8248cdc9e 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
2020-08-27 21:47:00 +12:00
Nazar Gargol
262d33b73c 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
2020-08-27 19:07:27 +12:00
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
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
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
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
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
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
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
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
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
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
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
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
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
Nazar Gargol
60ae9e82f9 Fixed integration_id assignment for webhook when creating through API key auth
refs 173e3292fa

- The bug was initially introduced in referenced commit. When request is done with `api_key` context, there should always be an `integration` object associated with it - 71c17539d8/core/server/services/permissions/parse-context.js (L36) . An `id` from `context.integration` not `context.api_key` has to be assigned to newly created webhook!
- The webhooks API is about to be declared stable in upcoming release, so no migration will be done
2020-08-04 16:43:24 +12:00
Nazar Gargol
1b449f4f53 🐛 Fixed 500 error in webhooks API when modifying non-existing webhooks
closes #12064

- Handled permission check bug by returning 404, same way it is returned in other permissions related places when handling non-existing resource. Example - 60907a7ae4/core/server/models/relations/authors.js (L355-L358)
2020-08-03 23:08:47 +12:00