joas8211 / payload-tenancy

Multi-tenancy plugin for Payload CMS
MIT License
149 stars 10 forks source link

Upload collection do not respect 'path' strategy #18

Closed silveltman closed 1 year ago

silveltman commented 1 year ago

The link/image displayed in upload collections do not respect the 'path' strategy of this plugin.

Dashboard shows link/image for mydomain.com/myfolder/myimage.jpg when it should be mydomain.com/mytenant/myfolder/myimage.jpg

Screenshot 2023-08-24 at 18 02 02

joas8211 commented 1 year ago

Right, I'll come up with a fix for this tomorrow. That is if you don't want to contribute? 😉

silveltman commented 1 year ago

I would gladly do so, but at the moment it is completely new to me. Once I have experience with payload plugins I will try for another bug/feature! :)

joas8211 commented 1 year ago

I have published a fix for file URLs with version 1.1.1

silveltman commented 1 year ago

Thanks a lot for the fix! 🙏 Two things i noticed:

  1. Is it intended that the url is not a relative url instead of an absolute url? before: domain.com/folder/image.jpg now: /tenant/folder/image.jpg

  2. Is it correct that all the images that existed before the update get invisible (or even deleted)? For me not a problem, I was in development, but might be important for people updating on a production environment.

joas8211 commented 1 year ago
  1. I forgot to prepend the base URL, so that's probably why it won't show as absolute URL. I'll create a fix for it.
  2. Not sure what could have caused the disappearance of images / files or documents. Can you specify what disappeared and how? Did documents disappear from database or from API, or did files disappear from filesystem?
joas8211 commented 1 year ago

Fixed file URLs to include server URL in version 1.1.2

silveltman commented 1 year ago
  1. Not sure what could have caused the disappearance of images / files or documents. Can you specify what disappeared and how? Did documents disappear from database or from API, or did files disappear from filesystem?

I can't answer all your questions, since I was just quickly trying some stuff out. What I can tell you is the following:

silveltman commented 1 year ago

Sorry to bother you again 🙈

The tenant slug is included for the main url, but not for the different generated sizes. Screenshot 2023-09-04 at 22 59 46

joas8211 commented 1 year ago

There should be a fix for size variants now (v1.1.4). Can you confirm that it's working correctly?

Also, thanks for letting me know about these bugs :smiley:

silveltman commented 1 year ago

Everything is working 👌

Thanks a lot for the effort!

silveltman commented 1 year ago

There seems to be inconsistency in the tenancy path being added yes or no. Some images do load with the tenant path, some without.

It appears to be completely random. Refreshing the page (therefore making a new fetch request to the payload api) randomly return some images with the path and some without.

I would gladly look into this a bit further, but the randomness of the situation makes me question where to look.

I face this problem both in dev mode as in build mode. Both local, have not tested payload cloud yet

joas8211 commented 1 year ago

Can you create some sort of reproduction of the issue? Steps to reproduce, or repository, or both?

To guide you where you might find the problem, here's some questions:

silveltman commented 1 year ago

What tenant isolation strategy are you using? tenancy({ isolationStrategy: 'path' }),

Is the user logged in and are you allowing anonymous access? Not logged in, but access: { read: () => true }, on Images collection

Is serverURL set in the Payload config? Yes but I believe this does not have anything to do with my problem, since the server url is provided correctly, but the tenant slug not.

_Is there default "depth" set in the Payload config?_ yes, 3

Is the upload collection staticURL set to relative or absolute URL or unset?

  upload: {
    mimeTypes: ['image/*'],
    staticURL: '/images',
    staticDir: 'images',
  }

In the admin dashboard always only the first one is displayed: Screenshot 2023-09-29 at 10 04 43

When going to the next page in the collection, again only the top one is displayed.

As you can see here in the sourcecode, they both have the correct href (which is a relative url prefixed with the tenant slug), but only the first has has the tenant slug included in the src url: Screenshot 2023-09-29 at 10 20 58

On my frontend only 1 or 2 images are loaded randomly, sometime after 10 refreshes. ![Uploading Screenshot 2023-09-29 at 10.25.06.png…]() Screenshot 2023-09-29 at 10 26 45

One refresh later...: Screenshot 2023-09-29 at 10 27 21

I hope I provided you with valuable info. It's hard for me to make a reproduction, since this is a quiet large and private repo. If you need anything else, let me know!

silveltman commented 1 year ago

I also found dat the console is logging the following when opening the image collection"

