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`)
fixes https://github.com/TryGhost/Team/issues/2054
This change adds the sentiment and positive_feedback counts to the posts models. This change isn't really ideal because there are some problems here:
- sentiment isn't really a count
- we don't need to include the sentiment and positive_feedback as a default for posts (but the same is true for attribution)
It would make sense to move this to separate endpoints that only fetch the analytics for a given post when the analytics page is opened. But for our initial skateboard version of audience feedback this should be a good start to already see the data.
refs https://github.com/TryGhost/Team/issues/2047
- We anticipate upcoming changes in the PUT /members/:id/subscriptions/:subscription_id endpoint , so covered it with a snapshot test to track the differences more precisely.
- Note, the test case contains a more explicit outgoing HTTP request mocking.
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.
closes: https://github.com/TryGhost/Ghost/issues/15450
- the object property "note" was updated only on focus out, which is wrong
- the property should be updated on input
-this problem occurred only for TextArea component
- up until this commit, git hooks were only used by a handful of people
because they were a pain:
- they'd only be set up when you did `yarn setup`
- the existing hooks ran `yarn lint` on all projects, which was
incredibly slow
- as a result, not many of us actually had them enabled, but this would
cause issues in CI because people were pushing un-linted commits
- other JS projects tend to use husky to automate the git hook setup and
lint-staged to speed up linting on changed files
- this commit switches to using them both
- `lint-staged` only runs `eslint` on staged JS files that are about to
be committed - if there's a linting error, it will stop the commit
- I've configured the pre-commit hook to successfully exit in CI because we
don't want to run pre-commit hooks right now
- this means we can remove Grunt - yay!
- all are minor issues but they stop the editor showing function names,
parameters and return types otherwise
- this should help with a better developer experience
no issue
- if a plain native class instance with tracked properties is validated against the `ValidationEngine` and it's associated validators would cause errors by assuming that the instance has a `.get()` method
- updated all model access in `ValidationEngine` and the validators to use direct property access which works for both native class and `EmberObject` instances
refs https://github.com/TryGhost/Ghost/issues/14101
- migrated to full native class syntax
- swapped use of `<GhTextInput>` in favor of native `<input>` and removed use of `{{action}}` in associated template
refs https://github.com/TryGhost/Ghost/issues/14101
- migrated to full native class syntax
- removed loading of snippets as they are not needed on post lists (they are needed on the editor screen which does it's own loading)
- removed `access` query param definition leftover from earlier development
- removed use of `{{action}}` in associated templates
no issue
- moved screen-specific components out of the top-level components directory
- top-level directory should eventually only contain generally re-usable/application-wide components
no issue
- moved screen-specific components out of the top-level components directory
- top-level directory should eventually only contain generally re-usable/application-wide components
no issue
- the `ella-sparse-array` dependency used for the sparsely populated list on the members screen creates ProxyObjects that wrap the underlying member model instances meaning the forced use of `.get()` and `.set()` required by ProxyObject was leaking through to other areas of the app causing a mismatch in code patterns
- moved the loading state for each member into a separate component and put the loading conditional directly inside the `{{#each members}}` block so that we can pass the real model instance through to components via `{{member.content}}` rather than passing the ProxyObject wrapper, avoiding unexpected errors when not using `.get()` and `.set()` on member arguments
no issue
The `config` service has been a source of confusion when writing with modern Ember patterns because it's use of the deprecated `ProxyMixin` forced all property access/setting to go via `.get()` and `.set()` whereas the rest of the system has mostly (there are a few other uses of ProxyObjects remaining) eliminated the use of the non-native get/set methods.
- removed use of `ProxyMixin` in the `config` service by grabbing the API response after fetching and using `Object.defineProperty()` to add native getters/setters that pass through to a tracked object holding the API response data. Ember's autotracking automatically works across the native getters/setters so we can then use the service as if it was any other native object
- updated all code to use `config.{attrName}` directly for getting/setting instead of `.get()` and `.set()`
- removed unnecessary async around `config.availableTimezones` which wasn't making any async calls
no issue
The `settings` service has been a source of confusion when writing with modern Ember patterns because it's use of the deprecated `ProxyMixin` forced all property access/setting to go via `.get()` and `.set()` whereas the rest of the system has mostly (there are a few other uses of ProxyObjects remaining) eliminated the use of the non-native get/set methods.
- removed use of `ProxyMixin` in the `settings` service by grabbing the attributes off the setting model after fetching and using `Object.defineProperty()` to add native getters/setters that pass through to the model's getters/setters. Ember's autotracking automatically works across the native getters/setters so we can then use the service as if it was any other native object
- updated all code to use `settings.{attrName}` directly for getting/setting instead of `.get()` and `.set()`
- removed use of observer in the `customViews` service because it was being set up before the native properties had been added on the settings service meaning autotracking wasn't able to set up properly
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 issue
- Added Ghost Explore screen behind alpha flag
- Moved existing /explore route to /explore/connect which we'll redirect to for outside requests
- Added iframe communication with Ghost Explore App
closes https://github.com/TryGhost/Toolbox/issues/402
- The SQL error was thrown whenever a job error was happening and was trying to persist an error. Persisting an error should only happen for "named" one-off jobs, instead of just one-off jobs.
refs/closes TryGhost/Team#2014
- Updated preview text.
- By default email client pulls text from email body for preview. Was added spaces to prevent such behaviour.
fixes https://github.com/TryGhost/Team/issues/1946
Problem:
- When running the admin tests in a timezone that is later than UTC, the tests failed.
Causes:
- Some tests needed some adjustements
- The DateTimePicker did not always use the correct timezone.
- Test models createdAt times sometimes depended on the timezone of the test runner
Solution:
- All the input DateTimePicker gets should be processed in the blog's timezone.
- Make sure that all communication (properties, setters, minDate...) with `PowerDatepicker` happens in the local timezone. When setting, convert that date to the blog timezone and use that as the real value.
- we only need to provide the patch if we want to force Ghost to use a
specific version
- otherwise, we can just use major.minor because we use the tilde
versioning method
- having the patch version here just encourages you to bump it
unnecessarily, so removing it cleans up the usage for now
closes https://github.com/TryGhost/Toolbox/issues/399
- The MemberCreatedEvent event is more accurate representation of the limit nature - counting the number of members created. The previous MemberSubscribeEvent was slightly hacky solution because a member could be subscribed/unsubscribed multiple times and distorting the limit counts.
- Closes dropdown when removing labels
- Larger width to handle multiple labels better
- Subtle focuses when using tabs only
- Bigger close button hitpoints on labels
- Added text ellipis for larger labels in dropdown
- Added some event propagation stops
no issue
refs https://github.com/TryGhost/Toolbox/issues/320
- Added more complex mobiledoc structure in the post.published test to check for correct transformation of special purpose `__GHOST_URL__`. The snapshot has a correct URL transformation, which gives confidence it works properly
- I lowered the code coverage on the repo to the point where
it started failing because I added a new export to the config library
- this wasn't easy to add tests for because the existing config tests
use the loader directly and not the library export
- instead, I'm just going to make the dev script access the loader, and
make a note to clean this up in the future when we pull out the config
module
fixes https://github.com/TryGhost/Team/issues/2009
- When an email is sent, but it failed there was no way to retry once you left the retry screen
- There was no indication that the email failed to send in the post list and editor
- because the cwd of `.github/dev.js` is not `ghost/core`, it doesn't
pick up config.local.json files, so any configuration you set in there
isn't applied
- this meant that developers with HTTPS configured locally couldn't use
`--stripe` because it wouldn't configure the Stripe listening URL
correctly
- this adds an exports to the config lib to allow passing options in,
which I then utilize to pass the directory that config resides in
- this should fix the aforementioned problem with HTTPS
refs https://github.com/TryGhost/Toolbox/issues/320
- There noe "roles" attached to the post's author when the 'post.added' event is fired. Webhooks function based of the model events and differ slightly with it's output comparing to the API response. For example, in case of Posts API, there'a an additional 'findOne' call (ref.: https://github.com/TryGhost/Ghost/blob/main/ghost/core/core/server/models/post.js#L1224-L1227) before returning the post to the endpoint handler and then passing that to the output serializer.
- If we want to have 1:1 copy of webhooks outputs and API outputs, we should rethink how we rely on model event data which is never the same as API controller level data.
refs a499f866f3
refs d817e5830d
- The user-agent used in outgoing Ghost requests (webhooks mostly) is dependent on the Ghost version - snapshots break if the matcher is not dynamic.
- There will be a few more webhooks tests coming soon, so makes sense to have this matcher moved to a common "framework matchers"
fixes https://github.com/TryGhost/Ghost/issues/14508
This change requires the frontend to send an explicit `emailType` when sending a magic link. We default to `subscribe` (`signin` for invite only sites) for now to remain compatible with the existing behaviour.
**Problem:**
When a member tries to login and that member doesn't exist, we created a new member in the past.
- This caused the creation of duplicate accounts when members were guessing the email address they used.
- This caused the creation of new accounts when using an old impersonation token, login link or email change link that was sent before member deletion.
**Fixed:**
- Trying to login with an email address that doesn't exist will throw an error now.
- Added new and separate rate limiting to login (to prevent user enumeration). This rate limiting has a higher default limit of 8. I think it needs a higher default limit (because it is rate limited on every call instead of per email address. And it should be configurable independent from administrator rate limiting. It also needs a lower lifetime value because it is never reset.
- Updated error responses in the `sendMagicLink` endpoint to use the default error encoding middleware.
- The type (`signin`, `signup`, `updateEmail` or `subscribe`) is now stored in the magic link. This is used to prevent signups with a sign in token.
**Notes:**
- Between tests, we truncate the database, but this is not enough for the rate limits to be truly reset. I had to add a method to the spam prevention service to reset all the instances between tests. Not resetting them caused random failures because every login in every test was hitting those spam prevention middlewares and somehow left a trace of that in those instances (even when the brute table is reset). Maybe those instances were doing some in memory caching.
refs https://github.com/TryGhost/Ghost/issues/14101
- migrated staff user controller to native class syntax
- removed use of `{{action}}` helper
- moved from custom components to native `<input>` and `<textarea>` for form fields
- added `{{select-on-click}}` modifier to cover the `<GhTextingInput @selectOnClick>` option behaviour for any input element
- added `submitForm()` test helper that finds closest `form` element and trigger's a `submit` event on it simulating <kbd>Enter</kbd> being pressed whilst a field has focus
fixes https://github.com/TryGhost/Ghost/issues/14508
This change requires the frontend to send an explicit `emailType` when sending a magic link. We default to `subscribe` (`signin` for invite only sites) for now to remain compatible with the existing behaviour.
**Problem:**
When a member tries to login and that member doesn't exist, we created a new member in the past.
- This caused the creation of duplicate accounts when members were guessing the email address they used.
- This caused the creation of new accounts when using an old impersonation token, login link or email change link that was sent before member deletion.
**Fixed:**
- Trying to login with an email address that doesn't exist will throw an error now.
- Added new and separate rate limiting to login (to prevent user enumeration). This rate limiting has a higher default limit of 8. I think it needs a higher default limit (because it is rate limited on every call instead of per email address. And it should be configurable independent from administrator rate limiting. It also needs a lower lifetime value because it is never reset.
- Updated error responses in the `sendMagicLink` endpoint to use the default error encoding middleware.
- The type (`signin`, `signup`, `updateEmail` or `subscribe`) is now stored in the magic link. This is used to prevent signups with a sign in token.
**Notes:**
- Between tests, we truncate the database, but this is not enough for the rate limits to be truly reset. I had to add a method to the spam prevention service to reset all the instances between tests. Not resetting them caused random failures because every login in every test was hitting those spam prevention middlewares and somehow left a trace of that in those instances (even when the brute table is reset). Maybe those instances were doing some in memory caching.
no issue
- All content-length snapshots should be using the same matcher for consistency - anyContentLength. It's more explicit about what the matcher is all about and might be useful to have content-length matchers in one place if it ever changes (the header value should be a damn digit after all, not a string!) (ref. https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2)
refs https://github.com/TryGhost/Team/issues/2024
Without validation it was possible to send a string of comma separated
email addresses to the endpoint, and an email would be sent to each
address, bypassing any rate limiting.
This bug does not allow for an authentication bypass exploit. It is purely a
spam email concern.
Credit: Sandip Maity <maitysandip925@gmail.com>
no issue
- The milliseconds configuration here is different to "seconds" used in the max-age header value itself and other middlewares (like CORS). It's not going to be fixed upstream, so whenever this piece of code is touched again would be smart to get our own converter from seconds to milliseconds going, or some other mechanism making max-age configuration uniform across codebase
refs https://github.com/TryGhost/Toolbox/issues/320
- The URL matcher is very likely to be reused in the future, so having it abstracted away gives two benefits:
1. Central place to document hacky behavior and easier future cleanup
2. The implementer of the e2e test does not have to see the "hacky note" and just concentrate on the implementation of the test
refs https://github.com/TryGhost/Toolbox/issues/320
- Header snapshot matching was missing from webhook e2e tests. With a bumped version of webhook-mock-receiver it's now possible to record and match webhook request headers.
refs https://github.com/TryGhost/Toolbox/issues/426
- with the existing `ship` command, it publishes to npm before pushing
to GitHub
- I fear we're likely to run into a scenario where the HEAD of `main` is
behind, so the git push fails, and then rebasing the HEAD will break
the tags that we created
- in this scenario, I think it's better to hard reset back, git pull and
then try again
- in that case, we need to publish to npm after GitHub
- this commit should implement that
refs https://github.com/TryGhost/Toolbox/issues/426
- now Portal is in the monorepo, the dev script doesn't need to wipe the
console output nor print instructions as that should be handled with
setting environment variables
- these changes were needed ahead of the upcoming bump to react-scripts,
which bumps eslint to v8, and therefore we can update
eslint-plugin-ghost, which introduces some new linting rules
refs e0430b4efc
- acceptance test had mixed up name/slug fields so was testing for incorrect values now that cmd+s inside the slug field is working as really expected
no issue
- swapped from route actions triggered by shortcuts mixin to explicit `{{on-key}}` actions
- when saved via keyboard, blur any focused element to trigger it's on-blur action and schedule the save to run after those actions
refs https://github.com/TryGhost/Ghost/issues/14101
Twitter/facebook URL validation doesn't follow our typical validation and was duplicated across multiple screens making the controllers unnecessarily complex.
- extracted url input fields and their validation into separate components
- uses tracked scratch values so that the input field values can reset to the saved value on save
- twitter/facebook URL inputs are different to our other inputs because invalid values won't prevent saving, instead they are reset to their previous value on save
- added `this.validate()` call after a successful save in `settings` service so the service and underlying model validations are both in sync (fixes validation error sticking around after saving with invalid twitter/facebook values that have been reset)
no issue
- reset logic was incorrect meaning that if you focused a twitter/facebook URL field then moved to a different field without inputting anything an error would be thrown from trying to use `.match()` on `null`
refs https://github.com/TryGhost/Team/issues/2019
- the anchor chart keeps showing the loading spinner for a site that has no paid tiers and the source attribution flag switched on.
- this was because it tries to load the the MRR chart by default, which doesn't has any data when paid tiers are disabled.
- updated the chart to use `total members` data when paid tiers is disabled
refs https://github.com/TryGhost/Team/issues/2019
- the anchor chart keeps showing the loading spinner for a site that has no paid tiers and the source attribution flag switched on.
- this was because it tries to load the the MRR chart by default, which doesn't has any data when paid tiers are disabled.
- updated the chart to use `total members` data when paid tiers is disabled
* 🐛 Fixed broken `close` button on modals
no issue
Some modals' close elements containing `{{on "click" @close}}` and `href=""` would not take the 'on cl
ick' function into account, meaning the whole page would get redirected to "/" (back to the root of the dashboard) instead of just closing the modal.
refs 85cce39af7
- removed out of date transfer owner modal integration test
- fixed missing `?` in optional chaining in `<GhImageUploader>` error handling
closes: https://github.com/TryGhost/Ghost/issues/15506
- The sidebar items that don't open the editor were missing some relative current-when attributes so the nested views weren't considered "active".
We're planning to change this from a warning to an error and need to
clean the codebase up before we do so.
In all of these cases the shadowing was known about and was not
causing unexpected behaviour, so the refactor consists entirely of
renaming, rather than refactoring/bug fixes.
refs 568e4183e3
refs 258f56ded9
- when in test environment add a `X-Test-User` header to API requests that allows the mirage endpoints to check the logged in user without having to cross boundaries into the application or test contexts
refs 258f56ded9
- `@ember/test-helpers` is not included outside of test builds but we load mirage in development too so this was causing import errors that completely broke the app in development
- commented out the checks for now to get development working again whilst we look for alternatives to `getContext()`
