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

Tenant unaware feature and overall "features" feature improvements #1021

Closed
wants to merge 37 commits into from

Conversation

abrardev99
Copy link
Contributor

@abrardev99 abrardev99 commented Nov 25, 2022

todo update description after resolving reviews.

This PR adds the ability to bootstrap features early in the request life cycle. In simple words, You can bootstrap the feature without waiting for tenancy initialization. We are calling these tenant unaware feature.

Solution

PR introduces the new config key, which is an array of tenant-unaware features. For now, CrossDomainRedirect is added to this array. This also closes #949

Usage

You add your own tenant unaware feature in tenancy.php.

'tenant_unaware_features' => [
        Stancl\Tenancy\Features\CrossDomainRedirect::class, // https://tenancyforlaravel.com/docs/v3/features/cross-domain-redirect
       // ... add yours
    ],

Improvements

With the old approach, I noticed that the "$tenancy" object is passed but the tenancy is not initialized at this point. This happening because tenancy is not initiated yet. Maybe that's why current feature classes are not making use of the tenancy parameter. One feature was using actually, to add macro, but macro can be added statically like Tenancy::macro.

So the following got changed.

  • Removed the $tenancy parameter from the Feature interface bootstrap method. Now all tenant unaware & aware features have the same abstractions.
  • Tenant-aware features are being bootstrapped in the ->resolving(Tenancy) event. That means we can use DI.

The question can be how to develop tenant-aware features. Most probably using the Tenancy events.

@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2022

Codecov Report

Merging #1021 (f9e0277) into master (8b7862d) will decrease coverage by 0.02%.
The diff coverage is 90.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff              @@
##             master    #1021      +/-   ##
============================================
- Coverage     89.71%   89.69%   -0.02%     
- Complexity      575      585      +10     
============================================
  Files           138      138              
  Lines          1769     1785      +16     
============================================
+ Hits           1587     1601      +14     
- Misses          182      184       +2     
Impacted Files Coverage Δ
src/Features/TelescopeTags.php 0.00% <0.00%> (ø)
src/Features/CrossDomainRedirect.php 100.00% <100.00%> (ø)
src/Features/TenantConfig.php 100.00% <100.00%> (ø)
src/Features/UserImpersonation.php 100.00% <100.00%> (ø)
src/Features/ViteBundler.php 100.00% <100.00%> (ø)
src/TenancyServiceProvider.php 97.64% <100.00%> (+0.17%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@abrardev99 abrardev99 changed the title Features improvements Tenant unaware feature and overall "features" feature improvements Nov 25, 2022
@abrardev99 abrardev99 marked this pull request as ready for review November 25, 2022 12:36
tests/Pest.php Outdated Show resolved Hide resolved
tests/Pest.php Outdated Show resolved Hide resolved
assets/config.php Outdated Show resolved Hide resolved
@stancl stancl self-assigned this Feb 1, 2023
@stancl stancl marked this pull request as draft February 1, 2023 05:47
@lukinovec lukinovec marked this pull request as ready for review February 24, 2023 12:39
@stancl
Copy link
Member

stancl commented Dec 11, 2023

Resubmitted in v4 repo

@stancl stancl closed this Dec 11, 2023
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

Successfully merging this pull request may close these issues.

[4.x] Features are not registered before Tenancy is resolved from the container
4 participants