refs
https://www.notion.so/ghost/Marketing-Milestone-email-campaigns-1d2c9dee3cfa4029863edb16092ad5c4?pvs=4
- When milestones will be activated we would send out emails to users
that are way above the achieved milestone, as we didn't record
milestones before
- The plan is to implement a 0 milestone and don't send an email for
achieving those and also add all achieved milestones in the first run
until a first milestone is stored in the DB, then increment from there.
- This change takes care of two cases:
1. Milestones gets enabled and runs initially. We don't want to send
emails unless there's already at least one milestone achieved. For that
we add a 0 milestone helper and add a `initial` reason to the meta
object for the milestone event, so we can choose not to ping Slack and
also disable email sending for all milestones achieved in this initial
run.
2. All achieved milestones will be stored in the DB, even when that
means we skip some. This introduces the `skipped` reason which also
doesn't send emails for the skipped milestones, but will do for
correctly achieved milestones (always the highest one).
- Added handling for slack notifications to not attempt sending when
reason is `skipped` or `initial`
- this cleans up all imports or variables that aren't currently being used
- this really helps keep the tests clean by only allowing what is needed
- I've left `should` as an exemption for now because we need to clean up
how it is used
no issue
- Nock doesn't support multiple calls to enableNetConnect -> only the last one counts. This fixes that issue.
- Some tests interacted directly with nock instead of using the mockManager to restore everything.
fixes https://github.com/TryGhost/Team/issues/2611
The old email flow is no longer used since we introduced the email stability flow. This commit removes the related code and tests. The general test coverage decreased a bit as a result, because the old email flow probably had a high test coverage. The new flow is in separate packages, so it couldn't contribute to a higher test coverage (but it does have 100% unit test coverage).
refs https://github.com/TryGhost/Team/issues/2667
Some tests still accessed the internet. Now network access is disabled
by default. This change also introduces two helper methods related to
networking (mocking Slack and Mailgun).
This fixes two unreliable tests:
- Staff service was accessing a Slack test API -> timeout possible
- MentionSendingService was trying to send webmentions for every post
publish/change -> possible timeouts and job issues
refs: https://github.com/TryGhost/Toolbox/issues/389
This removes many error logs when the end-to-end test suite is run with the log-level set to error. Many errors are intentional, so the resolution is typically to stub the error log function and assert that it would have been called.
no issue
- Switches to used newly added config values throughout the services
- Updated the `shouldSendEmail` fn to check if actual value is too far from achieved milestone as determined by the percentage setting (e. g. 998 members should not accidentally receive an email for achieving 100 members)
no issue
- Instead of running milestone service directly on boot, set a random
timeout of 0-4 days to run after boot
- Updated tests
- Service is still behind a beta flag
[Added initial mentions-jobs
service](3656190114)
This is the result of running `cp -r jobs mentions-jobs` in the services
directory.
[Waited for mentions-jobs queue before
shutdown](2bb1a12a89)
This matches the functionality of the existing jobs service where we
will wait
for jobs to complete before closing the process.
[Used mentions-jobs service in the mentions
service](4e4f9fdd00)
This ensures that any delays in the mentions jobs queue does not effect
other
parts of the application.
refs
https://www.notion.so/ghost/Marketing-Milestone-email-campaigns-1d2c9dee3cfa4029863edb16092ad5c4?pvs=4
- Added a `slack-notifications` repository which handles sending Slack
messages to a URL as defined in our Ghost(Pro) config (also includes a
global switch to disable the feature if needed) and listens to
`MilestoneCreatedEvents`.
- Added a `slack-notification` service which listens to the events on
boot.
- In order to have access to further information such as the reason why
a Milestone email hasn't been sent, or the current ARR or Member value
as comparison to the achieved milestone, I added a `meta` object to the
`MilestoneCreatedEvent` which then gets accessible by the event
subscriber. This avoid doing further requests to the DB as we need to
have this information in relation to the event occurred.
---------
Co-authored-by: Fabien "egg" O'Carroll <fabien@allou.is>
no issue
- The way we're going to implement milestones diverged from the original idea of handling email sending within the milestone-emails package, as we'll be sending events instead and will utilise the StaffService to listen to them and send the emails
- This renames the package as well as the service in core itself and all relevant tests
no issue
- For better testability with in-memory repository, refactor the
milestones service to preserve the API instance
- Fetching the information about Stripe live mode from Stripe service
was causing difficulties when testing. As a workaround we switched to
reading the live mode keys and determine it that way.
---------
Co-authored-by: Fabien "egg" O'Carroll <fabien@allou.is>
refs TryGhost/Team#2508
-added sending service e2e tests
-should job off this sending service for better tests
-and for ghost to finish processing the job before shutdown
no issue
There are a couple of issues with resetting the Ghost instance between
E2E test files:
These issues came to the surface because of new tests written in
https://github.com/TryGhost/Ghost/pull/16117
**1. configUtils.restore does not work correctly**
`config.reset()` is a callback based method. On top of that, it doesn't
really work reliably (https://github.com/indexzero/nconf/issues/93)
What kinda happens, is that you first call `config.reset` but
immediately after you correcty reset the config using the `config.set`
calls afterwards. But since `config.reset` is async, that reset will
happen after all those sets, and the end result is that it isn't reset
correctly.
This mainly caused issues in the new updated images tests, which were
updating the config `imageOptimization.contentImageSizes`, which is a
deeply nested config value. Maybe some references to objects are reused
in nconf that cause this issue?
Wrapping `config.reset()` in a promise does fix the issue.
**2. Adapters cache not reset between tests**
At the start of each test, we set `paths:contentPath` to a nice new
temporary directory. But if a previous test already requests a
localStorage adapter, that adapter would have been created and in the
constructor `paths:contentPath` would have been passed. That same
instance will be reused in the next test run. So it won't read the new
config again. To fix this, we need to reset the adapter instances
between E2E tests.
How was this visible? Test uploads were stored in the actual git
repository, and not in a temporary directory. When writing the new image
upload tests, this also resulted in unreliable test runs because some
image names were already taken (from previous test runs).
**3. Old 2E2 test Ghost server not stopped**
Sometimes we still need access to the frontend test server using
`getAgentsWithFrontend`. But that does start a new Ghost server which is
actually listening for HTTP traffic. This could result in a fatal error
in tests because the port is already in use. The issue is that old E2E
tests also start a HTTP server, but they don't stop the server. When you
used the old `startGhost` util, it would check if a server was already
running and stop it first. The new `getAgentsWithFrontend` now also has
the same functionality to fix that issue.
refs https://github.com/TryGhost/Team/issues/2400
- we've deemed it useful to start to return `Content-Version` for all
API requests, because it becomes useful to know which version of Ghost
a response has come from in logs
- this should also help us detect Admin<->Ghost API mismatches, which
was the cause of a bug recently (ref'd issue)
fixes https://github.com/TryGhost/Team/issues/2282
Added a new email service package that is used when the email stability
flag is enabled. Currently not yet implemented so will throw an error
for all entry points (if flag enabled).
Removed usage of `labs.isSet.bind` across the code, because that breaks
the stubbing of labs by `mockManager.mockLabsEnabled` and
`mockManager.mockLabsDisabled`. `flag => labs.isSet(flag)` should be
used instead.
All email depending tests now disable the `emailStability` feature flag
to keep the tests passing + make sure we still run all the tests for the
old flow while the email stability package is being built.
refs https://github.com/TryGhost/Toolbox/issues/461
- Having a 'Origin' in vary header value present on each `OPTIONS` allows to correctly bucket "allowed CORS" and "disallowed CORS" responses in shared caches
refs https://github.com/TryGhost/Toolbox/issues/461
- Having a 'Origin' in vary header value present on each `OPTIONS` allows to correctly bucket "allowed CORS" and "disallowed CORS" responses in shared caches
refs https://github.com/TryGhost/Toolbox/issues/461
- The codebase has ambiguous behavior with OPTIONS request. Adding tests covering edge cases for all possible variations of OPTIONS responses is the first step to solving cahceability of these requests.
- The obvious question if you look into the changeset itself would also be: "WTF did you do with test suite naming? What are these changes in admin and click tracking suites? You having a bad day Naz?". The answer is "yes" (╯°□°)╯︵ ┻━┻
- On a serious note. I've introduced multiple hacks here that should be fixed:
1. Forced test suite execution order for options request - extreme blasphemy. This was last resort decision. I went deep into trying to fixup the server shutdown in the "admin" test suite, which cascaded into failing "click tracking" suite, which has shortcomings on it's own (see notes left in that suite)
2. Exposed "ghostServer" from the e2e-framework's "getAgentsWithFrontend" method. Exposing ghostServer to be able to shut it down (or do other manipulations) was one of the pitfalls we had in the previous test utils, which ended up plaguing the test codebase. Ideally the framework should only be exposing the agents and the rest would happen behind the scenes.
- To fix the hacks above I've raised a cleanup issue (https://github.com/TryGhost/Toolbox/issues/471). I'm very sorry for this mess. The issue at hand has very little to do with fixing the e2e framework, so leaving things "as is".
refs https://github.com/TryGhost/Team/issues/1967
This tests the full flow of publishing a newsletter, and then checking
that clicked links will increase the click count, generate events for
the member which clicked the link as well as the redirects contain the
correct query params.
- renames `refSource`, `refMedium` and `refUrl` to `referrerSource`, `referrerMedium` and `referrerUrl` respectively for consistent naming across files and usages
closes https://github.com/TryGhost/Toolbox/issues/372
- The admin assets are served with a unique hash depending on the build with a year-long "max-age" value in the response cache-control header. The client browsers still do send 'If-None-Match' requests when there is a hard-refresh on the client side. There's no need for 'If-None-Match' requests though!
- With 'immutable' value in the cache-control header, the browser caches are treating responses as "hard-fresh" without sending redundant requests.
- For more about 'immutable' value read https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control#immutable
refs TryGhost/Team#1907
- calculates final attribution source and medium using captured referrer information in history
- adds new referrer-translator that goes through available history and based to determine most valid referrer info
- includes referrer url, source and medium in the attribution data for storage
refs https://github.com/TryGhost/Team/issues/1899
- Added `addEmailAttributionToUrl` method to MemberAttributionService. This adds both the source attribution (`rel=newsletter`) and member attribution (`?attribution_id=123&attribution_type=post`) to a URL.
- The URLHistory can now contain a new sort of items: `{type: 'post', id: 'post-id', time: 123}`.
- Updated frontend script to read `?attribution_id=123&attribution_type=post` from the URL and add it to the URLHistory + clear it from the URL.
- Wired up some external dependencies to LinkReplacementService and added some dummy code.
- Increased test coverage of attribution service
- Moved all logic that removes the subdirectory from a URL to the UrlTranslator instead of the AttributionBuilder
- The UrlTranslator now parses a URLHistoryItem to an object that can be used to build an Attribution instance
- Excluded sites with different domain from member id and attribution tracking
In case there is an issue with the filtering of items in our client
side attribution script, we also check for and remove out of date
items here. This ensures that we do not erroneously attribute signups
or conversions to webpages from more than 24h ago.
refs https://github.com/TryGhost/Team/issues/1833
refs https://github.com/TryGhost/Team/issues/1834
We've added the attribution property to subscription and signup events when the
flag is enabled. The attributions resource is fetched by creating multiple relations
on the model, rather than polymorphic as we ran into issues with that as they can't
be nullable/optional.
The parse-member-event structure has been updated to make it easier to work with,
specifically `getObject` is only used when the event is clickable, and there is now a
join property which makes it easier to join the action and the object.
refs https://github.com/TryGhost/Team/issues/1833
refs https://github.com/TryGhost/Team/issues/1834
We've added the attribution property to subscription and signup events when the
flag is enabled. The attributions resource is fetched by creating multiple relations
on the model, rather than polymorphic as we ran into issues with that as they can't
be nullable/optional.
The parse-member-event structure has been updated to make it easier to work with,
specifically `getObject` is only used when the event is clickable, and there is now a
join property which makes it easier to join the action and the object.
fixes https://github.com/TryGhost/Team/issues/1829
- Remove the subdirectories when creating the Attribution instances
- URLs are now always stored relative to the subdirectory instead of the root directory (makes changing the subdirectory easier)
- Fixed returning absolute urls
- Added tests
refs https://github.com/TryGhost/Team/issues/1808
refs https://github.com/TryGhost/Team/issues/1809
refs https://github.com/TryGhost/Team/issues/1820
refs https://github.com/TryGhost/Team/issues/1814
### Changes in `member-events` package
- Added MemberCreatedEvent (event, not model)
- Added SubscriptionCreatedEvent (event, not model)
### Added `member-attribution` package (new)
- Added the AttributionBuilder class which is able to convert a url history to an attribution object (exposed as getAttribution on the service itself, which handles the dependencies)
```
[{
"path": "/",
"time": 123
}]
```
to
```
{
"url": "/",
"id": null,
"type": "url"
}
```
- event handler listens for MemberCreatedEvent and SubscriptionCreatedEvent and creates the corresponding models in the database.
### Changes in `members-api` package
- Added urlHistory to `sendMagicLink` endpoint body + convert the urlHistory to an attribution object that is stored in the tokenData of the magic link (sent by Portal in this PR: https://github.com/TryGhost/Portal/pull/256).
- Added urlHistory to `createCheckoutSession` endpoint + convert the urlHistory to attribution keys that are saved in the Stripe Session metadata (sent by Portal in this PR: https://github.com/TryGhost/Portal/pull/256).
- Added attribution data property to member repository's create method (when a member is created)
- Dispatch MemberCreatedEvent with attribution
### Changes in `members-stripe-service` package (`ghost/stripe`)
- Dispatch SubscriptionCreatedEvent in WebhookController on subscription checkout (with attribution from session metadata)
refs https://forum.ghost.org/t/admin-template-issues-default-install/31750
- we recently switched to using different folders within `core/built`, to
indicate the assets that are applicable for development/production
environments
- unfortunately, this came with the side effect of the "development" assets
missing in the published tarball, which meant Admin wouldn't load when
running in development mode
- this was a regression from how it previously worked because we used to
just copy the production HTML file to the development HTML name, and
use the same assets
- after thinking about it, I think we can get rid of the split folders
for assets, because I don't think the use-case is there for having
them:
- if you run Ghost from source, you're 99% only using the
development-built assets
- if you want production ones, you can run with a flag, but the
development ones get wiped anyway
- those running Ghost from a published package are using the same
assets and HTML file
- therefore, I think we can make our lives simpler by removing the env
folders and using a folder under `core/built/admin/...`
- this commit implements that across Ghost and Admin
refs https://github.com/TryGhost/Admin/pull/2252
closes https://github.com/TryGhost/Team/issues/1182
- Admin now copies it's build output to a single env-specific directory rather than splitting html and assets
- `core/built/admin/{development|production}/*`
- updated the admin app's `serveStatic` definition for assets and controller's html serving to reflect the new asset paths
refs https://github.com/TryGhost/Toolbox/issues/354
- this commit turns the Ghost repo into a monorepo so we can bring our
internal packages back in, which makes life easier when working on
Ghost
