michaelbromley Github contribution chart
michaelbromley Github Stats
michaelbromley Most Used Languages

Activity

05 Oct 2022

Michaelbromley

Feat/mollie paymentmethod selection

Allow pre-selecting a payment method before being redirected to Mollie's hosted checkout. closes #1809

Forked On 05 Oct 2022 at 03:19:03

Michaelbromley

@martijnvdbrug there were some bad imports in the admin ui, which were fixed but I hadn't pushed my latest commits to the branch. Just did that now, so you should be able to rebase onto the latest minor and force push an update to this PR 👍

Commented On 05 Oct 2022 at 03:19:03

Michaelbromley

chore(admin-ui): Fix imports

Pushed On 05 Oct 2022 at 03:18:09

Michaelbromley

docs(admin-ui): Add documentation for bulk actions

Pushed On 05 Oct 2022 at 03:18:09

Michaelbromley

feat(testing): Enable e2e test logging using the LOG env var

Pushed On 05 Oct 2022 at 03:18:09

Michaelbromley

docs: Add support for "experimental" docs tag

Pushed On 05 Oct 2022 at 03:18:09

Michaelbromley

fix(admin-ui): Fix variant editing when 2 options have same name

Fixes #1813

Pushed On 05 Oct 2022 at 03:15:48

Michaelbromley

Postgres DB index row size error on search_index_item description column index

Describe the bug

QueryFailedError: index row size 3208 exceeds btree version 4 maximum 2704 for index "IDX_9a5a6a556f75c4ac7bfdd03410"


it is: "IDX_9a5a6a556f75c4ac7bfdd03410" ON search_index_item (description); 

To Reproduce Steps to reproduce the behavior:

  1. add a long text on product description
  2. Click on admin ui re-build index
  3. fail
  4. See error

Expected behavior A clear and concise description of what you expected to happen.

Environment (please complete the following information):

  • @vendure/core version: 1.6..4
  • Database (mysql/postgres etc): postgres

Forked On 05 Oct 2022 at 01:47:21

Michaelbromley

I've spent a bit of time looking into this today.

First of all, I cannot reproduce this locally, and I'm not sure why.

I added a very long description to a product (71,330 bytes long) and had no issue updating the search index and searching. I'm not sure whether this is related to the postgres config?

I looked further into the topic of full-text searching and indexing in postgres and came across this very good article: Indexing for full text search in PostgreSQL. My take-away is that there are better ways to index the data for postgres - namely creating explicit ts vector indexes by pre-computing the tsvectors rather than converting them at search-time as the current implementation does.

However, there are a couple of issues here:

  1. I don't want to introduce a breaking change to the behaviour of the search, unless this is a big problem for lots of people. So far I've not heard any other complaints about this.
  2. The DefaultSearchPlugin is intentionally basic, and also needs to be generic enough to work across all supported databases. So postgres-specific special indexes are tough to fit into that constraint.
  3. If you want remove the index from your project, you can now use the EntityMetadataModifier API to do so in a safe manner.

For these reasons I'm going to leave this issue for now, and re-consider it at a later point or if further reports come in.

Commented On 05 Oct 2022 at 01:47:21

Michaelbromley

fix(core): persist customField relations in PromotionService

#1821

Forked On 05 Oct 2022 at 12:52:27

Michaelbromley

Good catch! Thanks for the fix! 👍

Commented On 05 Oct 2022 at 12:52:27

Michaelbromley