[10:01:43] ERROR (payload): TypeError: Cannot set properties of undefined (setting 'skipTenancyUploadAfterReadHook')
    at /Users/silveltman/Documents/Github/monorepo/payload/node_modules/payload-tenancy/dist/hooks/upload.js:78:68
    at step (/Users/silveltman/Documents/Github/monorepo/payload/node_modules/payload-tenancy/dist/hooks/upload.js:44:23)
    at Object.next (/Users/silveltman/Documents/Github/monorepo/payload/node_modules/payload-tenancy/dist/hooks/upload.js:25:53)
    at /Users/silveltman/Documents/Github/monorepo/payload/node_modules/payload-tenancy/dist/hooks/upload.js:19:71
    at new Promise (<anonymous>)
    at __awaiter (/Users/silveltman/Documents/Github/monorepo/payload/node_modules/payload-tenancy/dist/hooks/upload.js:15:12)
    at /Users/silveltman/Documents/Github/monorepo/payload/node_modules/payload-tenancy/dist/hooks/upload.js:61:16
    at /Users/silveltman/Documents/Github/monorepo/payload/node_modules/payload/src/collections/operations/find.ts:223:24
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async /Users/silveltman/Documents/Github/monorepo/payload/node_modules/payload/src/collections/operations/find.ts:220:7
[10:01:43] ERROR (payload): NotFound: The requested resource was not found.
    at findByID (/Users/silveltman/Documents/Github/monorepo/payload/node_modules/payload/src/collections/operations/findByID.ts:105:13)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
silveltman commented 1 year ago

It seems to be causes at the API level, when fetching a collection via REST API. Here is my response for fetching my images collection:

yarn run v1.22.19
$ ts-node ./src/regenerate-images.ts
image {
  id: '651825559ef9e1e75c9592e0',
  filename: 'CGL logo horizontaal 2-5.png',
  mimeType: 'image/png',
  filesize: 511481,
  width: 2560,
  height: 2560,
  sizes: {
    w640: {
      url: 'http://localhost:3000/lavis-detailing/images/CGL logo horizontaal 2-5-640x640.webp',
      width: 640,
      height: 640,
      mimeType: 'image/webp',
      filesize: 16876,
      filename: 'CGL logo horizontaal 2-5-640x640.webp'
    },
    w2560: {
      url: 'http://localhost:3000/lavis-detailing/images/CGL logo horizontaal 2-5-2560x2560.webp',
      width: 2560,
      height: 2560,
      mimeType: 'image/webp',
      filesize: 71546,
      filename: 'CGL logo horizontaal 2-5-2560x2560.webp'
    }
  },
  createdAt: '2023-09-30T13:40:37.580Z',
  updatedAt: '2023-09-30T13:45:07.213Z',
  url: 'http://localhost:3000/lavis-detailing/images/CGL logo horizontaal 2-5.png'
}
image {
  id: '6518254d9ef9e1e75c95928a',
  alt: 'asfas',
  filename: 'vivint-solar-9CalgkSRZb8-unsplash.jpg',
  mimeType: 'image/jpeg',
  filesize: 671992,
  width: 2560,
  height: 3840,
  sizes: {
    w640: {
      width: 640,
      height: 960,
      mimeType: 'image/webp',
      filesize: 39556,
      filename: 'vivint-solar-9CalgkSRZb8-unsplash-640x960.webp',
      url: 'http://localhost:3000/images/vivint-solar-9CalgkSRZb8-unsplash-640x960.webp'
    },
    w2560: {
      width: 2560,
      height: 3840,
      mimeType: 'image/webp',
      filesize: 295780,
      filename: 'vivint-solar-9CalgkSRZb8-unsplash-2560x3840.webp',
      url: 'http://localhost:3000/images/vivint-solar-9CalgkSRZb8-unsplash-2560x3840.webp'
    }
  },
  createdAt: '2023-09-30T13:40:29.665Z',
  updatedAt: '2023-09-30T13:40:29.665Z',
  url: 'http://localhost:3000/images/vivint-solar-9CalgkSRZb8-unsplash.jpg'
}
image {
  id: '651825329ef9e1e75c9590ff',
  alt: 'testing alt',
  filename: 'benjamin-jopen-p2GuLUu79Rg-unsplash-1.jpg',
  mimeType: 'image/jpeg',
  filesize: 1098718,
  width: 2560,
  height: 3840,
  sizes: {
    w640: {
      url: 'http://localhost:3000/images/benjamin-jopen-p2GuLUu79Rg-unsplash-1-640x960.webp',
      width: 640,
      height: 960,
      mimeType: 'image/webp',
      filesize: 44666,
      filename: 'benjamin-jopen-p2GuLUu79Rg-unsplash-1-640x960.webp'
    },
    w2560: {
      url: 'http://localhost:3000/images/benjamin-jopen-p2GuLUu79Rg-unsplash-1-2560x3840.webp',
      width: 2560,
      height: 3840,
      mimeType: 'image/webp',
      filesize: 655200,
      filename: 'benjamin-jopen-p2GuLUu79Rg-unsplash-1-2560x3840.webp'
    }
  },
  createdAt: '2023-09-30T13:40:02.918Z',
  updatedAt: '2023-09-30T13:40:15.138Z',
  url: 'http://localhost:3000/images/benjamin-jopen-p2GuLUu79Rg-unsplash-1.jpg'
}
✨  Done in 2.37s.
joas8211 commented 1 year ago

It seems to me based on your findings the problem is that the hook this plugin is using, is not getting called more than once per request. I'll take a look more closely on few coming days.

joas8211 commented 1 year ago

Version 2.0.0 has this fixed. Payload 2.0.13 is required for that version. Please let me know if you depend on Payload 1 for your project.

Bohooslav commented 1 year ago

It works only with media collection but doesn't work for other collections with upload enabled tested on latest version of both payloadv2 and the lib