search

medusajs / medusa

The world's most flexible commerce platform.
https://medusajs.com
MIT License
26.12k stars 2.63k forks source link

[Bug]: product images are duplicated and/or not removed on save #10252

Closed docloulou closed 56 minutes ago

docloulou commented 2 hours ago

Package.json file

{
  "name": "medusa-starter-default",
  "version": "0.0.1",
  "description": "A starter for Medusa projects.",
  "author": "Medusa (https://medusajs.com)",
  "license": "MIT",
  "keywords": [
    "sqlite",
    "postgres",
    "typescript",
    "ecommerce",
    "headless",
    "medusa"
  ],
  "scripts": {
    "build": "medusa build",
    "seed": "medusa exec ./src/scripts/seed.ts",
    "seed-categories": "medusa exec ./src/scripts/seed_categories.ts",
    "seed-products": "medusa exec ./src/scripts/seed_products.ts",
    "start": "medusa start",
    "dev": "medusa develop",
    "predeploy": "medusa db:migrate",
    "test:integration:http": "TEST_TYPE=integration:http NODE_OPTIONS=--experimental-vm-modules jest --silent=false --runInBand --forceExit",
    "test:integration:modules": "TEST_TYPE=integration:modules NODE_OPTIONS=--experimental-vm-modules jest --silent --runInBand --forceExit",
    "test:unit": "TEST_TYPE=unit NODE_OPTIONS=--experimental-vm-modules jest --silent --runInBand --forceExit"
  },
  "dependencies": {
    "@medusajs/admin-sdk": "^2.0.5",
    "@medusajs/cache-redis": "^2.0.5",
    "@medusajs/cli": "^2.0.5",
    "@medusajs/event-bus-redis": "^2.0.5",
    "@medusajs/framework": "^2.0.5",
    "@medusajs/medusa": "^2.0.5",
    "@medusajs/workflow-engine-redis": "^2.0.5",
    "@mikro-orm/core": "5.9.7",
    "@mikro-orm/knex": "5.9.7",
    "@mikro-orm/migrations": "5.9.7",
    "@mikro-orm/postgresql": "5.9.7",
    "awilix": "^8.0.1",
    "pg": "^8.13.0"
  },
  "devDependencies": {
    "@medusajs/test-utils": "^2.0.5",
    "@mikro-orm/cli": "5.9.7",
    "@swc/core": "1.5.7",
    "@swc/jest": "^0.2.36",
    "@types/jest": "^29.5.13",
    "@types/node": "^20.0.0",
    "@types/react": "^18.3.2",
    "@types/react-dom": "^18.2.25",
    "jest": "^29.7.0",
    "prop-types": "^15.8.1",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "ts-node": "^10.9.2",
    "typescript": "^5.6.2",
    "vite": "^5.2.11"
  },
  "engines": {
    "node": ">=20"
  },
  "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"
}

Node.js version

v22

Database and its version

postgre 16

Operating system name and version

linux docker

Browser name

No response

What happended?

on 2.0.5, multiple bugs:

  1. if i change image order and save, all images are duplicated
  2. if i open gallery modal and just save, all images are duplicated
  3. if i delete image, selected image are not removed from the table "image" (that causing error during last Migration20241122120331, see error log just below)
  4. if i delete image, images are not delete on the S3 