fix(core): Persist customField relations in PromotionService (#1822)

Pushed On 05 Oct 2022 at 12:52:06

Michaelbromley

fix(core): persist customField relations in PromotionService

Created On 05 Oct 2022 at 12:52:05

Michaelbromley

fix(admin-ui): Wrap long promotion condition/action names

Pushed On 03 Oct 2022 at 10:15:57

Michaelbromley

Make Vendure serverless proof

Is your feature request related to a problem? Please describe. Most (if not all?) serverless environments don't allow processing outside the requestcontext. Vendure's event handling has some async processes that occur outside the requestcontext. For examaple the apply-collection-filters

Event handling occurs after a response has been sent, because event.publish() is fire and forget (as it should be).

Describe the solution you'd like It would be nice if we can somehow be sure that no background tasks are running when a response has been returned.

Describe alternatives you've considered One option I see is to be able to implement our own remote eventBus strategy. We could do await eventBus.publish() and be sure that the event is published. Event handling can be done similar to how worker Job's are handled: in their own async request context if that's what's implemented in the implemented 'eventbus strategy'.

A workaround for Google Cloud Run is to enable background processes, which basically disables the serverless function and makes it a normal running container. (Resulting in a ~3x price increase)

Additional context This problem hasn't occurred yet, but if an eventhandler does resource intensive tasks that take longer than a few seconds, serverless environments would just kill the process.

This problem can occur with Google Cloud Run, Google Cloud Functions, AWS Lambda and probably similar products from other providers.

Is this something that needs to be implemented in Vendure, or is this a "won't support" feature? There is no shame in not supporting function/lambda like envs, but semi-serverless/autoscaling envs like Google Cloud Run are great platforms to save costs.

Forked On 03 Oct 2022 at 09:24:32

Michaelbromley

Thanks @skid & everyone for the feedback.

Everything that runs out-of-order wrt the request, should be, abstracted away in a task queue. If not possible - it should be "synchronous" - i.e. the request should not return until it's done. This means that events should be either awaitable or executed via a task queue. This is most likely a breaking change, but IMHO better make it early than late.

So the core issue here is that event subscribers might get killed mid-processing because the response has already been sent. This issue is also discussed here:

  • #1758

As I mention there and in the slack conversation above, I am hesitant to completely alter the observer pattern on the EventBus - and also yeah this is actually not at all possible with rxjs as it stands. But maybe there is some clever way to have a global setting like "await all subscribers before sending response", and then we do some kind of ref counting on all events published in that context, and figure out some way of waiting for the subscriber functions to return. Again, I'm not at all sure whether this is technically possible, but it is worth some research.

Or the other option is, as has been suggested, to scrap the EventBus altogether and unify all async tasks into the job queue. One major issue with that is the overhead: the default job queue will add a row on every system event (of which there are many) and not only that, will have to poll in order to handle new jobs. That introduces not only the network latency to the events system, but also the polling latency as well as increased storage and DB load.

The great thing about the rxjs observable implementation is that it is very lightweight and fast, with no external systems involved.

I'm definitely interested in solving this and having a totally solid design for serverless architectures. I'm just not yet sure what the best option is. If anyone wants to put together a POC of some new approach, I'm very happy to look at suggestions!

Vendure should significantly reduce the startup time. It takes around 20 seconds to spin up an instance, which is a problem with autoscaling. We are, right now, experiencing some issues with random spikes in response latencies, most likely connected with the fact that new instances started by GCR take too long to become operational. I can't confirm this yet, so grains of salt and all that.

I think there is a lot of work that can be done here with regard to things like deferring work until first needed. I believe @hendrik-advantitge & team have done a little research into this are for the same reason, as they are running on GCR too.

Commented On 03 Oct 2022 at 09:24:32

Michaelbromley

fix(core): Fix race condition when updating order addresses in parallel

Pushed On 03 Oct 2022 at 09:02:54

Michaelbromley

Standalone Admin UI and custom fields

Describe the bug When compiling Admin UI as standalone app as per docs, there is no way to pass the vendure config, therefore custom fields are not getting rendered in admin UI.

To Reproduce To reproduce this issue, simply create vendure admin ui app same as in docs and it will not contain custom fields in vendure config.

Expected behavior A way to pass either whole vendure config or at least custom fields to reconize them during compiling.

Environment (please complete the following information): All vendure versions

Forked On 03 Oct 2022 at 07:15:18

Michaelbromley

Hi,

The Admin UI does not require the VendureConfig to know what custom fields are defined. Instead, it makes an API call to the server, which supplies all the data on the configured custom fields, which the Admin UI then uses to display the appropriate form inputs.

So I'm not sure what the issue is that you are running into here. You can check in your network tab and see if, after logging in, the Admin UI makes an API call with the GetServerConfig query, which should return an object with

globalSettings -> serverConfig -> customFieldConfig 

which should contain the custom field config data.

Commented On 03 Oct 2022 at 07:15:18

Michaelbromley

In admin UI, unable to update product after product variant name is auto-updated when stock is 0

Hi Michael, Eric from the marlin team at IBM here. I'm hoping you can help me out with a little bug!

Describe the bug When I have a product that has a stock of 0, Upon editing a product, I am able to save updates only so long as I haven't first made an update (such as changing the product variant name) that causes the field to auto-update. After this has happened, the Update button remains disabled even after I make further changes. When I navigate away from the product, I am prompted to either stay or discard unsaved changes. When I return, none of the changes made are saved – including the automatically updated field.

https://user-images.githubusercontent.com/83984184/192352708-7625431e-5e40-444e-8ed5-aa2c1d2ffbee.mov

https://user-images.githubusercontent.com/83984184/192352732-65a6b7ba-957b-40e7-acdb-0b58f00738fd.mov

To Reproduce Steps to reproduce the behavior:

  1. Create a product with stock 0 & navigate to its product variant tab on the admin ui
  2. Attempt to edit a field such as 'price' or product variant name, that will cause the field to automatically update.
  3. You will see the 'Update' button remains disabled
  4. Return to product page and attempt to rename the product
  5. You will see the 'Update' button remains disabled
  6. Attempt to navigate away. You will be prompted to remain or discard changes.
  7. Discard changes & return to the product page. You will see none of the changes have saved.

Expected behavior When changes are made that require saving, that the Update button should be enabled.

Environment (please complete the following information):

  • @vendure/core version: v1.5.2
  • Nodejs version v16.15.1
  • Database (mysql/postgres etc): postgres

Thanks!

Forked On 30 Sep 2022 at 06:39:37

Michaelbromley

It could very well be that it has been fixed in a more recent version. I'd encourage your team to update and enjoy all the cool new features & fixes!

Commented On 30 Sep 2022 at 06:39:37

Michaelbromley

When build project, do not create /dist/ngx-pagination. Error: Uncaught (in promise): Error: NG0203

Angular version: 14

ngx-pagination version: 3.0.1

Description of issue: when i do build, i got an error NG0203. I think i'm getting this error because when i run ng build, i do not get the paste ngx-pagination in dist.

Steps to reproduce: run ng build, everything looks okay, but my web site isn't working well

Expected result: run in build the same as i'm running when i do ng serve

Actual result: getting ng0203

image

Any relevant code:

Here's my angular.json and tsconfig.json

{
  "$schema": "./node_modules/@angular/cli/lib/config/schema.json",
  "version": 1,
  "newProjectRoot": "projects",
  "projects": {
    "SUBPLE_Angular": {
      "projectType": "application",
      "schematics": {},
      "root": "",
      "sourceRoot": "src",
      "prefix": "app",
      "architect": {
        "build": {
          "builder": "@angular-devkit/build-angular:browser",
          "options": {
            "outputPath": "dist/subple-angular",
            "index": "src/index.html",
            "main": "src/main.ts",
            "polyfills": "src/polyfills.ts",
            "tsConfig": "tsconfig.app.json",
            "assets": [
              "src/favicon.ico",
              "src/assets"
            ],
            "styles": [
              "node_modules/bootstrap/dist/css/bootstrap.min.css",
              "./node_modules/@angular/material/prebuilt-themes/deeppurple-amber.css",
              "src/styles.css"
            ],
            "scripts": []
          },
          "configurations": {
            "production": {
              "budgets": [
                {
                  "type": "initial",
                  "maximumWarning": "3mb",
                  "maximumError": "4mb"
                },
                {
                  "type": "anyComponentStyle",
                  "maximumWarning": "15kb",
                  "maximumError": "16kb"
                }
              ],
              "fileReplacements": [
                {
                  "replace": "src/environments/environment.ts",
                  "with": "src/environments/environment.prod.ts"
                }
              ],
              "outputHashing": "all"
            },
            "development": {
              "tsConfig": "projects/subple-angular/tsconfig.app.json"
            }
          },
          "defaultConfiguration": "production"
        },
        "serve": {
          "builder": "@angular-devkit/build-angular:dev-server",
          "configurations": {
            "production": {
              "browserTarget": "SUBPLE_Angular:build:production"
            },
            "development": {              
              "browserTarget": "SUBPLE_Angular:build:development"
            }
          },
          "defaultConfiguration": "development"
        },
        "extract-i18n": {
          "builder": "@angular-devkit/build-angular:extract-i18n",
          "options": {
            "browserTarget": "SUBPLE_Angular:build"
          }
        },
        "test": {
          "builder": "@angular-devkit/build-angular:karma",
          "options": {
            "main": "src/test.ts",
            "polyfills": "src/polyfills.ts",
            "tsConfig": "tsconfig.spec.json",
            "karmaConfig": "karma.conf.js",
            "assets": [
              "src/favicon.ico",
              "src/assets"
            ],
            "styles": [
              "./node_modules/@angular/material/prebuilt-themes/deeppurple-amber.css",
              "src/styles.css"
            ],
            "scripts": []
          }
        }
      }
    }
  },
  "cli": {
    "analytics": "a3fa7531-b702-4c06-acb6-2c3e35a53e08"
  }
} 
/* To learn more about this file see: https://angular.io/config/tsconfig. */
{
  "compileOnSave": false,
  "compilerOptions": {
    "baseUrl": "./",
    "outDir": "./dist/out-tsc",
    "forceConsistentCasingInFileNames": true,
    "strict": true,
    "noImplicitAny": false,
    "noImplicitOverride": true,
    "noPropertyAccessFromIndexSignature": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "sourceMap": true,
    "paths": {
      "ngx-pagination": [
        "dist/ngx-pagination/ngx-pagination",
        "dist/ngx-pagination"
      ]
    },
    "declaration": false,
    "downlevelIteration": true,
    "experimentalDecorators": true,
    "moduleResolution": "node",
    "importHelpers": true,
    "target": "es2017",
    "module": "es2020",
    "lib": ["es2020", "dom"]
  },
  "typeRoots": ["node_modules/@angular/material"],
  "angularCompilerOptions": {
    "enableI18nLegacyMessageIdFormat": false,
    "strictInjectionParameters": true,
    "strictInputAccessModifiers": true,
    "strictTemplates": true
  }
} 

Forked On 30 Sep 2022 at 07:40:59

Michaelbromley

ok thanks.

I'm not sure how related to this library this issue is. Double-check you have wired it up correctly as per the docs, and then if the issue persists, provide a bit more info, i.e. show the code that you believe is responsible for triggering this error.

Commented On 30 Sep 2022 at 07:40:59

Michaelbromley

When build project, do not create /dist/ngx-pagination. Error: Uncaught (in promise): Error: NG0203

Angular version: 14

ngx-pagination version: 3.0.1

Description of issue: when i do build, i got an error NG0203. I think i'm getting this error because when i run ng build, i do not get the paste ngx-pagination in dist.

Steps to reproduce: run ng build, everything looks okay, but my web site isn't working well

Expected result: run in build the same as i'm running when i do ng serve

Actual result: getting ng0203

image

Any relevant code:

Here's my angular.json and tsconfig.json

{
  "$schema": "./node_modules/@angular/cli/lib/config/schema.json",
  "version": 1,
  "newProjectRoot": "projects",
  "projects": {
    "SUBPLE_Angular": {
      "projectType": "application",
      "schematics": {},
      "root": "",
      "sourceRoot": "src",
      "prefix": "app",
      "architect": {
        "build": {
          "builder": "@angular-devkit/build-angular:browser",
          "options": {
            "outputPath": "dist/subple-angular",
            "index": "src/index.html",
            "main": "src/main.ts",
            "polyfills": "src/polyfills.ts",
            "tsConfig": "tsconfig.app.json",
            "assets": [
              "src/favicon.ico",
              "src/assets"
            ],
            "styles": [
              "node_modules/bootstrap/dist/css/bootstrap.min.css",
              "./node_modules/@angular/material/prebuilt-themes/deeppurple-amber.css",
              "src/styles.css"
            ],
            "scripts": []
          },
          "configurations": {
            "production": {
              "budgets": [
                {
                  "type": "initial",
                  "maximumWarning": "3mb",
                  "maximumError": "4mb"
                },
                {
                  "type": "anyComponentStyle",
                  "maximumWarning": "15kb",
                  "maximumError": "16kb"
                }
              ],
              "fileReplacements": [
                {
                  "replace": "src/environments/environment.ts",
                  "with": "src/environments/environment.prod.ts"
                }
              ],
              "outputHashing": "all"
            },
            "development": {
              "tsConfig": "projects/subple-angular/tsconfig.app.json"
            }
          },
          "defaultConfiguration": "production"
        },
        "serve": {
          "builder": "@angular-devkit/build-angular:dev-server",
          "configurations": {
            "production": {
              "browserTarget": "SUBPLE_Angular:build:production"
            },
            "development": {              
              "browserTarget": "SUBPLE_Angular:build:development"
            }
          },
          "defaultConfiguration": "development"
        },
        "extract-i18n": {
          "builder": "@angular-devkit/build-angular:extract-i18n",
          "options": {
            "browserTarget": "SUBPLE_Angular:build"
          }
        },
        "test": {
          "builder": "@angular-devkit/build-angular:karma",
          "options": {
            "main": "src/test.ts",
            "polyfills": "src/polyfills.ts",
            "tsConfig": "tsconfig.spec.json",
            "karmaConfig": "karma.conf.js",
            "assets": [
              "src/favicon.ico",
              "src/assets"
            ],
            "styles": [
              "./node_modules/@angular/material/prebuilt-themes/deeppurple-amber.css",
              "src/styles.css"
            ],
            "scripts": []
          }
        }
      }
    }
  },
  "cli": {
    "analytics": "a3fa7531-b702-4c06-acb6-2c3e35a53e08"
  }
} 
/* To learn more about this file see: https://angular.io/config/tsconfig. */
{
  "compileOnSave": false,
  "compilerOptions": {
    "baseUrl": "./",
    "outDir": "./dist/out-tsc",
    "forceConsistentCasingInFileNames": true,
    "strict": true,
    "noImplicitAny": false,
    "noImplicitOverride": true,
    "noPropertyAccessFromIndexSignature": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "sourceMap": true,
    "paths": {
      "ngx-pagination": [
        "dist/ngx-pagination/ngx-pagination",
        "dist/ngx-pagination"
      ]
    },
    "declaration": false,
    "downlevelIteration": true,
    "experimentalDecorators": true,
    "moduleResolution": "node",
    "importHelpers": true,
    "target": "es2017",
    "module": "es2020",
    "lib": ["es2020", "dom"]
  },
  "typeRoots": ["node_modules/@angular/material"],
  "angularCompilerOptions": {
    "enableI18nLegacyMessageIdFormat": false,
    "strictInjectionParameters": true,
    "strictInputAccessModifiers": true,
    "strictTemplates": true
  }
} 

Forked On 29 Sep 2022 at 04:03:09

Michaelbromley

Hi, why are you using v3.0.1 of this library? That's a really old version which was built at the time when Angular was on like v4.

Commented On 29 Sep 2022 at 04:03:09

Michaelbromley

In admin UI, unable to update product after product variant name is auto-updated when stock is 0

Hi Michael, Eric from the marlin team at IBM here. I'm hoping you can help me out with a little bug!

Describe the bug When I have a product that has a stock of 0, Upon editing a product, I am able to save updates only so long as I haven't first made an update (such as changing the product variant name) that causes the field to auto-update. After this has happened, the Update button remains disabled even after I make further changes. When I navigate away from the product, I am prompted to either stay or discard unsaved changes. When I return, none of the changes made are saved – including the automatically updated field.

https://user-images.githubusercontent.com/83984184/192352708-7625431e-5e40-444e-8ed5-aa2c1d2ffbee.mov

https://user-images.githubusercontent.com/83984184/192352732-65a6b7ba-957b-40e7-acdb-0b58f00738fd.mov

To Reproduce Steps to reproduce the behavior:

  1. Create a product with stock 0 & navigate to its product variant tab on the admin ui
  2. Attempt to edit a field such as 'price' or product variant name, that will cause the field to automatically update.
  3. You will see the 'Update' button remains disabled
  4. Return to product page and attempt to rename the product
  5. You will see the 'Update' button remains disabled
  6. Attempt to navigate away. You will be prompted to remain or discard changes.
  7. Discard changes & return to the product page. You will see none of the changes have saved.

Expected behavior When changes are made that require saving, that the Update button should be enabled.

Environment (please complete the following information):

  • @vendure/core version: v1.5.2
  • Nodejs version v16.15.1
  • Database (mysql/postgres etc): postgres

Thanks!

Forked On 29 Sep 2022 at 03:10:03

Michaelbromley

Hi Eric,

Thanks for the detailed report and videos - that's really helpful for my understanding of the issue!

So I just tested this flow out on the live demo at https://demo.vendure.io/admin, but I cannot reproduce what you describe.

I notice from the video that you have quite a number of custom fields. Could it be that some of them are required and need a value filling out?

If that turns out to be the case, I think the handling would be to provide clear feedback about this to the admin.

Commented On 29 Sep 2022 at 03:10:03

Michaelbromley

Edge case - increase limits of monetary columns

Is your feature request related to a problem? Please describe. The limit for monetary fields (cart sum, order sum, product variant price) is now (2^31-1)/100 = 21 474 836,47 This limit can be not enough for countries with high denomination currencies.

Describe the solution you'd like Limit appears from TypeORM data type Number. It is considered as int with corresponding limit in node.js and selected DB.

Quick research shows, that there are couple of approaches:

  1. Using bigint It's solid approach, but Typeorm consider bigint as string, so many modifications should be done at the Vendure side
  2. Using decimal(15,4) or even bigger Also solid approach, but it requires using accurate data type and math operations at the Vendure side.

Describe alternatives you've considered Continue using int. But there will still exist problems with overflowing.

Forked On 29 Sep 2022 at 02:59:58

Michaelbromley

Thanks @mattgills for your input on this. If we can settle on a robust solution for the DB data type, we could perhaps introduce a new Scalar type on the GraphQL side which is able to handle bigint data as a number rather than a string.

Then I guess we'd only be limited by JavaScript's Number.MAX_SAFE_INTEGER which is 9007199254740991, so that would allow us to go up to $9 quadrillion, which should be enough even for your customers ;)

