Commit Graph

2547 Commits

Author SHA1 Message Date
kirrg001
355ef54702 Removed isNew usages in model layer
no issue

- `isNew` does not work in Ghost, because Ghost does not use auto increment id's
- see https://github.com/bookshelf/bookshelf/issues/1265
- see https://github.com/bookshelf/bookshelf/blob/0.10.3/src/base/model.js#L211
- we only had one occurance, which was anyway redundant
  - if you add a user, `hasChanged('password') is true
  - if you edit a user and the password has changed, `hasChanged('password')` is true as well

NOTE #1:

1. We can't override `isNew` and throw an error, because bookshelf makes use of `isNew` as well, but it's a fallback if `options.method` is not set.
2. It's hard to re-implement `isNew` based on `options.method`, because then we need to ensure that this value is always set (requires a couple of changes)

NOTE #2:
If we need to differentiate if a model is new or edited, we should manually check for `options.method === insert`.

NOTE #3:
The unit tests are much faster compared to the model integration tests.
I did a comparision with the same test assertion:
  - unit test takes 70ms
  - integration test takes 190ms
2018-02-15 22:11:49 +01:00
kirrg001
2b76d7a492 Added lib.security.password lib
no issue

- move password hashing and password comparison to lib/security/password
- added two unit test
- FYI: password hashing takes ~100ms
  - we could probably mock password hashing in certain cases when unit testing
2018-02-15 21:13:04 +01:00
Katharina Irrgang
c6a95c6478
Sorted out the mixed usages of include and withRelated (#9425)
no issue

- this commit cleans up the usages of `include` and `withRelated`.

### API layer (`include`)
- as request parameter e.g. `?include=roles,tags`
- as theme API parameter e.g. `{{get .... include="author"}}`
- as internal API access e.g. `api.posts.browse({include: 'author,tags'})`
- the `include` notation is more readable than `withRelated`
- and it allows us to use a different easier format (comma separated list)
- the API utility transforms these more readable properties into model style (or into Ghost style)

### Model access (`withRelated`)
- e.g. `models.Post.findPage({withRelated: ['tags']})`
- driven by bookshelf

---

Commits explained.

* Reorder the usage of `convertOptions`

- 1. validation
- 2. options convertion
- 3. permissions
- the reason is simple, the permission layer access the model layer
  - we have to prepare the options before talking to the model layer
- added `convertOptions` where it was missed (not required, but for consistency reasons)

* Use `withRelated` when accessing the model layer and use `include` when accessing the API layer

* Change `convertOptions` API utiliy

- API Usage
  - ghost.api(..., {include: 'tags,authors'})
  - `include` should only be used when calling the API (either via request or via manual usage)
  - `include` is only for readability and easier format
- Ghost (Model Layer Usage)
  - models.Post.findOne(..., {withRelated: ['tags', 'authors']})
  - should only use `withRelated`
  - model layer cannot read 'tags,authors`
  - model layer has no idea what `include` means, speaks a different language
  - `withRelated` is bookshelf
  - internal usage

* include-count plugin: use `withRelated` instead of `include`

- imagine you outsource this plugin to git and publish it to npm
- `include` is an unknown option in bookshelf

* Updated `permittedOptions` in base model

- `include` is no longer a known option

* Remove all occurances of `include` in the model layer

* Extend `filterOptions` base function

- this function should be called as first action
- we clone the unfiltered options
- check if you are using `include` (this is a protection which could help us in the beginning)
- check for permitted and (later on default `withRelated`) options
- the usage is coming in next commit

* Ensure we call `filterOptions` as first action

- use `ghostBookshelf.Model.filterOptions` as first action
- consistent naming pattern for incoming options: `unfilteredOptions`
- re-added allowed options for `toJSON`
- one unsolved architecture problem:
  - if you override a function e.g. `edit`
  - then you should call `filterOptions` as first action
  - the base implementation of e.g. `edit` will call it again
  - future improvement

* Removed `findOne` from Invite model

- no longer needed, the base implementation is the same
2018-02-15 10:53:53 +01:00
Hannah Wolfe
fe0197b226 🐛Fixed {{get}} helper's date comparison (#9454)
no issue