info:    MODULE: product
info:      ● Migrating Migration20241122120331
error:   Failed with error Migration Migration20241122120331 (up) failed: Original error: alter table if exists "image" alter column "product_id" set not null; - column "product_id" of relation "image" contains null values
error:   Migration Migration20241122120331 (up) failed: Original error: alter table if exists "image" alter column "product_id" set not null; - column "product_id" of relation "image" contains null values
MigrationError: Migration Migration20241122120331 (up) failed: Original error: alter table if exists "image" alter column "product_id" set not null; - column "product_id" of relation "image" contains null values
    at /home/docloulou/backend-savourea-pro/node_modules/umzug/src/umzug.ts:261:12
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Umzug.runCommand (/home/docloulou/backend-savourea-pro/node_modules/umzug/src/umzug.ts:210:11)
    at async Migrator.runInTransaction (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/migrations/Migrator.js:301:21)
    at async PostgreSqlConnection.transactional (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/knex/AbstractSqlConnection.js:36:25)
    at async Migrations.run (/home/docloulou/backend-savourea-pro/node_modules/@medusajs/utils/src/migrations/index.ts:87:19)
    at async AsyncFunction.<anonymous> (/home/docloulou/backend-savourea-pro/node_modules/@medusajs/utils/src/modules-sdk/migration-scripts/migration-up.ts:47:22)
    at async runMigrations (/home/docloulou/backend-savourea-pro/node_modules/@medusajs/modules-sdk/src/loaders/utils/load-internal.ts:481:9)
    at async Function.migrateUp (/home/docloulou/backend-savourea-pro/node_modules/@medusajs/modules-sdk/src/medusa-module.ts:799:9)
    at async applyMigration (/home/docloulou/backend-savourea-pro/node_modules/@medusajs/modules-sdk/src/medusa-app.ts:517:9) {
  cause: NotNullConstraintViolationException: alter table if exists "image" alter column "product_id" set not null; - column "product_id" of relation "image" contains null values
      at PostgreSqlExceptionConverter.convertException (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/postgresql/PostgreSqlExceptionConverter.js:24:24)
      at PostgreSqlDriver.convertException (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/core/drivers/DatabaseDriver.js:201:54)
      at /home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/core/drivers/DatabaseDriver.js:205:24
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async Function.runSerial (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/core/utils/Utils.js:611:22)
      at async connection.transactional.ctx (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/migrations/MigrationRunner.js:23:17)
      at async PostgreSqlConnection.transactional (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/knex/AbstractSqlConnection.js:36:25)
      at async MigrationRunner.run (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/migrations/MigrationRunner.js:20:13)
      at async createMigrationHandler (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/migrations/Migrator.js:191:13)
      at async /home/docloulou/backend-savourea-pro/node_modules/umzug/src/umzug.ts:259:6

  previous error: alter table if exists "image" alter column "product_id" set not null; - column "product_id" of relation "image" contains null values
      at Parser.parseErrorMessage (/home/docloulou/backend-savourea-pro/node_modules/pg-protocol/src/parser.ts:368:69)
      at Parser.handlePacket (/home/docloulou/backend-savourea-pro/node_modules/pg-protocol/src/parser.ts:187:21)
      at Parser.parse (/home/docloulou/backend-savourea-pro/node_modules/pg-protocol/src/parser.ts:102:30)
      at Socket.<anonymous> (/home/docloulou/backend-savourea-pro/node_modules/pg-protocol/src/index.ts:7:48)
      at Socket.emit (node:events:519:28)
      at Socket.emit (node:domain:488:12)
      at addChunk (node:internal/streams/readable:559:12)
      at readableAddChunkPushByteMode (node:internal/streams/readable:510:3)
      at Socket.Readable.push (node:internal/streams/readable:390:5)
      at TCP.onStreamRead (node:internal/stream_base_commons:191:23) {
    length: 150,
    severity: 'ERROR',
    code: '23502',
    detail: undefined,
    hint: undefined,
    position: undefined,
    internalPosition: undefined,
    internalQuery: undefined,
    where: undefined,
    schema: 'public',
    table: 'image',
    column: 'product_id',
    dataType: undefined,
    constraint: undefined,
    file: 'tablecmds.c',
    line: '6256',
    routine: 'ATRewriteTable'
  },
  jse_cause: NotNullConstraintViolationException: alter table if exists "image" alter column "product_id" set not null; - column "product_id" of relation "image" contains null values
      at PostgreSqlExceptionConverter.convertException (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/postgresql/PostgreSqlExceptionConverter.js:24:24)
      at PostgreSqlDriver.convertException (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/core/drivers/DatabaseDriver.js:201:54)
      at /home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/core/drivers/DatabaseDriver.js:205:24
      at processTicksAndRejections (node:internal/process/task_queues:95:5)
      at async Function.runSerial (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/core/utils/Utils.js:611:22)
      at async connection.transactional.ctx (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/migrations/MigrationRunner.js:23:17)
      at async PostgreSqlConnection.transactional (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/knex/AbstractSqlConnection.js:36:25)
      at async MigrationRunner.run (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/migrations/MigrationRunner.js:20:13)
      at async createMigrationHandler (/home/docloulou/backend-savourea-pro/node_modules/@mikro-orm/migrations/Migrator.js:191:13)
      at async /home/docloulou/backend-savourea-pro/node_modules/umzug/src/umzug.ts:259:6

  previous error: alter table if exists "image" alter column "product_id" set not null; - column "product_id" of relation "image" contains null values
      at Parser.parseErrorMessage (/home/docloulou/backend-savourea-pro/node_modules/pg-protocol/src/parser.ts:368:69)
      at Parser.handlePacket (/home/docloulou/backend-savourea-pro/node_modules/pg-protocol/src/parser.ts:187:21)
      at Parser.parse (/home/docloulou/backend-savourea-pro/node_modules/pg-protocol/src/parser.ts:102:30)
      at Socket.<anonymous> (/home/docloulou/backend-savourea-pro/node_modules/pg-protocol/src/index.ts:7:48)
      at Socket.emit (node:events:519:28)
      at Socket.emit (node:domain:488:12)
      at addChunk (node:internal/streams/readable:559:12)
      at readableAddChunkPushByteMode (node:internal/streams/readable:510:3)
      at Socket.Readable.push (node:internal/streams/readable:390:5)
      at TCP.onStreamRead (node:internal/stream_base_commons:191:23) {
    length: 150,
    severity: 'ERROR',
    code: '23502',
    detail: undefined,
    hint: undefined,
    position: undefined,
    internalPosition: undefined,
    internalQuery: undefined,
    where: undefined,
    schema: 'public',
    table: 'image',
    column: 'product_id',
    dataType: undefined,
    constraint: undefined,
    file: 'tablecmds.c',
    line: '6256',
    routine: 'ATRewriteTable'
  },
  migration: {
    direction: 'up',
    name: 'Migration20241122120331',
    path: '/home/docloulou/backend-savourea-pro/node_modules/@medusajs/product/dist/migrations/Migration20241122120331.js',
    context: {}
  }
}