Commented On 29 Sep 2022 at 02:59:58

Michaelbromley

Better Promotions Free Gift support

Is your feature request related to a problem? Please describe. It is quite common to have a promotion that entitles the customer to a free gift. Currently we are able to support making an item free using an PromotionItemAction, but the customer still has to manually add it to the order.

The ideal flow in this case however is that once the condition(s) pass, then Vendure can automatically add the free gift to the order.

To work around this, I have suggested in the past the creation of a custom addItemToOrder mutation which contains logic to achieve this. However, it would be better to support this common use-case natively.

Describe the solution you'd like Perhaps a new kind of PromotionAction which is dedicated to adding items the order. The exact mechanism of how this would actually work is not yet clear.

Currently all promotion actions are processed as part of the OrderCalculator.applyPriceAdjustments() method. I would not suggest using this same method to add a new item, since this is mixing concerns and also you get this recursive issue where adding a new item during the price calculation can then potentially trigger new promotions, potentially changing prices of those already added etc.

It would probably work in a separate stage, so:

  1. Calculate order price including non-free gift promotion actions
  2. Check for any free gifts to add
  3. call OrderService.addItemToOrder() with any free gifts

Open questions

  1. Do we want the free gift to be removed if the Promotion is no longer applicable? Or leave it in the order and just allow it to go to full price. The latter would be a much simpler implementation. With the former, we'd need to somehow mark the item as "auto added" and verify its status on every change to the order.
  2. Do we want the free gift's discount to be coupled to the act of adding it to the order? Or have it so it's simply an "auto add" feature, and the "free" part is controlled in the present way with a PromotionItemAction.

