Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Multi tenancy on default setup doesnt work #131

Closed
Inoir opened this issue Sep 30, 2022 · 4 comments
Closed

[BUG] Multi tenancy on default setup doesnt work #131

Inoir opened this issue Sep 30, 2022 · 4 comments

Comments

@Inoir
Copy link

Inoir commented Sep 30, 2022

Describe the bug
Currently the multi tenancy on default setup of the project does not work. Mostly caused of the behaviour medusa is loading the configModule and strips off all custom props. Since the TenantService is using the resolved configModule from the container, there is no multi_tenancy config in the one provided in the tenant service.

Other point why its not working is the TenantService itself does not get registered anymore. I only got it working with own module and importing the service and middleware there. So seems something is wrong in the module, but couldnt figure out. ( Just looked into the code and seems same issue as in TenantService, module uses the configModule resolved from container, so no multi_tenancy options in there)

To Reproduce
Steps to reproduce the behavior:

  1. Checkout new medusa-extender proj
  2. Configure multi tenancy and use the tenant module
  3. Try make a request to some tenant products (i.e list products)
  4. Error is thrown cause either tenantService couldnt be resolved (first issue) or Error is thrown cause of multi_tenancy options not set in the configModule loaded in the TenantService

Expected behavior
Should work

Screenshots
Screenshot 2022-09-30 at 13 59 08

Package version:
medusa-extender: 1.7.6
medusa: 1.4.1

@Inoir
Copy link
Author

Inoir commented Sep 30, 2022

Maybe also a hint here => i opened a pull request, to get this working in medusa without loading the config over and over again: medusajs/medusa#1896
Maybe also easier to understand why the multi_tenancy options are missing in the configModule from the container

@adrien2p
Copy link
Owner

adrien2p commented Sep 30, 2022

Indeed, the config get in the module is from your config so that one contains it, but in the service it is resolved through the container.

I did not marked it as experimental but as i explain to someone it is experimental. Since there is not that much interest into that architecture at the moment i am not sure to maintain it or maybe it will be deprecated in favour of custom approches in a case by case bases. Let see once medusa will allow to have custom properties to be able to look a bit more into it. This module will probably mainly serve as an example purpose that a ready to use module. Nut you are free to create a module to share with the community 😀

About loading the module. How did you import it to get the service not registered ?

@Inoir
Copy link
Author

Inoir commented Oct 1, 2022

Thanks for responding. Maybe should be more clearly in the docs, so people actually dont try to use it and only take it as guide how to do. Maybe also the custom props not supported in medusa should be pointed out. Took me a while to figure out whats wrong.

About the import: nothing special, only registered the module in the load function and enabled it in the config with some tenant settings. the middleware was registered but service not. since it was only for a test, i give up on this point.

@adrien2p
Copy link
Owner

adrien2p commented Oct 3, 2022

Sorry for the miss understanding, just seen that the module was marked as experimental in the documentation. But just in case, I did add a disclaimer block to be sure :)

@adrien2p adrien2p closed this as completed Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants