file service "file key" mismatch with api

[agmyomyat]

Bug report

Describe the bug

delete upload file not working because of "file_key" from API body and "fileKey" in fileservice delete method params mismatch

System information

Medusa version (including plugins): 1.11.0
Node.js version:
Database:
Operating system:
Browser (if relevant):

Expected behavior

should delete upload object

Screenshots

[pevey]

I don't think that is an issue. The API uses file_key, and is validated as file_key. But the file service interface you reference above specifies fileKey, which is also correct, as that is the name of the argument in the file service delete function. The argument could be named anything.

The delete function actually does work fine. I've tested it once before when I did a small PR related to the s3 file plugin, and I just tested it again. It is just not being called by the medusa admin.

[pevey]

As a side note, and this is more in wish list territory:

When I noticed that the admin was not deleting files when I first started using it, I thought it might be because it is transitioning to have more of a media manager function than just having product-by-product images.

In many store admins, you manage media/images in one place. Then you can add the same image to multiple products. In this case, you don't WANT an image to be deleted from the file service if you remove it from a product, because it may be used by other products or elsewhere on the site. You want all deletions to happen only from the media manager.

Right now, if you have the same image for multiple products, it gets uploaded multiple times. There is no way to avoid this without manually editing the database.

[agmyomyat]

but the argument is object and it's "argument.fileKey" is passed to storage client. so i had to manually pass argument.file_key to really delete the object

[pevey]

The argument is not (supposed to be) an object. It expects a string, so I tested it using a string, like this:

await fileService.delete(req.body.file_key)

If the admin endpoint is passing an object to the function, and not a string, that is the real issue. I would guess that all of the file plugin delete functions are currently set up to use a string since they are all modeled after each other.

The name of the argument for s3 is file, and it is used here:

    const params = {
      Bucket: this.bucket_,
      Key: `${file}`,
    }

[agmyomyat]

this is the current source code of medusa it passes the whole body. so file service delete method argument is an object type

export default async (req, res) => {
  const validated = req.validatedBody as AdminDeleteUploadsReq

  const fileService = req.scope.resolve("fileService")

  await fileService.delete(validated)

  res
    .status(200)
    .send({ id: validated.file_key, object: "file", deleted: true })
}

[pevey]

Yes, and I'm saying that is the issue, not the naming. If it said await fileService.delete(validated.file_key), it would work with file service plugins as they are now.

The route and the typing need to be updated, not the naming.

It was probably intended that it be an object for consistency, but since plugins are already implemented using a string, the route should probably be changed to match.

[agmyomyat]

yeah i guess i was saying that too. It took me a day why the object is not deleting until i realized the type was wrong sorry for misunderstanding

[viicslen]

Could issue #4057 be related to this?

[pevey]

Just to circle back on this:

The core issue is that there are signs the argument for the delete function in file services was intended to be an object, as pointed out by the OP. DeleteFileType is defined as an object.

However, as implemented in medusa-file-s3, the delete function expects a string. So there is a mismatch, and the delete function is not currently working when called by the API.

There are two possible ways to solve this, and if the core teams weighs in on which way they want to go, I'm happy to work on a pull request.

1. Change the plugins. Make them implement delete with the argument as an object.
2. Change the type. Make it a string and not an object.

Con of Option 1: The medusa-plugin-s3 plugin implements delete by expecting a string argument. This seems to have been copied by at least one other plugin, medusa-file-r2. This change would break plugins that are not maintained as part of the core project.

Con of Option 2: It seems like the MinIO plugin may require the argument to be an object. Passing a string would not work.

I think option 1 is probably the way to go. I could submit a small PR to https://github.com/yinkakun/medusa-file-r2 as well to change the argument to an object.