Forked On 29 Sep 2022 at 02:02:55

Michaelbromley

First feedback from testing:

  • We need a way to handle errors that might arise in the side-effects. For example, I set up a free gift side-effect similar to the example above, but it wasn't working. I found out it was because there was insufficient stock of the selected ProductVariant, so the orderService.addItemToOrder() call was returning an ErrorResult.

I think we can handle this by wrapping the calls to onActivate and onDeactivate in a try-catch, and in the case of an error, we convert that into a new type of ErrorResult which we'll have to add to all the union types that can trigger a promotion side effect.

Commented On 29 Sep 2022 at 02:02:55

Michaelbromley

Better Promotions Free Gift support

Is your feature request related to a problem? Please describe. It is quite common to have a promotion that entitles the customer to a free gift. Currently we are able to support making an item free using an PromotionItemAction, but the customer still has to manually add it to the order.

The ideal flow in this case however is that once the condition(s) pass, then Vendure can automatically add the free gift to the order.

To work around this, I have suggested in the past the creation of a custom addItemToOrder mutation which contains logic to achieve this. However, it would be better to support this common use-case natively.

Describe the solution you'd like Perhaps a new kind of PromotionAction which is dedicated to adding items the order. The exact mechanism of how this would actually work is not yet clear.

Currently all promotion actions are processed as part of the OrderCalculator.applyPriceAdjustments() method. I would not suggest using this same method to add a new item, since this is mixing concerns and also you get this recursive issue where adding a new item during the price calculation can then potentially trigger new promotions, potentially changing prices of those already added etc.

