As per our architecture guidelines we want to keep bookshelf implementations of
Repositories in Ghost core, so that all the bookshelf code is kept together, and
the packages implementing business logic with entities and services require less
dependencies to test. This separation should also help us inadvertently add
business logic to repository implementations by having a more "physical"
boundary between them.
I don't think we need to instantiate and use the `equals` method here. It adds
an extra dependency to the Repository implementation, but it is slightly more
"correct" as it means that we leak internals by comparing strings exactly?
The ValueObject pattern here was very much a trial and isn't something we're
necessarily sticking with, so I don't see a problem with this string comparison.
We do not want our Repositories to implement business logic like deciding which
events should be created. As that creates too tight of a coupling, and makes it
harder to swap out repositories in future. Instead they should only be concerned
with the storage, retrieval and hydration of Entities from the persistence engine.
fixes https://github.com/TryGhost/Product/issues/3728
- When importing members from Stripe with an existing offer, that didn't
exist in Ghost, the offer never got linked with the imported
subscription because of a missing return statement.
- Fixes importing offers with duplicate names
- Added E2E tests for creating members from a Stripe Customer ID
closes https://github.com/TryGhost/Product/issues/3595
- when importing paid members with a coupon in Stripe, we currently
search for the corresponding offer in our database and attach it to the
subscription if found. However, if an offer doesn't exist in the
database, we do not create one and don't attach any offer to the
subscription
- with this change, we now support the creation of a new offer, based on
a Stripe coupon, if it didn't exist already
As discussed with the product team we want to enforce kebab-case file names for
all files, with the exception of files which export a single class, in which
case they should be PascalCase and reflect the class which they export.
This will help find classes faster, and should push better naming for them too.
Some files and packages have been excluded from this linting, specifically when
a library or framework depends on the naming of a file for the functionality
e.g. Ember, knex-migrator, adapter-manager
- without this, the model doesn't have the context on who was
adding/editing it
- this resulted in being unable to store actions for Offers because the
`actor` is unknown
- this is the pattern we use elsewhere in the code so I've copied it
into here
refs https://github.com/TryGhost/Team/issues/1726
- updates offer setup to allow new `trial` as discount type, was prev only `fixed` and `percent`
- updates offer setup to allow `amount` as free trial days value
- updates offer setup to allow `trial` as discount duration value for trial offers, was prev only `once`/`forever`/`repeating`
refs https://github.com/TryGhost/Ghost/pull/14488
**Bug:**
When creating a new offer, sometimes no discount object is created in Stripe (only in MySQL). This issue causes tests to fail in https://github.com/TryGhost/Ghost/pull/14488
**Cause:**
When creating an offer with the OffersAPI, we create a new transaction to save the offer and check for uniqueness. Inside that transaction, we dispatch an OfferCreatedEvent event before the transaction is committed to the database.
When listening for this event in the `members-payments` package, we create the discount object in Stripe and save the stripe_coupon_id to the newly created offer. But because (in most cases) the offer doesn't yet exist in the database, that listener fails without warning and exits early.
**Solution:**
Creating a discount in Stripe should only happen after we succesfully committed the offer to the database. If we don't wait for the commit, the offer often (by chance) won't exist in the database, so the coupon won't be created inside Stripe. The solution is to wait for the transaction to succeed/be committed before dispatching the event.
refs https://github.com/TryGhost/Team/issues/1519
- Removed offer metadata again from offer (added in one of the previous commits)
- Added `getByStripeCouponId` method in offer repository (required to find an offer based on the stripe_coupon_id)
- Match discounts from Stripe based on the stripe_coupon_id instead of metadata
no-issue
When attempting to read a non-existent offer we were running into issues
with calling toJSON() on `null`. This updates the handling to explicitly
return null - so that the controller can correctly throw a NotFoundError
refs https://github.com/TryGhost/Team/issues/1163
This allows users to not provide a title for an Offer. We store the lack
of a title as `NULL` in the DB, but we will always provide a string to
the API so that the title can safely be used in HTML.
refs https://github.com/TryGhost/Team/issues/1166
Since we removed the creation of coupons from the Offers module, we must
emit events so that the Payments module can handle creating Coupons when
Offers are created.
We also export the events from the module so that they can be listened
to by the Payments module.
We also export other internals of the module so that the types can be
used.
refs https://github.com/TryGhost/Team/issues/1166
This will be handled by a payments module instead. In order to
disconnect Stripe we must delete all Stripe related data, which means an
Offer doesn't inherently have a stripe coupon id. Instead we can use a
payments service which will get/create the coupon for us when we need
it.
no-issue
Because we were checking for truthyness rather than existence when
updating properties on an Offer - it was impossible to set the
description to a blank string, as this is falsy.
refs https://github.com/TryGhost/Members/commit/5172e40646
When we updated to use the OffersAPI instead of OfferRepository this was
missed, and we were passing blank coupon to Stripe Checkout. This should
eventually be replaced with a call like `getCoupon(offerId)` from a
payments service.
refs https://github.com/TryGhost/Team/issues/1131
This adds a mapping between the status property used in the domain & API
and the active column used in the database. As we only have the usecase
of filtering by `status` right now, we have not added support for all
the other columns. Instead of these potentially erroring where the
column name does not match the property name in the domain/api - we've
added a transformer which will ignore all filters for properties other
than `status`. This follows postels law, in that we can be liberal with
the filters we accept, but conservative in the ones we implement.
refs https://github.com/TryGhost/Team/issues/1131
- Includes `status` on OfferDTO so client can use it
- Allows editing `status` of Offers
- Allows setting initial `status` when creating Offers
no-issue
This simplifies the handling of updating redirects for a code, and
doesn't affect our application layer because we never have the need to
change a code twice.
In future this should be replaced with events at the domain level - so
that we do not have to track changed properties and instead a redirect
service can listen to events, which would be dispatched on a successful
save by the repository.