Expected behavior

  1. if i change image order and save, all images are duplicated => only change order
  2. if i open gallery modal and just save, all images are duplicated => no change expected
  3. if i delete image, selected image are not removed from the table "image" (that causing error during last Migration20241122120331, see error log just below) => if orphan image are deleted, migration should happen
  4. if i delete image, images are not deleted on the S3 => remove image from the S3

Actual behavior

see "What happended?" section

Link to reproduction repo

-

olivermrbl commented 2 hours ago

@docloulou, thanks for the report. We are aware of the issue and will patch it shortly.

olivermrbl commented 1 hour ago

@docloulou, can you please try the latest preview version? Easiest way to test is by updating the Medusa packages in your package.json to following version: 2.0.6-preview-20241125104525

docloulou commented 1 hour ago

let me try

docloulou commented 1 hour ago

@docloulou, can you please try the latest preview version? Easiest way to test is by updating the Medusa packages in your package.json to following version: 2.0.6-preview-20241125104525

everything is fine now, (wasnt able to test the first migration because i already applied my own fix) But: image from S3 is not deleted (normal behavior ? )

kasperkristensen commented 56 minutes ago

Hi @docloulou, thanks for testing it out and glad that everything is working as expected now. Yes, the images aren't deleted from S3 when they are deleted from Medusa, it's something we are aware of and intend to fix at a later time.