It would probably work in a separate stage, so:

  1. Calculate order price including non-free gift promotion actions
  2. Check for any free gifts to add
  3. call OrderService.addItemToOrder() with any free gifts

Open questions

  1. Do we want the free gift to be removed if the Promotion is no longer applicable? Or leave it in the order and just allow it to go to full price. The latter would be a much simpler implementation. With the former, we'd need to somehow mark the item as "auto added" and verify its status on every change to the order.
  2. Do we want the free gift's discount to be coupled to the act of adding it to the order? Or have it so it's simply an "auto add" feature, and the "free" part is controlled in the present way with a PromotionItemAction.

Forked On 29 Sep 2022 at 12:49:46

Michaelbromley

The flow is like this:

1 .order change (add item, change quantity, apply coupon etc) 2. Snapshot current active promotions on the order. 3. OrderService.applyPriceAdjustments(), invoke all execute() functions on PromotionActions where the conditions are satisfied. 4. Compare active promotions with the snapshot 5. For any promotions that are newly active, execute onActivated 6. For any promotions that were in the snapshot but no longer active, execute onDeactivated.

I've just published this POC on the major branch: https://github.com/vendure-ecommerce/vendure/commit/1a4a117f6fca946dfa4999bad194c28c97f865eb

And released it in pre-release v2.0.0-next.18, so I can do some real-world testing with it.

