fixes https://github.com/TryGhost/Team/issues/2366
refs https://ghost.slack.com/archives/C02G9E68C/p1670232405014209
Probem described in issue.
In the old MEGA flow:
- The `email_verification_required` check is now repeated inside the job
In the new email service flow:
- The `email_verification_required` is now checked (didn't happen
before)
- When generating the email batch recipients, we only include members
that were created before the email was created. That way it is
impossible to avoid limit checks by inserting new members between
creating an email and sending an email.
- We don't need to repeat the check inside the job because of the above
changes
Improved handling of large imports:
- When checking `email_verification_required`, we now also check if the
import threshold is reached (a new method is introduced in
vertificationTrigger specifically for this usage). If it is, we start
the verification progress. This is required for long running imports
that only check the verification threshold at the very end.
- This change increases the concurrency of fastq to 3 (refs
https://ghost.slack.com/archives/C02G9E68C/p1670232405014209). So when
running a long import, it is now possible to send emails without having
to wait for the import. Above change makes sure it is not possible to
get around the verification limits.
Refactoring:
- Removed the need to use `updateVerificationTrigger` by making
thresholds getters instead of fixed variables.
- Improved awaiting of members import job in regression test
- this was all getting terribly behind so I've done several things:
- majority of `@tryghost/*` except Lexical packages
- gscan + knex-migrator to remove old `@tryghost/errors` usage
- bumped lockfile
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.
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.
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
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
refs: https://github.com/TryGhost/Ghost/issues/14882
- Removed bluebird from members-csv package-json and update-check-service
- Removing bluebird specific methods in favour of the Ghost sequence method so we can remove the bluebird dependency
refs https://github.com/TryGhost/Team/issues/2077
- The "productRepository" methods have been deprecated in favor of "tiers" and "Tiers API".
- The changes migrated usages of "productRepository.getDefaultProduct" to Tiers API's "readDefaultTier"
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)
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)
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.
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'
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
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
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.
refs https://github.com/TryGhost/Toolbox/issues/430
- The job "status" is never anything different than "pending" and never leaves the module itself. It's an outdated concept that only takes up lines of code!
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.
refs https://github.com/TryGhost/Toolbox/issues/430
refs https://github.com/TryGhost/Ghost/issues/14882
- Having an explicit mappings passed into the members CSV parser makes it easier to control and understand the transforms for package clients
- Eventually the parser will receive a strict map with the fields it should parse - skipping all unknown & unmapped fields
refs/closes https://github.com/TryGhost/Team/issues/2004
- for imports, members are created inside a transaction, which causes the member created events to be dispatched.
- its possible that transactions for import can be rolled back if for some reason there is an error down the line while inserting other member properties. The rollback doesn't commit the member to DB, but the event dispatched earlier will still try to create the member created event which fails due to missing member id.
- knex transactions resolve the `executionPromise` both in case of explicit commit or rollback from the user, so just the transaction end check will not be good enough to make sure the member exists in DB
- adds explicit config to knex to reject transaction in case of rollback, which is then caught and event is not dispatched
no refs.
- CSV overview table headings were selectable
- spinner on the last screen was confusing: users might think they had to wait for something to finish, but that's not true
- spacings in import confirmation emails were off
- cleaned up unused dependencies
- adds missing dependencies that are used in the code
- this should help us be more explicit about the dependencies a package
uses
- because of how the npm scripts were set up, we were running the full
Admin integration tests during the unit tests phase of CI
- this commit renames the majority of `test` to `test:unit` in the
package.json files, and aliases `test` to `test:unit`
- special packages like Admin have no-op'd `test:unit` scripts so we
don't end up running its tests
refs https://github.com/TryGhost/Team/issues/1723
- Added count.replies to comments
- Added replies endpoint
- Limited returned replies to 3.
- Replaced likes_count with count.likes in comments
- Instead of fetching all the likes of a comment to determine the total count, we'll now use count.likes
- Instead of fetching all the likes of a comment to determine whether a member liked a comment, we'll now use count.liked (which returns the amount of likes of the current member, being 0 or 1). This is mapped to `liked` to make it more natural to work with.
The `members.test.snap` file changed because we no longer include `liked: false` if we didn't fetch the liked relation. And in the comments events of the activity feed the liked property is therefore removed.
These changes requires an update to the `bookshelf-include-count` plugin:
- Updated to also work for nested relations
- This moves the count queries from the `bookshelf-include-count` plugin to the `countRelations` method of each model.
- Updated to keep the counts after saving a model (crud.edit didn't return the counts before)
refs https://github.com/TryGhost/Toolbox/issues/354
- set packages to `private: true`
- removed repository link - these packages won't be published so this
link won't be seen anywhere
- removed `publishConfig`
refs https://github.com/TryGhost/Toolbox/issues/354
- these READMEs were migrated over from when each package was in a
different repo
- they also assume you're going to be publishing the packages because it
mentions install instructions
- only a few of them contain custom content
- this commit deletes the majority of these files because they're now
not useful
- any that contained other instructions have been cut down
refs https://github.com/TryGhost/Toolbox/issues/354
- these repository links made sense when they were in different repos
and published to NPM but we don't publish these packages any more
- this commit deletes those keys from the files
- these were copied over during the monorepo conversion but we're not
going to be publishing these packages so the top-level LICENSE file
covers all packages here
- we're going to be pinning all dependencies within the monorepo
- this shouldn't change anything anyway because we're using the same
version across all packages
