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"
- Made a new function that checks if stubs are needed per tab
- Renamed the check for pagination to be a little more clear
- Made sure the pagination block is always to the right
- Made sure the pagination buttons don't show when there is no pagination
refs https://github.com/TryGhost/Team/issues/2149
- Removed the dummy rows as they looked like loading states
- Improved the spacing of the pie chart
- Showing total number when pagination isn't required
- Tweaks across the page to look more consistent
refs https://github.com/TryGhost/Team/issues/2149
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/2078
This adds handling to the Tier events to create Stripe Products & Prices when
Tiers are created and have their pricing changed. We also update the Stripe
Product names when the Tier name is edited.
We also add a method to generate a payment URL which will replace our current
implementation of creating a Stripe Checkout Session
refs https://github.com/TryGhost/Team/issues/2078
We want to be able to easily get the price for a Tier based on the cadence for
use when creating Stripe Prices. We also want to be able to set the pricing data
to `null` for free Tiers, rather than erroring.
refs: https://github.com/TryGhost/Ghost/issues/15537
- snapshot test created to add confidence to webhook stability and increase overall test coverage.
Co-authored-by: Kritika Sharma <kritikasharma@Kritikas-MacBook-Pro-2.local>
- Main goal to improve modal when audience feedback fails
- Felt right to also improve the newsletter unsubscribe issue, too
- Makes this more pleasing to read and look at, nothing fancy
refs https://github.com/TryGhost/Team/issues/2081
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)
fixes https://github.com/TryGhost/Team/issues/2137
For the analytics page, we need the sent events to show up immediately
after sending an email. Otherwise we need to wait for emails to be
marked as received (which takes too long) before being able to show them
on the analytics page.
This adds the email_sent_event, which is hidden by default everywhere
and used on the analytics page.
fixes https://github.com/TryGhost/Team/issues/2114
fixes https://github.com/TryGhost/Team/issues/2115
When a new newsletter is created, the frontend will send feedback_enabled to true. We'll catch this in the backend and don't allow setting feedback_enabled to true when audience_feedback flag is disabled. This is also handled for editing newsletters.
To fix this in existing sites, I added a migration that disables feedback for all sites (since this is an alpha feature). Once we'll release the feature later, it will be disabled for existing newsletters, just like expected.
fixes: https://github.com/TryGhost/Ghost/issues/15291
- An attempt to improve re-authenticate modal toggle - show re-authenticate modal every time user save (ctrl/cmd + s)
- An attempt to fix redirection when user re-login on different tab. Prevent redirection to sign-in page since the user already logged in on another tab.
- Re-enable `editor` test on `authentication-test.js`
refs: https://github.com/TryGhost/Ghost/issues/14882
- Opted to use the in-house `sequence` function when refactoring Bluebird's `Promise.each` to avoid deadlock issues (see 734ef66e6c).
-It's hard to know without tonnes of context if any `Promise.each` are safe to refactor to `Promise.all`.
refs: https://github.com/TryGhost/Ghost/issues/14882
- Removing bluebird specific methods in favour of the Ghost sequence method so we can remove the bluebird dependency
Co-authored-by: Hannah Wolfe <github.erisds@gmail.com>
fixes https://github.com/TryGhost/Team/issues/2112
- Removed a bit of duplicate code across templates and components that was used to handle filters
- Updated filter objects to contain information about the filter
- Added resource filters that are able to select a single resource, which can be used in columns
- Filters can now define columns by themselves. Not all columns already make use of this functionality, but we can move those over later (cleanup: https://github.com/TryGhost/Team/issues/2133)
- The filter definitions became quite long. We should move them to separate files in the future: https://github.com/TryGhost/Team/issues/2134
- Filters can now have custom NQL parsing
- Improved support for parsing recursive or grouped NQL queries
- Added support for filtering members by feedback
refs https://github.com/TryGhost/Team/issues/2116
- allows site owners to edit a link in a post that has already been sent out, fixing any typos or other mistakes
- resets click counter for the edited link back to 0 so site owners can see the clicks on new link, doesn't change the overall click count
closes: https://github.com/TryGhost/Ghost/issues/15542
- custom theme settings were not reinstated on import
- importing custom theme settings for the current active theme requires the theme be re-activated
- Careful of flags, having different paginations depending on those
- Little stylistic adjustment to how pagination is shown on engagement bar
- Quite a few feature flag checks to make sure the right style is showing
refs https://github.com/TryGhost/Team/issues/2136
refs https://github.com/TryGhost/Team/issues/1076
- The 'should' assertion library is deprecated. Native 'assert' is the recommended lib to use
- Migrating this bit of code allows to remove the should's "utils" folder. Less code, yey!
no issue
Improve the route communication between Ghost Admin and Ghost Explore to
reflect route changes in the URL and correctly navigate to Explore sub
routes
refs https://github.com/TryGhost/Team/issues/2135
The email link redirects on Pro are cached as 302 redirects in Varnish, so we're missing further clicks after the first one for each member, until the cache is invalidated. This change invalidates cache on link edits to ensure that we correctly redirect members to updated link everytime
refs https://github.com/TryGhost/Team/issues/2116
- editing a link caused it to jump in the list as its count is reset to 0
- this forces the order of links same after edit, the new order is only visible on refresh or navigating away and coming back
refs https://github.com/TryGhost/Team/issues/2116
- wires link update API on post analytics UI page to save edited links for a newsletter
- all links matching the edited link are updated by the API in the backend
refs https://github.com/TryGhost/Team/issues/2104
When a newsletter link is edited, we reset its click count to 0 to show only the clicks on newly edited links. This is done by only counting the member click events for a link which are greater than its last updated at, so that all previous click events are not counted for the link, but are included in the total count of all links on the page.
- Newsletter clicks module has new header and box
- Source attribution module has new header and box
- Resources have new headers inside the box
- New split layout if there are two modules side-by-side
refs https://github.com/TryGhost/Team/issues/2119
refs https://github.com/TryGhost/Team/issues/2078
These events are all required for other parts of the Ghost system to stay in
sync. The events on each Tier object will be dispatched by the TierRepository
once they've been persisted.
TierCreatedEvent - generate Stripe Products & Prices
TierNameChangeEvent - update Stripe Products
TierPriceChangeEvent - update Stripe Products & Prices
TierArchivedEvent - update the Portal settings for visible tiers
- disable Stripe Products & Prices
TierActivatedEvent - enable Stripe Products & Prices
refs 82ed10473b
refs https://github.com/TryGhost/Team/issues/1869
- getDefaultProduct has unified logic across different places (see refed commit). It is recommended to use instead of writing custom queries prone to mistakes.
- Also added more readable name to the possible error message thrown by setComplimentarySubscription
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.
refs https://github.com/TryGhost/Team/issues/1869
- There are multiple places in the codebase fetching "default product". The code is slightly divergent in each one of them and has been a source of bugs (like the one referenced). Having the logic captured in one place will allow reducing the code duplication, making code less bug prone, and making testing the modules dependent on the "setDefaultProduct" method easier
refs https://github.com/TryGhost/Team/issues/2078
The check for creating free Tiers needs to be explicit, rather than implicit
because the type is optional, we only want to error if it is explicitly passed
as "free", not if it is missing as "paid".
refs https://github.com/TryGhost/Team/issues/2078
Having to map between snake_case and camelCase was becoming confusing, so this
updates the Tier object to exclusively use camelCase, and the snake_case for the
API can be handled by the serializer/mapper or at the controller level.
refs https://github.com/TryGhost/Team/issues/2078
This removes the burden from the Tier object, and allows us to reuse
the existing slug generation implementation we have in our bookshelf
models.
refs https://github.com/TryGhost/Team/issues/2034
- this table will be used to link Stripe subscriptions to Ghost
subscriptions via a foreign key that we add at a later point
- this also includes `constraintName` as the auto-generated one would be
too long for MySQL 8
refs https://github.com/TryGhost/Team/issues/2104
- adds edit permissions for links endpoints to fixtures
- new `bulkEdit` endpoint will use the permissions and allow fixing newsletter links via Admin
refs 5fcf5098a8
- links browse endpoint had permissions switched off unintentionally and was also missing the necessary permissions in fixtures.
- enables permissions for browse endpoint and adds migration insert permissions in DB
- fixes an edge case where if a site has no unavailable sources in a particular period, it broke the table view as the `Others` data doesn't get fetched
fixes https://github.com/TryGhost/Team/issues/2090
- This changes how sentiment is exposed in the API. Now it is exposed as a `sentiment` relation, directly on the model (no longer in counts). Internally we still use `count.sentiment`.
- Content API users (and themes) can include the 'sentiment' relation and order by sentiment.
- Updated Admin to use sentiment instead of count.sentiment
refs https://github.com/TryGhost/Team/issues/2116
- wires handling of success and error messages on editing a newsletter link
- has the update api commented out temporarily to mock the changes
closesTryGhost/Team#2080
- If the post was published and emailed the link leads the user to the
post.
- If the post was just emailed the link leads the user to the home page.
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
- Previously the CSV parser had "map whatever you can and pass on unknown properties further" approach to CSV parsing. This logic has led to unwanted fields leaking through CSV imports - messy, dangerous.
- The strict mapping rules act as a "validator" to the user input, only passing through the fields we expect explicitly - safer clean cut solution with no unintended side-effects.
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 https://github.com/TryGhost/Toolbox/issues/430
- Not having any extra logic in the mapper will allow to have a generalized "mapping" concept for CSV input serialization
- This is groundwork for stricter header value filtering on the parsing stage
refs https://github.com/TryGhost/Team/issues/2078
As part of decoupling Stripe from the members feature, we are going to be using
payment URLs rather than Stripe sepcific session ids and publick keys.
This prepares Portal to work with the new system, whilst retainign the existing
functionality
refs https://github.com/TryGhost/Team/issues/2078
This pulls the current Tiers logic into its own package, the persistence part of
the work has not been done yet, that will be handled in core, so all bookshelf
model specific stuff is kept together.
fixes https://github.com/TryGhost/Team/issues/2091
fixes https://github.com/TryGhost/Team/issues/2089
- Added new fixtures to make testing easier for the activity feed
- Improved E2E test coverage of activity feed with separate test file
- Added data.post_id filter to enable filtering by events related to a
given post
- Fixed return types in JSDoc of test agents (TypeScript interprets
these as `typeof Agent` if we don't add `InstanceType<Agent>`)
- Added total pagination metadata to activity feed API (to allow a basic
type of pagination using filters)
fixes https://github.com/TryGhost/Team/issues/2096
When generating the recipient data for emails, the email clicks
implementation is resulting in a recipient variable being added called
replacement_xxx once for each link containing the same UUID.
This generates a lot of unnecessary data overhead for emails, and it
turns out that mailgun has a 25MB message limit. We wouldn't have come
close if we only included the uuid once.
fixes https://github.com/TryGhost/Team/issues/2102
- this column was added with `nullable: true` but it should never be
nullable, so we should drop the nullable status whilst it's easy to
- de-duped the exports at the bottom if they export the same name as the
function
- added types to all functions, or fixed existing ones
- renamed `table` to `tableBuilder` to represent it better
- these should help with code readability and autocomplete in editors
no issue
- the `memberRecord` getter on the members activity controller was
returning the direct result of `store.findRecord()` which in reality is
a `PromiseObject` that is a type of proxy. Although templates handle
this OK, any JS code using the object needs to know that it's not a
typical type of object and has to use `.get()` for all property access
to avoid errors
- switched `memberRecord` from a getter to a resource so we have more
control over the returned object and loading lifecycle
- added a `MemberFetcher` resource class that awaits the result of the
store find and only then sets the value meaning we always have a fully
resolved model object
- being a resource the fetch will only occur when the `member` id
property changes, unlike the getter which performed the fetch every time
it was accessed
fixes https://github.com/TryGhost/Team/issues/2084
- When audience feedback is enabled, we use a single 'conversions' count instead of having separate ones for signups and paid conversions.
- The analytics component is separated so we can change it without breaking the existing page.
refs https://github.com/TryGhost/Team/issues/2082
- in the event the API doesn't return a 200 OK, we shouldn't be
processing the response from it, as we can end up doing weird things
if, for example, an error object is returned
refs https://github.com/TryGhost/Team/issues/2082
- in the event the API doesn't return a 200 OK, we shouldn't be
processing the response from it, as we can end up doing weird things
if, for example, an error object is returned
refs https://github.com/TryGhost/Team/issues/2082
- if a site has comments enabled but doesn't use the `comments_count`
helper, the comments-count.min.js will still be loaded and it'll send
a POST request to Ghost with an empty array of post IDs to fetch
- this is unnecessary and we should avoid this extra request for pages
that don't need to show comment counts
- this commit prevents the comment-counts JS from sending the request if
there are no post IDs to fetch
refs https://github.com/TryGhost/Team/issues/2082
- if a site has comments enabled but doesn't use the `comments_count`
helper, the comments-count.min.js will still be loaded and it'll send
a POST request to Ghost with an empty array of post IDs to fetch
- this is unnecessary and we should avoid this extra request for pages
that don't need to show comment counts
- this commit prevents the comment-counts JS from sending the request if
there are no post IDs to fetch
refs https://jsdoc.app/tags-param.html#optional-parameters-and-default-values
- using an equals sign in the type definition is part of the Google
Closure syntax but we use the JSDoc syntax in all other places, and
tsc detects the different syntax
- this commit standardizes the syntax ahead of enforcing a certain style
down the line
refs https://github.com/TryGhost/Team/issues/2029
This will allow us to start decoupling the Stripe side of things once we've got
the core data stored. We've also add some integrity checks on the incoming
monthly_price and yearly_price to ensure they are the same currency.
refs https://github.com/TryGhost/Team/issues/2072
Google is indexing our redirects and storign the redirected content
against the redirect URL in search results. This seems to be caused by
us using a 302 redirect rather than 301. We don't want to switch to a
301 however, so that we can support the ability to update redirects in
the future.
refs https://github.com/TryGhost/Team/issues/1765
In order to better handle deleted objects in Stripe we want to decouple
Members from Stripe.
These changes allow us to have the Tier concept completely independent
of the Stripe tables, such that the Stripe data can be generated as/when
it's needed - which will help to protect against missing data.
refs: https://github.com/TryGhost/Ghost/issues/14882
- Removing bluebird specific methods in favour of native promises so we can remove the bluebird dependency.
Co-authored-by: Carol-Barno <cbarno@innovexsolutions.co.ke>
Co-authored-by: Hannah Wolfe <github.erisds@gmail.com>
closes: https://github.com/TryGhost/Ghost/issues/14973
- When fetching content using a non-standard charset, characters were notproperly decoded to utf-8 resulting in mangled text in the editor -> Detect charset and use iconv to decode the page text
- When requesting a non bookmark card, if no oembed data could be foundand we fallback to bookmark, a second network request to fetch the content was issued. This seemed unnecessary -> refactored to avoid that
refs: https://github.com/TryGhost/Ghost/issues/15537
- snapshot test created to add confidence to webhook stability and increase overall test coverage.
Co-authored-by: Kritika Sharma <kritikasharma@Kritikas-MacBook-Pro-2.local>
closes: https://github.com/TryGhost/Ghost/issues/15267
- This was because the URLs were not being encoded and matched correctly - it is solved by encoding the URL before adding to the router.
refs https://github.com/TryGhost/Team/issues/2030
- we tend to use US spellings in the code and this was merged with the
British spelling
- nothing has been added to this table yet so it's safe to switch
refs https://github.com/TryGhost/Team/issues/2030
- adds `subscriptions` table to the DB schema
- this new table is aimed to support a native "subscription" primitive in Ghost
that most resembles previously used `members_stripe_customers_subscriptions` table
refs https://github.com/TryGhost/Toolbox/issues/441
- whilst reviewing another PR, I noticed we were incorrectly using
`maxLength` instead of `maxlength` in the schema column definition
- it turns out we've already been doing this wrong for a while with
other columns
- this key is not acted upon, so the maximum column length was not applied
- fixing up the DB to the correct maximum length is something to fix in the
future but right now, the schema does not reflect the size of the
column that actually got created
- the fallback when `maxlength` is not provided is currently 191 [0], so
this commit switches the schema and migrations to using the correct
key name and column length that they are using when applied
[0]: 24670aa555/ghost/core/core/server/data/schema/commands.js (L27)
refs https://github.com/TryGhost/Toolbox/issues/441
- we tend to have a mix of `bool` and `boolean` in the schema and
migrations, which has become a real nit for me at this point
- we don't do any special handling between `bool` and `boolean`, it's
just something we pass to Knex
- `bool` is an alias for `boolean` but `boolean` is actually documented - https://knexjs.org/guide/schema-builder.html#boolean
- this commit switches Ghost to only using `boolean` in the schema and
migrations, and removes `bool` from the allowlist in tests to prevent
us from adding it again in the future
- this should make absolutely no difference to the DB because both
resulted in the same column
refs https://github.com/TryGhost/Toolbox/issues/430
- The 'readCSV' method was only exposed to be used in the unit tests. To keep the module code to the minimum moved readCSV to the unit test itself - the only place where it's used and belongs to.
refs https://github.com/TryGhost/Toolbox/issues/441
- I'm currently working on cleaning up our uses of `bool` and `boolean`
in favor of `boolean`, and I've noticed we only handle converting
numbers into booleans when the type is `bool`, so validation would
otherwise fail
- given these can be used interchangeably, we should also support
converting the numbers into booleans when the type is `boolean`
- this is going to get cleaned up again when I remove `bool` but this
fixes the validation bug for now
refs: https://github.com/TryGhost/Ghost/issues/15537
- snapshot test created to add confidence to webhook stability and increase overall test coverage.
Co-authored-by: Kritika Sharma <kritikasharma@Kritikas-MacBook-Pro-2.local>
closes: https://github.com/TryGhost/Ghost/issues/15500
- Per the issue, Ghost has a policy to never throw 500 Internal Server errors for theme issues. This change adds a check inside of `ghost\core\core\frontend\helpers\t.js` if `text` or `options` is undefined, to throw an `IncorrectUsageError` error within the function.
- Messaging was borrowed from `ghost\core\core\frontend\web\middleware\error-handler.js`.
refs: https://github.com/TryGhost/Ghost/issues/15537
- this adds an e2e test and test snapshot for the `tag.edited` webhook so we can prevent regressions and bugs in the future
Co-authored-by: Hannah Wolfe <github.erisds@gmail.com>
closes: https://github.com/TryGhost/Ghost/issues/14981
- Taxonomy-specific sitemaps were invalid xml when there was no data
- These invalid empty sitemaps were referenced in the index sitemap causing SEO tools to report errors
closes: https://github.com/TryGhost/Ghost/issues/15470
- When multiple browser tabs are open, each manipulate a different copy of ember data model, changes to the model in one tab are not reflected in the model of the other tab.
- When updating some settings, all current settings were sent to the API.
- As a result, when updating two different categories of settings (navigation/code inspection) in different tabs, the second update was overriding the first one.
- From a user perspective, this is not a natural behaviour. Only settings visible on-screen when clicking save should be modified.
Co-authored-by: Kevin Ansfield <kevin@lookingsideways.co.uk>
refs https://github.com/TryGhost/Toolbox/issues/441
- this is only v1 of the test I would like but it validates the keys on
a column definition are part of an allowlist
- this has already uncovered a bug with `maxLength` (vs `maxlength`)
