Commit Graph

25 Commits

Author SHA1 Message Date
Simon Backx
6e72767a50
Fixed verification trigger not working for large imports (#15887)
fixes https://github.com/TryGhost/Team/issues/2326

When importing more than 500 members, we didn't testImportThreshold at
the right time. It was called too early because the importing job was
not awaited. This also adds an E2E test for this case.
2022-11-28 18:22:10 +01:00
Naz
cc72e8f599
🐛 Fixed complimentary_plan Member imports
closes https://github.com/TryGhost/Team/issues/2219

- The CSV importer was failing when a "complimentary_plan" flag was present with a "true" value. The root of the issue was the data model change where the "id" of the Tier object is no longer a String but an ObjectID instance. It's a slight departure from previous bookshelf object behavior where 'id' property is always a string that is a stringified ObjectID.
- In the future we should unify the logic across all data access objects to either keep the convention of using a String under id property or switch to ObjectId instances.
2022-11-08 11:47:00 +07:00
Naz
7fe9e06c4d
Fixed rule for import background job detection
refs https://github.com/TryGhost/Team/issues/1076

- The members CSV import would go into background job (longer running and resulting with an email) when it contained a complimentary_plan column.
- With recent members codebase decoupling Ghost does not make any connection to Stripe as "complimentary plan" data is saved purely in native data structures. Making no need for a background job for complimentary plans
2022-10-31 16:31:55 +08:00
Naz
4d26c50e0b
Fixed data type of hasStripeData in member importer
refs https://github.com/TryGhost/Team/issues/1076

- What appeared to be a "boolean" by nature and name, the hasStripeData was holding a result of "find" method - and object or an undefined value
- Fixed the typing, to avoid ambiguity in the future
2022-10-31 16:31:35 +08:00
Naz
a7f5ee0ad5
Simplified members CSV importer constructor
refs https://github.com/TryGhost/Team/issues/2077

- Passing in the whole "getMembersApi" is just too much state to know about for the importer - it only uses a concept of default tier and members repository, the rest is distracting fluff making it hard to reason about what the importer **has to** know to function
- Passing in two functions breaking up the above state simplifies the constructor API.
- This is also a groundwork before substituting productsRepository for tiersRepository (refed issue objective)
2022-10-25 16:40:28 +08:00
Naz
59209a07a5
Cleaned up leftover "product" variable naming
refs https://github.com/TryGhost/Team/issues/1076

- "product" in the context of members has been deprecated since introduction of "tiers"
2022-10-24 18:06:02 +08:00
Naz
840deaf8d7
Restricted members importer to ignore "products" column
refs https://github.com/TryGhost/Team/issues/1076
refs 70229e4fd3 (diff-b67ecda91b5bd79c598e5c5a9ec2ccf28dbfab6a924b21352273865e07cd7ceaR57)

- The "products" column has not been doing any logic anything since at least 5.20.0 (see refed commit). The concept of columns in the export file was mostly there for analytical/data filtering reasons - so the user could analyze their exports. CSV was never a good suite for relational data that "products" (or now tiers) represent
- The "tiers" column will still be present in the exported CSV file, but there is not going to be any logic attached to it.
- The only columns that can effect the "tiers" state of the member are: "complimentary_plan" (assign default tier to the member) and "stripe_customer_id" (pulls in subscription/tier data from Stripe)
2022-10-24 18:06:02 +08:00
Naz
5d5b77d32f
Fixed comped tier assignment
closes https://github.com/TryGhost/Team/issues/1869

- When there were "archived" tiers in the system the importer incorrectly fetched them instead of only taking "active" ones into account. The "getDefaultProduct" on product repository does exactly that.
- Additionally, reusing the "getDefaultProduct" makes testing the importer slightly less complex.
2022-10-20 17:19:52 +08:00
Naz
b589a66cd4
Fixed broken CSV importer tests
refs 90768e9985

- With introduction of strict field mapping the regression test testing for "imports of not mapped fields" failed.
2022-10-19 18:33:47 +08:00
Naz
90768e9985
Added strict field mapping to member CSV importer
closes https://github.com/TryGhost/Toolbox/issues/430

- The members importer used to  import all fields present in the uploaded CSV if the headers match, even if they're not mapped in the UI. This behavior has lead to have misleading consequences and "hidden" features. For example, if the field was present but intentionally left as "Not imported" in the UI the field would still get imported.
- Having a strict list of supported import fields also allows for manageable long-term maintenance of the CSV Import API and detect/communicate changes when they happen.
- The list of the current default field mapping is:

    email: 'email',
    name: 'name',
    note: 'note',
    subscribed_to_emails: 'subscribed',
    created_at: 'created_at',
    complimentary_plan: 'complimentary_plan',
    stripe_customer_id: 'stripe_customer_id',
    labels: 'labels',
    products: 'products'
2022-10-19 18:10:40 +08:00
Naz
f0b68846cc
Moved header mapping configuration to importer
refs https://github.com/TryGhost/Toolbox/issues/430

- To be able to introduce strict mapping rules (exclude unknown fields) we need to control the CSV header mapping on the importer level. This change moves the configuration up from CSV parser to the importer
- Also adds tests covering correct inserts for specially treated "subscribed_to_emails" field
2022-10-19 18:10:40 +08:00
Naz
c08d4f8ad1
Added basic test coverage for perform method
refs https://github.com/TryGhost/Toolbox/issues/430

- "perform()" is what gets executed by the import job for both immediate import and "inline job" import. Testing it on granular level will allow to change it with more confidence when introducing strict field mapping rules
2022-10-19 18:10:40 +08:00
Naz
8dc630b3a0
Removed cleaned up use of "Job" object
refs https://github.com/TryGhost/Toolbox/issues/430

- Importer code was filled with an unnecessarily complex "job" object that was passed around. It had an "id" property, which confusingly was a path to a file at all times.
- Simplified the logic significantly by keeping and passing around the path to a "prepared" members CSV.
2022-10-19 18:10:40 +08:00
Naz
d165a0017c
Extracted MembersCSVImporter to a builder method
refs https://github.com/TryGhost/Toolbox/issues/430
refs https://github.com/TryGhost/Ghost/issues/14882

- The MembersCSVImporter constructor is way to complex and needs refactoring. This complexity makes initialization in tests too bulky and makes tests hard to read.
- Having a builder method is a stopgap solution to avoid going into MembersCSVImporter refactoring too deep.
2022-10-19 18:10:40 +08:00
Rishabh Garg
c16abbf085
🐛 Fixed empty error csv file for member imports (#15274)
closes https://github.com/TryGhost/Team/issues/1828

- members importer was sending empty error files in case of invalid rows in CSV, hiding both error and affected rows
- fixes typo in `content` option(was `contents` before) passed to attachment (ref - https://nodemailer.com/message/attachments )
2022-08-24 00:49:30 +05:30
Aileen Nowak
dd702f6ef8 Allow setting context for members importer and use correct source
no issue

When importing members, the members-importer isn't aware of the context and therefore falls back to use `member` as a source in members event table. This makes it impossible to determine imported members from others.
- Added an `context` property to the options in the members-importer constructor
- Checked for `importer` context when creating member events and assigned the source `admin` to it
2022-04-27 15:27:19 -04:00
Sam Lord
455778662c Email verification for imports based on 30 days of import
refs: https://github.com/TryGhost/Toolbox/issues/293

Things needed to create this:
* MemberSubscriptionEvent now has an import source
* Importer now creates events with this type
* Verification trigger logic changed to use 30 day window of imports
2022-04-13 17:35:30 +01:00
Sam Lord
3c5cf21274 Added email verification trigger package
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.
2022-01-27 10:57:51 +00:00
Fabien O'Carroll
75d003816e Fixed the importer from overriding properties
refs https://github.com/TryGhost/Team/issues/1202

When importing we were transforming the CSV and add missing columns to
it before storing it in preparation to perform the import. This resulted
in the missing columns being updated for existing members with blank
data.

We've updated the Members CSV parsing library to take an options list of
columns to include, which then allows imports to not include all of the
default columns.
2021-12-01 17:02:30 +02:00
Naz
9cbe5efdf5 Removed checkEmailList flag check
refs https://github.com/TryGhost/Team/issues/906

- The feature is entering GA stage and the flag is no longer needed
2021-08-20 17:28:06 +04:00
Naz
fd7f515055 Changed MembersCSVImporter constructor
refs https://github.com/TryGhost/Team/issues/958

- The import threshold has to become a dynamic number based on external parameters. Because of this it makes most sense to have it as an async function parameter
- There's a package API change for importThreshold constructor paramter becoming a fetchThreshold async funciton parameter
2021-08-18 16:09:33 +04:00
Naz
96b3629280 Exposed originalImportSize from members importer
refs https://github.com/TryGhost/Team/issues/912

- The variable is needed as a datapoint in Ghost when sending stats about what the import size was used
2021-07-28 19:13:48 +04:00
Naz
a90d996f33 Added import threshold check to importer
refs https://github.com/TryGhost/Team/issues/912

- We need a way to check if the import threshold has been reached within the members importer. See more deets in refed issue
2021-07-23 20:29:19 +04:00
Naz
a99fec1a7b Fixed members importer tests from leaving files behind
no issue

- After each test run there were csv files lefover which polluted the workspace.
- Ideally the write file would be some kind of a smart mock, so we'd won't have to do synchronous file remoal after each test but that's for later improvement if it becomes problematic
2021-07-21 19:25:41 +04:00
Naz
218a1334bd Added very basic unit test coverage to cover happy path import
refs https://github.com/TryGhost/Team/issues/916

- These tests are nowhere close to perfect of even good. They barely pass "good enough" watermark... it's a start for new tests to be added easier in the future once the features are developed around CSV improrter
2021-07-22 01:53:21 +12:00