- Date comparisons are possible via API, but there's no way to inject a valid date into the get helper
- JavaScript's Date.toString() function outputs dates in a useless format
- Swap to using Date.toISOString() and now the format can be understood anywhere!
- {{#get "posts" filter="published_at:<='{{published_at}}'"}}{{/get}} works now as expected
2018-02-14 18:33:07 +01:00
Katharina Irrgang
9ede5905f6
Reduce toJSON implementation: use the power of bookshelf (#9423)
refs #6103

- simplify `toJSON`
- `baseKey` was not used - have not find a single use case
- all the functionality of our `toJSON` is offered in bookshelf
- `omitPivot` does remove pivot elements from the JSON obj (bookshelf feature)
- `shallow` allows you to not return relations
- make use of `serialize`, see http://bookshelfjs.org/docs/src_base_model.js.html#line260
- fetching nested relations e.g. `users.roles` still works (unrelated to this refactoring)

> pick('shallow', 'baseKey', 'include', 'context')

We will re-add options validation in https://github.com/TryGhost/Ghost/pull/9427, but then with the official way: use `filterOptions`.

---

We return all fetched relations (pre-defined with `withRelated`) by default.
You can disable it with `shallow:true`.
2018-02-14 17:32:11 +01:00
Katharina Irrgang
58157b1411
🐛Fixed asset redirects (#9455)
closes #9445

- redirects all asset requests if https is configured (theme, core, images)
- re-use and extend our url-redirect middleware
- add proper integration tests for our express site app (no db interaction, component testing required for such important use cases)
- i added some more general tests
- should avoid mixed content warnings in the browser
2018-02-14 17:21:31 +01:00
kirrg001
ed4fde4f00 🐛 Fixed migrating from < 1.13 to 1.21
no issue

- discovered while testing
- the fixture utility needed a protection against non existent roles in the database
  - it tries to fetch the contributor role from the database, which does not exist yet
2018-02-07 12:31:21 +01:00
Austin Burdine
777247cbc7 Contributor Role (#9315)
closes #9314 

* added fixtures for contributor role
* update post api tests to prevent contributor publishing post
* update permissible function in role/user model
* fix additional author code in invites
* update contributor role migration for knex-migrator v3
* fix paths in contrib migration
* ensure contributors can't edit or delete published posts, fix routing tests [ci skip]
* update db fixtures hash
* strip tags from post if contributor
* cleanup post permissible function
* excludedAttrs to ignore tag updates for now (might be removed later)
* ensure contributors can't edit another's post
* migration script for 1.21
2018-02-07 10:46:22 +01:00
Katharina Irrgang
a274d61a8c Removed html usage in error messages (#9444)
no issue

- all of the error message keys were unused
- the only html anchor i found was for mail, but this doesn't change anything, because the admin does only show the message and not the context at the moment
2018-02-07 09:35:48 +01:00
Kevin Ansfield
fb973dbbf2 Fixed missing export of card-markdown card and broken tests
no issue
- fixes rendering bug introduced in 0833b28557
- updates test generators/fixtures to use new card names
2018-02-01 16:26:56 +01:00
Kevin Ansfield
05bcf7ee6a Fixed missing export of card-markdown card
no issue
- fixes the bug introduced in 0833b28557
2018-02-01 15:50:43 +01:00
Kevin Ansfield
0833b28557 Koenig - Rename server-side cards
refs https://github.com/TryGhost/Ghost/issues/9311
- match card names to the new generic Koenig card names introduced in 95a068615d
2018-02-01 12:40:49 +01:00
Chuck Lam
ffc6088c7a Fixed comment in facebook_url helper (#9430)
no issue
2018-01-28 18:25:06 +01:00
kirrg001
39ee95cc07 Make use of ES6 arrow functions in our data importer
no issue

- reduces the usage of `self`
2018-01-28 15:48:24 +01:00
kirrg001
2a10c83d92 Improved memory usage in importer
no issue

- returning and remembering the data, which was imported, is...
  - not required when using the API
  - not required when importing via script
  - required for tests
  - added an option to have control over it
- make more usage of local variables
  - the GC cannot tidy up variables, which are defined outside of a loop, but used in the loop
- try to keep less memory in process
  - reduce the number of properties we have to remember
2018-01-28 14:26:38 +01:00
kirrg001
5f9c3b92bd Improved Base importer constructor readability
no issue

- better differentiation between options and data
- better readability how to access required data from file
2018-01-28 13:35:21 +01:00
kirrg001
82bb3aaea1 Do not import unknown author id, fallback to owner id
no issue

- if you import a JSON file with a post, which has an unknown author,
  the target user was removed from the blog
- Ghost can handle this case and still succeeds with import
  - but we have stored an `author_id` in the database, which does not map to any user and won't map in the future
- this can trouble if we add support for multiple authors
  - currently, we only return the `author_id` to the client and the client can map with `author_id` with users fetched by the API
    - if it does not find a user, it just falls back to a different user
  - but multiple authors have to be included explicit (`include=authors`) and we will return a mapped (author_id => user) result
  - it won't be able to find the user, because we lookup the database
  - this would result in an error
- there is in general no reason to import (or store) an unknown/invalid `author_id` into the database
- on import, we show you a warning and you can choose a different author if you want
- solution: fallback to owner user and extend warning
  - it's not a behaviour change, you still can import unknown author id's and the import won't fail
  - but we ensure valid author id's
- updated test
- further more: returning `author={}` when requesting `include=author` could trouble with ember currently
  - it expects the author to be returned
2018-01-27 12:54:36 +01:00
kirrg001
c6f30a46d2 Avoid knex warning when destroying a user
no issue

- the warning is "Transaction was already complete"
- destroying a user happens in a transaction, but the event is not asynchronous
  - so we have to ensure that we don't operate on a finished transaction
2018-01-27 12:31:51 +01:00
kirrg001
b4f355f713 Removed the usages of this.forge(null, {context: options.context})
no issue

- refs fe461da110
- the access plugin was removed
- no need to pass `context` as parameter for `.forge`
2018-01-26 00:35:39 +01:00
kirrg001
fe461da110 Deleted bookshelf access plugin
refs #9127

- permission checks can happen everywhere in the code base
  - we would like to create a context class
- global access to `options.context.is(...)`
- please read more about the access plugin in #9127 section "Model layer and the access plugin".
- removed the plugin and use direct context checks
2018-01-25 17:54:28 +01:00
Kevin Ansfield
69d5fac61e
Resurrect the old alpha Koenig editor (#9277)
requires https://github.com/TryGhost/Ghost-Admin/pull/916
- add "enableDeveloperExperiments" config flag
- allow any HTML payload through in the HTML mobiledoc card
  - same approach as taken in the markdown card, running the markup through SimpleDOM isn't necessary and is prone to breaking because of it's limited parsing and error handling abilities

To use Koenig modify your `config.development.json` file and add the following flag to the top-level object:

```
"enableDeveloperExperiments": true
```

If you restart the dev server you will then see a new section on the Labs screen with a Koenig Editor checkbox to enable/disable the editor.

⚠️ The editor is in a _very_ broken state, it's there for developer testing and on-going development. _Do not_ try to use this on any production data!
2018-01-18 15:43:22 +00:00
kirrg001
357ea3dffd 🐛 Fixed showing old release notifications in the about page
no issue

- reported in slack (https://ghost.slack.com/files/U8QV8DXQB/F8TSBQ532/image.png)
- do not expose old release notification
  - e.g. you are on 1.20.0
  - you receive a notification for 1.20.1 to update
  - you update to 1.20.1
- ensure we protect exposing the release notification (compare against blog version)
- protect against wrong formats
- @TODO: the notifications could store a `version` property
  - by that we could use `notification.version` and don't have to match the version in the message
2018-01-18 12:19:55 +01:00
Peter Garland
35fbba3bcb Fixed bad references to common.i18n (#9402)
no issue

- happened during refactoring (search+replace)
2018-01-17 17:28:42 +01:00
Katharina Irrgang
c062114e47
Increased cache-control for ghost-sdk.min.js and ghost.min.css (#9399)
no issue

- we increase the client in-memory expiry for production built assets
- as soon as there will be another release, a new asset hash is generated and the client cache is invalidated automatically (doesn't matter how long we store the file in the client)
- the next step is to get rid of having asset hashs part as query params
  - ghost-sdk.min.js?v=1234 is becoming e.g. ghost-sdk-1234.min.js
  - reasons:
    - A: performance tools complain about it
    - B: we no longer invalidate the asset hashs for built assets if the theme changes
2018-01-16 12:22:29 +01:00
Katharina Irrgang
e480c7c50d 🐛 Reload translations if active theme get's overridden (#9392)
no issue

- discovered while testing
  - activate theme
  - download theme
  - modify theme
  - upload theme
  - override? yes
  - translation files are not reloaded, because the database is up-to-date
- remove un-used events in theme api layer
- trigger event from theme service
2018-01-12 09:19:28 +00:00
Hugo Marisco
ccb5fd837e Fixed wrong status code for incorrect token requests (#9374)
closes #9346

- server returned 500, happened when you send an empty username/password
- return 400 instead
- error message is/was correct
2018-01-11 16:17:03 +01:00
David Wolfe
b69b9780a9 Backup endpoint improvements (#9365)
closes #9297

- backup endpoint returns JSON
- allows setting of export filename
- DRY up code
- the endpoint is not documented, no breaking change
2018-01-11 16:03:21 +01:00
kirrg001
ccb3b3de38 Fixed 1.20 migration script
no issue

- `models.Settings.destroy` only accepts one argument
2018-01-09 21:38:13 +01:00
Katharina Irrgang
5b77f052d9
Update Notification improvements (#9123)
closes #5071

- Remove hardcoded notification in admin controller
  - NOTE: update check notifications are no longer blocking the admin rendering
  - this is one of the most import changes
  - we remove the hardcoded release message
  - we also remove adding a notification manually in here, because this will work differently from now on
    -> you receive a notification (release or custom) in the update check module and this module adds the notification as is to our database

- Change default core settings keys
  - remove displayUpdateNotification
    -> this was used to store the release version number send from the UCS
    -> based on this value, Ghost creates a notification container with self defined values
    -> not needed anymore

- rename seenNotifications to notifications
  -> the new notifications key will hold both
     1. the notification from the USC
     2. the information about if a notification was seen or not
  - this key hold only one release notification
  - and n custom notifications

- Update Check Module: Request to the USC depends on the privacy configuration
  - useUpdateCheck: true -> does a checkin in the USC (exposes data)
  - useUpdateCheck: false -> does only a GET query to the USC (does not expose any data)
  - make the request handling dynamic, so it depends on the flag
  - add an extra logic to be able to define a custom USC endpoint (helpful for testing)
  - add an extra logic to be able to force the request to the service (helpful for testing)

- Update check module: re-work condition when a check should happen
  - only if the env is not correct
  - remove deprecated config.updateCheck
  - remove isPrivacyDisabled check (handled differently now, explained in last commit)

- Update check module: remove `showUpdateNotification` and readability
  - showUpdateNotification was used in the admin controller to fetch the latest release version number from the db
  - no need to check against semver in general, the USC takes care of that (no need to double check)
  - improve readability of `nextUpdateCheck` condition

- Update check module: refactor `updateCheckResponse`
  - remove db call to displayUpdateNotification, not used anymore
  - support receiving multiple custom notifications
  - support custom notification groups
  - the default group is `all` - this will always be consumed
  - groups can be extended via config e.g. `notificationGroups: ['migration']`

- Update check module: refactor createCustomNotification helper
  - get rid of taking over notification duplication handling (this is not the task of the update check module)
  - ensure we have good fallback values for non present attributes in a notification
  - get rid of semver check (happens in the USC) - could be reconsidered later if LTS is gone

- Refactor notification API
  - reason: get rid of in process notification store
    -> this was an object hold in process
    -> everything get's lost after restart
    -> not helpful anymore, because imagine the following case
      -> you get a notification
      -> you store it in process
      -> you mark this notification as seen
      -> you restart Ghost, you will receive the same notification on the next check again
      -> because we are no longer have a separate seen notifications object
  - use database settings key `notification` instead
  - refactor all api endpoints to support reading and storing into the `notifications` object
  - most important: notification deletion happens via a `seen` property (the notification get's physically deleted 3 month automatically)
    -> we have to remember a seen property, because otherwise you don't know which notification was already received/seen

- Add listener to remove seen notifications automatically after 3 month
  - i just decided for 3 month (we can decrease?)
  - at the end it doesn't really matter, as long as the windows is not tooooo short
  - listen on updates for the notifications settings
  - check if notification was seen and is older than 3 month
  - ignore release notification

- Updated our privacy document
- Updated docs.ghost.org for privacy config behaviour
- contains a migration script to remove old settings keys
2018-01-09 15:20:00 +01:00
juan-g
f671f9d2c9 Theme translations and blog localisation (#8437)
refs #5345, refs #3801

- Blog localisation
  - default is `en` (English)
  - you can change the language code in the admin panel, see https://github.com/TryGhost/Ghost-Admin/pull/703
  - blog behaviour changes depending on the language e.g. date helper format
  - theme translation get's loaded if available depending on the language setting
  - falls back to english if not available

- Theme translation
  - complete automatic translation of Ghost's frontend for site visitors (themes, etc.), to quickly deploy a site in a non-English language
  - added {{t}} and {{lang}} helper
  - no backend or admin panel translations (!)
  - easily readable translation keys - very simple translation
  - server restart required when adding new language files or changing existing files in the theme
  - no language code validation for now (will be added soon)
  - a full theme translation requires to translate Ghost core templates (e.g. subscriber form)
  - when activating a different theme, theme translations are auto re-loaded
  - when switching language of blog, theme translations are auto re-loaded

- Bump gscan to version 1.3.0 to support more known helpers

**Documentation can be found at https://themes.ghost.org/v1.20.0/docs/i18n.**
2018-01-09 14:50:57 +01:00
Katharina Irrgang
dcb2aa9ad4
Improved forwarding model options in the API layer (#9380)
no issue

- our API layer uses a unit to combine incoming data and options
- e.g. `options.data` is the end result
- we have to take care that we don't pass data into the model layer

Credits: Olivier Arteau
2018-01-09 13:26:09 +01:00
Aileen Nowak
430973b85b 🐛 Fixed date helper timezone bug
closes #9381

Fixes a bug where the date helper would ignore any timezone settings, when called with a specific date option, e. g. `published_at`, as `timezone` was only ever assigned when called without options.
2018-01-09 13:05:54 +01:00
Aileen Nowak
b99d3bc70a Revert "🐛 Fixed date helper timezone bug (#9382)"
This reverts commit c3b0219d70.
2018-01-09 13:05:54 +01:00
汪磊
c3b0219d70 🐛 Fixed date helper timezone bug (#9382)
closes #9381

Fixes a bug where the date helper would ignore any timezone settings, when called with a specific date option, e. g. `published_at`.
2018-01-09 17:44:01 +07:00
kirrg001
a1393762f6 Lazily require modules in lib/common/index
no issue

- decreases chance of not-loaded modules or circular dependencies
- e.g. the i18n implementation will use the settings-cache and the settings-cache uses lib/common/events
2018-01-08 21:12:16 +01:00
Aileen Nowak
19a6c8a426 Added test for images w/o extensions for image size util (#9367)
closes #9022

Images without extensions don't need to be manipulated, as we're now reading the bytes and pass those to the `image-size` lib.

This PR adds another `user-agent` to emulate multiple browser requests, as I stumbled over an example where the image without extension is protected otherwise.

Added a test, that works with above mentioned image, but is currently mocked. Nevertheless, the image worked as a PoC, that we're able to read the bytes of an image without its extension and still return the dimensions of the image.
2018-01-04 12:03:41 +01:00
Katharina Irrgang
341f719d92
Extended ghost version utility (#9278)
refs https://github.com/TryGhost/Ghost-Release/issues/24

- differentiate between
  1. original package.json version (can contain pre and build suffix)
  2. full package.json version X.X.X-{pre} (optional)
  3. safe package.json version X.X (major+minor)
2018-01-03 20:20:18 +01:00
kirrg001
eb0a11d53a Fixed import test: post duplication detection within a file to import
no issue

- with 29e143fa9a import queries no longer run in parallel
- this commit simply adds a small code snippet to reflect the importer behaviour

1) duplicate slugs *within* a file are getting ignored
2) existing posts in the database and posts to import with the same slug, result in duplicates

Further improvements regarding duplication detection will happen via #8717.
2018-01-03 13:34:15 +00:00
kirrg001
428008e63d Revert "🐛 Fixed importer duplicate detection for posts"
refs #8717

- we decided to not changing the current importer behaviour
- no slug duplication detection means, importing posts can result in duplicates
2018-01-03 13:34:15 +00:00
kirrg001
02bd71d0f5 🐛 Fixed importer duplicate detection for posts
closes #8717

- this is now required, because we run import queries sequentiell
- this code protects two cases:
  - you have duplicate slugs in the JSON file (the first get's inserted, the second get's ignored)
  - you have an existing slug in the database and you try to import the same slug, get's ignored
2018-01-03 00:07:41 +01:00
kirrg001
29e143fa9a 🐛 Fixed large imports
closes #9348

- do not run import with `Promise.all`
- with a large import file, we run an enormous amount of queries in parallel, which does not allow Node to cleanup memory
- tested with an 13mb import file
- requires bookshelf-relations 0.1.4
2018-01-02 23:21:45 +01:00
Aileen Nowak
ae741b1a18 Removed bluebird promise wrap in request lib (#9343)
refs #9178, refs #8988

With 7353c87d7f we use Bluebird globally for Promises. Therefore, the request lib doesn't need to be wrapped in a bluebird Promise anymore.

This was originally done, so we can work with catch predicated in our image-size lib.

Updated the tests to proof, that the catch predicates work.

The tests fail, as soon as the Promise overwrite is commented out.
2018-01-02 12:18:56 +01:00
汪磊
7a98c99cba Renamed BasicErorRenderer to BasicErrorRenderer (#9358)
no issue

- typo 🙊
2018-01-02 10:33:43 +01:00
Hannah Wolfe
bcf6e9f517 Fixed error trying to render 404 for missing asset
refs #8868

- Loading the admin prior to a build results in: Failed to lookup view "error-404" in views directory
- This fixes that error, by splitting the HTMLErrorRenderer and the ThemeErrorRenderer into two separate things
2017-12-22 20:08:59 +00:00
juan-g
4cca2353e0 consistency change for english translation keys: use camel case (#9347)
no issue

- required for #8437 
- one instance of hyphenated key changed; the rest of keys in file
_core/server/translations/en.json_ are already camelCase
- also converted `common.i18n.t()` calls to this key in file
_core/server/update-check.js_
- this allows to simplify i18n to an unified use of `jsonpath`
2017-12-18 15:47:55 +01:00
Katharina Irrgang
a5af7d6384 Used request lib in xmlrpc (#9333)
refs #9178, refs https://github.com/TryGhost/Ghost/pull/8980
2017-12-15 16:50:18 +07:00
Katharina Irrgang
6e94cedfa2 Ensure cors check happens for /authentication/token route (#9317)
no issue

- otherwise external browser clients run into cors problems
2017-12-15 09:35:48 +00:00
Abijeet Patro
73c0bf554a 🐛 Fixed emoji in Slack test notification not rendered by Discord (#9313)
closes #9263

- changed emoji for Slack test notification to 😄 which will be correctly rendered as emoji by both, Slack and Discord.
2017-12-15 11:19:37 +07:00
kirrg001
991ccb1d35 Moved make-absolute-urls to url service
refs #9178

- this util uses the url services (!)
- moving this file into lib would not make sense right now
  - that would mean a module requires first ../lib/url, which then requires ../services/url
- the url service definitely need a clean up 😃
2017-12-14 22:34:37 +01:00
kirrg001
64626dedd1 Moved social utility to lib/social
refs #9178

- not 100% sure about this, but i think it makes right now the most sense
- we have already a url service and creating another lib/url is confusing at the moment
- i'll copy the last utility `makeAbsoluteUrls` to the url service for now
- see next commit for explanation (!)
2017-12-14 22:34:05 +01:00