Commented On 29 Sep 2022 at 12:49:46

Michaelbromley

fix(core): 'productVariantId' in group statement is ambiguous (#1793)

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

fix(core): Add null checks for relations in Order entity getters

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

feat(core): Pass order arg to OrderItemPriceCalculationStrategy and ChangedPriceHandlingStrategy (#1749)

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

fix(admin-ui): Fix ShippingMethod update error with falsy config values

Fixes #1800

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

chore: Publish v1.7.3

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

fix(admin-ui): Fix fix of ShippingMethod update error

Relates to #1800. The first "fix" contains an error in the negated formValue expression. It is very lucky that the precedence of the ! operator works on the formValue first, otherwise the "fix" would have totally broken things way worse than before! As it stands, it did fix the issue in question but at the same time broken the null check in a way that meant the throw statement could never be reached.

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

Merge branch 'master' into minor

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

feat(core): Add Facet/Collection Channel assignment mutations

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

test(core): Add e2e tests for Collection channel assignment

Relates to #1725

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

test(core): Add e2e tests for Facet channel assignment

Relates to #1725

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

feat(admin-ui): Add support for shift-select to DataTableComponent

Relates to #853

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

feat(admin-ui): Create supporting infrastructure for bulk actions API

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

feat(core): Add bulk product deletion mutations

Relates to #853

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

feat(admin-ui): Add support for bulk product deletion

Relates to #853

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

feat(admin-ui): Add support for bulk product channel assignment

Relates to #853

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

feat(core): Add bulk product update mutation

Relates to #853

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

feat(admin-ui): Add support for bulk product facet editing

Relates to #853

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

refactor(admin-ui): Simplify DataTable/BulkActionMenu relationship

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

feat(core): Add bulk facet delete mutation

Relates to #853

Pushed On 29 Sep 2022 at 12:37:38

Michaelbromley

feat(admin-ui): Add support for bulk facet deletion

Relates to #853

Pushed On 29 Sep 2022 at 12:37:38
Create Branch
Michaelbromley In vendure-ecommerce/vendure Create Branchv2.0.0-next.18

Michaelbromley

A headless GraphQL ecommerce framework for the modern web

On 29 Sep 2022 at 12:37:38

Michaelbromley

Better Promotions Free Gift support

Is your feature request related to a problem? Please describe. It is quite common to have a promotion that entitles the customer to a free gift. Currently we are able to support making an item free using an PromotionItemAction, but the customer still has to manually add it to the order.

The ideal flow in this case however is that once the condition(s) pass, then Vendure can automatically add the free gift to the order.

To work around this, I have suggested in the past the creation of a custom addItemToOrder mutation which contains logic to achieve this. However, it would be better to support this common use-case natively.

Describe the solution you'd like Perhaps a new kind of PromotionAction which is dedicated to adding items the order. The exact mechanism of how this would actually work is not yet clear.

Currently all promotion actions are processed as part of the OrderCalculator.applyPriceAdjustments() method. I would not suggest using this same method to add a new item, since this is mixing concerns and also you get this recursive issue where adding a new item during the price calculation can then potentially trigger new promotions, potentially changing prices of those already added etc.

It would probably work in a separate stage, so:

  1. Calculate order price including non-free gift promotion actions
  2. Check for any free gifts to add
  3. call OrderService.addItemToOrder() with any free gifts

Open questions

  1. Do we want the free gift to be removed if the Promotion is no longer applicable? Or leave it in the order and just allow it to go to full price. The latter would be a much simpler implementation. With the former, we'd need to somehow mark the item as "auto added" and verify its status on every change to the order.
  2. Do we want the free gift's discount to be coupled to the act of adding it to the order? Or have it so it's simply an "auto add" feature, and the "free" part is controlled in the present way with a PromotionItemAction.

Forked On 29 Sep 2022 at 11:09:53

Michaelbromley

it seems that onActivate, onDeActivate it seems it hard to tell what phase of this two states , why not conbine these to one event like onChange?

I'm not sure about what difference this makes? Is there some capability which would be possible with a single handler rather than 2? I tend to like 2 explicitly-named handlers better.

BTW should we make sure that onActive, onDeatvie always exec completed before execute() method invoke?

In the current POC design, onActivated and onDeactivated are always called after all order price calculations (including all execute functions) have completed.

Commented On 29 Sep 2022 at 11:09:53

Michaelbromley

Better Promotions Free Gift support

Is your feature request related to a problem? Please describe. It is quite common to have a promotion that entitles the customer to a free gift. Currently we are able to support making an item free using an PromotionItemAction, but the customer still has to manually add it to the order.

The ideal flow in this case however is that once the condition(s) pass, then Vendure can automatically add the free gift to the order.

To work around this, I have suggested in the past the creation of a custom addItemToOrder mutation which contains logic to achieve this. However, it would be better to support this common use-case natively.

Describe the solution you'd like Perhaps a new kind of PromotionAction which is dedicated to adding items the order. The exact mechanism of how this would actually work is not yet clear.

Currently all promotion actions are processed as part of the OrderCalculator.applyPriceAdjustments() method. I would not suggest using this same method to add a new item, since this is mixing concerns and also you get this recursive issue where adding a new item during the price calculation can then potentially trigger new promotions, potentially changing prices of those already added etc.

It would probably work in a separate stage, so:

  1. Calculate order price including non-free gift promotion actions
  2. Check for any free gifts to add
  3. call OrderService.addItemToOrder() with any free gifts

Open questions

  1. Do we want the free gift to be removed if the Promotion is no longer applicable? Or leave it in the order and just allow it to go to full price. The latter would be a much simpler implementation. With the former, we'd need to somehow mark the item as "auto added" and verify its status on every change to the order.
  2. Do we want the free gift's discount to be coupled to the act of adding it to the order? Or have it so it's simply an "auto add" feature, and the "free" part is controlled in the present way with a PromotionItemAction.

Forked On 29 Sep 2022 at 10:07:56

Michaelbromley

What do you mean by "upon order completion" ?

I'm referring to this: https://github.com/vendure-ecommerce/vendure/blob/master/packages/core/src/service/helpers/order-state-machine/order-state-machine.ts#L187

Namely, until the order is placed (transitions to PaymentAuthorized/PaymentSettled by default), the order.promotions array will always be empty, because no Promotions have been related to the order yet. Under the existing system, this relation is only established (i.e. order.promotions = activePromotions, save order) upon placing the order.

In my POC implementation, I am doing the side effect checks in the OrderService.applyPriceAdjustments() method, which I believe is called by all of the above.

Another caveat with free items is that the user might manually remove an automatically added free item, but if the promotion is still valid, it would just add it back leading to a confusing UX. The way we solve this is by adding a custom flag to free items which the frontend uses to disable the add/remove to cart buttons on that specific product.

This is a very good point, and can be accommodated with this POC design - you'd need to add the customField to the OrderLine and set it in the onActivate addItemToOrder call.

Commented On 29 Sep 2022 at 10:07:56

Michaelbromley

Better Promotions Free Gift support

Is your feature request related to a problem? Please describe. It is quite common to have a promotion that entitles the customer to a free gift. Currently we are able to support making an item free using an PromotionItemAction, but the customer still has to manually add it to the order.

The ideal flow in this case however is that once the condition(s) pass, then Vendure can automatically add the free gift to the order.

To work around this, I have suggested in the past the creation of a custom addItemToOrder mutation which contains logic to achieve this. However, it would be better to support this common use-case natively.

Describe the solution you'd like Perhaps a new kind of PromotionAction which is dedicated to adding items the order. The exact mechanism of how this would actually work is not yet clear.

Currently all promotion actions are processed as part of the OrderCalculator.applyPriceAdjustments() method. I would not suggest using this same method to add a new item, since this is mixing concerns and also you get this recursive issue where adding a new item during the price calculation can then potentially trigger new promotions, potentially changing prices of those already added etc.

It would probably work in a separate stage, so:

  1. Calculate order price including non-free gift promotion actions
  2. Check for any free gifts to add
  3. call OrderService.addItemToOrder() with any free gifts

Open questions

  1. Do we want the free gift to be removed if the Promotion is no longer applicable? Or leave it in the order and just allow it to go to full price. The latter would be a much simpler implementation. With the former, we'd need to somehow mark the item as "auto added" and verify its status on every change to the order.
  2. Do we want the free gift's discount to be coupled to the act of adding it to the order? Or have it so it's simply an "auto add" feature, and the "free" part is controlled in the present way with a PromotionItemAction.

Forked On 29 Sep 2022 at 09:35:16

Michaelbromley

when onDeactivate will be triggered? It gets triggered whenever a promotion which was active, becomes no longer active.

Example:

  • add couponCode, onActivate is invoked
  • remove couponCode, onDeactivate is invoked.

and then could add associate Promotion as parameter for PromotionAction?

You mean this issue?

  • https://github.com/vendure-ecommerce/vendure/issues/1787

yes I will probably be able to add that.

Commented On 29 Sep 2022 at 09:35:16

Michaelbromley

Better Promotions Free Gift support

Is your feature request related to a problem? Please describe. It is quite common to have a promotion that entitles the customer to a free gift. Currently we are able to support making an item free using an PromotionItemAction, but the customer still has to manually add it to the order.

The ideal flow in this case however is that once the condition(s) pass, then Vendure can automatically add the free gift to the order.

To work around this, I have suggested in the past the creation of a custom addItemToOrder mutation which contains logic to achieve this. However, it would be better to support this common use-case natively.

Describe the solution you'd like Perhaps a new kind of PromotionAction which is dedicated to adding items the order. The exact mechanism of how this would actually work is not yet clear.

Currently all promotion actions are processed as part of the OrderCalculator.applyPriceAdjustments() method. I would not suggest using this same method to add a new item, since this is mixing concerns and also you get this recursive issue where adding a new item during the price calculation can then potentially trigger new promotions, potentially changing prices of those already added etc.

It would probably work in a separate stage, so:

  1. Calculate order price including non-free gift promotion actions
  2. Check for any free gifts to add
  3. call OrderService.addItemToOrder() with any free gifts

Open questions

  1. Do we want the free gift to be removed if the Promotion is no longer applicable? Or leave it in the order and just allow it to go to full price. The latter would be a much simpler implementation. With the former, we'd need to somehow mark the item as "auto added" and verify its status on every change to the order.
  2. Do we want the free gift's discount to be coupled to the act of adding it to the order? Or have it so it's simply an "auto add" feature, and the "free" part is controlled in the present way with a PromotionItemAction.

Forked On 29 Sep 2022 at 08:22:46

Michaelbromley

OK I have a promising proof-of-concept design running locally. Here's what a free gift promotion looks like:

let orderService: OrderService;
export const freeGiftAction = new PromotionItemAction({
    code: 'free_gift',
    description: [{ languageCode: LanguageCode.en, value: 'Add free gifts to the order' }],
    args: {
        productVariantIds: {
            type: 'ID',
            list: true,
            ui: { component: 'product-selector-form-input' },
            label: [{ languageCode: LanguageCode.en, value: 'Gift product variants' }],
        },
    },
    init(injector) {
        orderService = injector.get(OrderService);
    },
    execute(ctx, orderItem, orderLine, args) {
        if (lineContainsIds(args.productVariantIds, orderLine)) {
            const unitPrice = ctx.channel.pricesIncludeTax ? orderLine.unitPriceWithTax : orderLine.unitPrice;
            return -unitPrice;
        }
        return 0;
    },
    async onActivate(ctx, order, args) {
        for (const id of args.productVariantIds) {
            if (!order.lines.find(line => idsAreEqual(line.productVariant.id, id))) {
                // The order does not yet contain this free gift, so add it
                await orderService.addItemToOrder(ctx, order.id, id, 1);
            }
        }
    },
    async onDeactivate(ctx, order, args) {
        for (const id of args.productVariantIds) {
            const lineWithFreeGift = order.lines.find(line => idsAreEqual(line.productVariant.id, id));
            if (lineWithFreeGift) {
                // Remove the free gift
                await orderService.adjustOrderLine(
                    ctx,
                    order.id,
                    lineWithFreeGift.id,
                    lineWithFreeGift.quantity - 1,
                );
            }
        }
    },
}); 

Going to do some more testing and make sure this is not interfering with any existing processes.

Note that in order for this API to work, I needed to make a functional change:

  • Previously, the Order.promotions relation would only get populated upon order completion (checkout).
  • Now, in order to correctly track changes to active promotions, I need to populate this relation as soon as an order becomes active.

I don't think this should be a breaking change, but who knows whether someone for some reason relied on the former behaviour 🤷 . I cannot remember the reasoning for only adding the relation upon order completion, but in any case it seems better to add the relation as soon as a Promotion activates.

Commented On 29 Sep 2022 at 08:22:46

Michaelbromley

started

Started On 29 Sep 2022 at 06:45:57

Michaelbromley

Better Promotions Free Gift support

Is your feature request related to a problem? Please describe. It is quite common to have a promotion that entitles the customer to a free gift. Currently we are able to support making an item free using an PromotionItemAction, but the customer still has to manually add it to the order.

The ideal flow in this case however is that once the condition(s) pass, then Vendure can automatically add the free gift to the order.

To work around this, I have suggested in the past the creation of a custom addItemToOrder mutation which contains logic to achieve this. However, it would be better to support this common use-case natively.

Describe the solution you'd like Perhaps a new kind of PromotionAction which is dedicated to adding items the order. The exact mechanism of how this would actually work is not yet clear.

Currently all promotion actions are processed as part of the OrderCalculator.applyPriceAdjustments() method. I would not suggest using this same method to add a new item, since this is mixing concerns and also you get this recursive issue where adding a new item during the price calculation can then potentially trigger new promotions, potentially changing prices of those already added etc.

It would probably work in a separate stage, so:

  1. Calculate order price including non-free gift promotion actions
  2. Check for any free gifts to add
  3. call OrderService.addItemToOrder() with any free gifts

Open questions

  1. Do we want the free gift to be removed if the Promotion is no longer applicable? Or leave it in the order and just allow it to go to full price. The latter would be a much simpler implementation. With the former, we'd need to somehow mark the item as "auto added" and verify its status on every change to the order.
  2. Do we want the free gift's discount to be coupled to the act of adding it to the order? Or have it so it's simply an "auto add" feature, and the "free" part is controlled in the present way with a PromotionItemAction.

Forked On 28 Sep 2022 at 02:11:34

Michaelbromley

Technically, since the execute method is async, you can also use it to do side-effects.

Yes, sure you can already do side-effects, but I really mean the intention is that it is pure, and then we can have an explicit API for side-effects only (i.e. no return value).

How to determine how gifts should be stored in the Product list? as standard product item? or special "gift" tagged products? how to determine when a special rule matched, which gift should be auto added?

I think this will need to be solved in the inplementation of the PromotionAction itself, but another way would be to just look up the variant ID, since we should already know the ID of the variant being added as a free gift from the args object.

Commented On 28 Sep 2022 at 02:11:34

Michaelbromley

Fix/testserver middleware

TestServer.init() doesn't apply middleware with beforeListen: true. This PR applies the middleware before the testserver listens for incoming requests.

Forked On 28 Sep 2022 at 02:01:28

Michaelbromley

Thanks! 🙌

Commented On 28 Sep 2022 at 02:01:28

Michaelbromley

fix(testing): Correctly apply beforeListen middleware on TestServer (#1802)

Pushed On 28 Sep 2022 at 02:01:22