refs https://github.com/TryGhost/Team/issues/588
refs 6a1e722648
- date-fns proved to be unable to manipulate dates in consistent UTC format and was substitured with luxon in referenced commit. Removing it from tests for consistency
refs https://github.com/TryGhost/Team/issues/588
- date-fns proved to be unable to manipulate dates consistently in UTC timezone. Keeping all calculations and formatting in UTC is key to have consistency in dates when dealing in inter-system dates
- day.js also failed the test for correct UTC manipulation. See https://github.com/iamkun/dayjs/issues/1271 for example bug which prevents from consistent correct calculation
- luxon was the best option which WORKED. It's also a recommended successor for moment.js with really nice docs and active support
refs https://github.com/TryGhost/Team/issues/588
- This is a basic implementation which needs a review. Implemented it to fix failing tests in main
- Start date is expected to come formatted for DB's needs
no issue
- `Error` is very generic for this case and `IncorrectUsageError`
will populate the resulting error with the correct error code
- the `message` was pulled out to its own statement so we can avoid long
lines
no issue
- we're preparing the `package-json` lib to be extracted out of Ghost into
its own package so moving the initialization wrapper outside of the
folder makes the process a lot easier
refs https://github.com/TryGhost/Team/issues/588
- The limit service can now be initialized with a config which has a 'maxPeriodic' key identifying it's a special type of limit taking subscription cycles into account
- Example configuration can be found in the included unit tests
refs https://github.com/TryGhost/Team/issues/588
- There's a need to calculate when the last period has started to be able to generate correct counting queries for the "maxPeriodical" limit
- It operatest on ISO strings as an input and output in UTC timezone to take timezone calculations out of the equation
- Refer to inclucded unit tests for example calculations
refs https://github.com/TryGhost/Team/issues/588
- This is a scaffolding for a new limit type which should allow to check limits based on periods (for example related to billing, subscription cycles)
refs 90ca836cb6
- i18n is used everywhere but only requires shared or external packages, therefore it's a good candidate for living in shared
- this reduces invalid requires across frontend and server, and lets us use it everywhere until we come up with a better option
- Having these as destructured from the same package is hindering refactoring now
- Events should really only ever be used server-side
- i18n should be a shared module for now so it can be used everywhere until we figure out something better
- Having them seperate also allows us to lint them properly
refs https://github.com/TryGhost/Team/issues/588
- This is by no means an thorought test coverage but ensures the basics work and provides examples of how the limit should be used. To be continued :)
refs https://github.com/TryGhost/Team/issues/588
- This is a step 1 in the introduction of email limits. Next step would be allowing this limit to support "periodical limit checks"
no issue
- I was a little confused seeing an empty object in the config moduele - `customThemes: {}` and initially thought we could get rid of it to reduce the amount of code. Afte quick dig found out that there's a purpuse behind it being there! It's an allowlist of the properites that can be defined within the limit service
- Added notes to clarify the usecase and avoid ambiguity in the future
no issue
- later updates of this package contain different types that we haven't
changed our code for yet, so I need to revert the pinning to force
this specific version for now
no issue
- `extract-zip` v2.0.1 currently requires Node 10.17.0, which we're not
ready to bump our minimums to
- we'll probably bump this when we drop Node 10 at the end of April 2021
no issue
- this Utils repo contains libraries, whose dependencies should not be
pinned in order to reduce multiple versions of the same package
appearing for consumers
refs https://github.com/TryGhost/Team/issues/599
- There are cases when there'a a need to reload limits with a new set of configuration. For example, when Ghost is run in a test environment is a soft reboot is done
- Resetting previous value of limits avoids having conflicting state after multiple calls
refs https://github.com/TryGhost/Team/issues/510
- Flag limits are impossible to check if they are "over a limit already" as they are just that - on/off flags. Therefore it should be directly noted that the method is there to keep the "Limit" interface and not be relied upon
refs https://github.com/TryGhost/Team/issues/587
- Improved description and provided example use of error message template variables that are available for "MaxLimit" types of limits
no issue
- These files kept generating a new ouput when trying to publish an unrelated package. Assuming the generation is orrect and commiting this just to get things out of the way (type definition files should not break any functionality)
refs https://github.com/TryGhost/Team/issues/510
- {{max}} and {{count}} variable usage was not covered but had valid usecases in the library client's, so considered to "document" them through tests
- For more context these variables are available in custom `error` templates that are provided with each limit
refs https://github.com/TryGhost/Team/issues/597
- When the library is used on a client without a DB connection (e.g. frontend client running in a browser) the library needs to expose a way to override count queries.
- The way these can be used is giving a count based on a HTTP request or some other data provider
- Example use with max limit like "staff" would be loading the limit servcie if following way:
```
const limitService = new LimitService();
let limits = {
staff: {
max: 2,
currentCountQuery: () => 5
}
};
limitService.loadLimits({limits, errors});
await limitService.checkIsOverLimit('staff')
```
refs https://github.com/TryGhost/Team/issues/597
- To be able to transpile the library for different runtimes (make it polymorphic) had to get rid of dependencies that were not compatible with ES Modules
- By making errors an injectable constructor option it removes the depencency and allows to transpile the library for multiple targets
- The `errors` option is now a required parameter for `loadLimits` method. It errors if it's missing (error message copy inspired by content api error 69fcea0582/packages/content-api/lib/index.js (L21))
refs https://github.com/TryGhost/Team/issues/597
- Before adding more parameters documented existing ones
- Created LimitConfig type definition to have easier look into the structure of limit conifiguration
refs https://github.com/TryGhost/Team/issues/587
- Documented common usecases such as:
1. initialization and configuration of limit service
2. usage of "max" types of limits
refs https://github.com/TryGhost/Team/issues/587
- There was no test coverage for MaxLimit's errorIfIsOverLimit check. Added basic test to make sure upcoming modifications don't break existing functionality
refs https://github.com/TryGhost/Team/issues/587
- The optional {max} passed as an option allows to override currently configured limit and do a theoretical new limit check. For example: check if the max limit will be exceeded if the limit changes (user changes plans)
refs https://github.com/TryGhost/Team/issues/587
- Having a JSDoc gives better intellisense when the class is instantiated and provides clues about what each parameter might be used for
refs https://github.com/TryGhost/Team/issues/587
- When the 'max' configuration is missing the instance of the class breaks when used unexpectedly. Followed similar approach to currentCountQuery check by failing fast in the constructor
refs https://github.com/TryGhost/Team/issues/587
- Test were missing for class initialization and around how the limit currently works.
- Before extending it's behavior throught its valuable to cover current functionality to not accidentally break anything
refs: https://github.com/TryGhost/Team/issues/510
- Ghost config always uses camelcase. This was incorrectly implemented with snake case originally
- Swap to use camelCase by default, which is desirable, but support both
- It's really easy to support both in the loader and isLimited check, so we do this to stop ourselves tripping on this later
refs: https://github.com/TryGhost/Team/issues/510
- we need to make sure we take into account any invites that could be accepted at any time
- this counts all invites for non-contributor roles as well as all users who aren't contributors
- this should stop there being loop holes to inviting staff users
refs: https://github.com/lodash/lodash/issues/705
- Was seeing unexpected token = errors when using lodash templates in Ghost
- This is because we're setting template settings globally in this dependency and it affects every other user of lodash
- Using runInContext keeps this templateSettings change local to this lib
- Test proves that after requiring limits we can require lodash and have the default values again
refs https://github.com/TryGhost/Ghost/issues/12496
- `workerMessageHandler` option allows for custom worker message handling and allows to eliminate a need for loggers of any type inside of jobs.
- removing loggers from jobs solves file hanle leak which used to cause Ghost process to crash (see referenced issue)
refs #122
- In future changes there's a plan to add "inline" scheduled jobs, which would conflict current method naming.
- The amount of parameters in the methods was more than 3, so it made sense to transform them into an options object
- Scheduling still doesn't work for "inline" jobs. This should be solved as a part of upstream library (https://github.com/breejs/bree/issues/68)
closes https://github.com/TryGhost/Ghost-Utils/issues/118
- Custom error handling is needed to be able to override default bree
error handling logic.
- bree bump to 4.1.0 also fixed logging errors (object Object fix in
tests)
- The handler function receives two parameters. First contains an error
that has been thrown by the job. Second, job and worker metadata
no issue
- Provided more context about which type of job does what and introduced naming to be able to distinguish them
- The naming is still to be reviewed by peers
closes#117
- Having immediately executable offloaded jobs is necessary to be able to run usecases like: send batched emails now, or any other job that does not need to be scheduled
- Changed "simple" job timeout to make tests run faster
closes#119
- A future use-case which this feature caters for is allowing to migrate "post scheduler" to use job manager instead of managing scheduling itself
- removeJob method will be needed to allow "rescheduling" of the post
- this helps simplify the code and gets rid of Promise chaining
- apparently I can't easily use an async function within filter, so I've
left it for now
- this helps bring all the code together so we can extract it in the
future
- turning it into a class also lets us easily inject the i18n instance
and store it locally
refs https://github.com/TryGhost/Ghost/issues/12402
- Describes different types of jobs that could be executed depending on the nature of the jobs.
- Lays down ground rules on how to design scheduled jobs
no issue
- When providing a crontab schedule expression it should always contain 6 elements first one of them being a "seconds" schedule description . For example: '0/5 * * * * *' - meaning to run every 5 seconds
no issue
- When jobs are performing CPU intensive tasks they block main process'
event loop. They also can cause memory leaks or unexpected crashes
effectively crashing the parent proccess. To address these issues jobs need to be performed off of main
process. Worker Threads (https://nodejs.org/dist/latest-v12.x/docs/api/worker_threads.html)
are the best candidate for such work.
- These changes introduce an integration on top of bree
(https://github.com/breejs/bree/) which allows to run recurring
jobs in worker thereads. It falls back to child process execution for
Node v10 running without `--experimental-worker` flag.
- bree was chosen not only because it gives a polyfill for older Node
versions. It has support for some of the future use-cases Ghost is looking to
implement, like scheduled jobs.
- This changeset also includes a complete example of job running on an
interval with a possibility for graceful shutdown
no issue
- This is a quick implementation change to prevent from queue stalling and never becoming idle in case there's an error thrown from within the job function/module
- More robust error handling should be designed soon!
no issue
- Jobs should not always be functions, one of the standard practices is having a job defined in a module as a self contained executable function
- First parameter of `addJob` function now also handles path to modules which it imports and executes
no issue
- Accepting the schedule or a data when scheduled job should be run would follow a signature used in other established frameworks: (1) https://github.com/mperham/sidekiq/wiki/Ent-Periodic-Jobs#definition
- Another reason to put scheduling parameter first is this would allow leaving an optional "data" parameter as last
