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

Added Facade for easy Flagsmith access #1

Closed
wants to merge 1 commit into from

Conversation

legoheld
Copy link

How about adding a Facades to retrieve the Flagsmith instance?

use Clearlyip\LaravelFlagsmith\Facades\LaravelFlagsmith;

// retrieve FlagSmith instance and call directly its function
LaravelFlagsmith::getFeatureValue( 'feature1' );

@legoheld legoheld changed the title Added Facades for easy Flagsmith access Added Facade for easy Flagsmith access Oct 13, 2022
@tm1000
Copy link
Member

tm1000 commented Oct 24, 2022

This is great! any reason to not just call it "Flagsmith"?

@legoheld
Copy link
Author

This is a good Point, makes more sense to call it directly Flagsmith.

Something else that I was thinking about was your UserTrait, which makes totally sense as long as you have a User.
If you have a system where you start without users you would probably access Flagsmith with this new Facade:

use Clearlyip\LaravelFlagsmith\Facades\Flagsmith;

Flagsmith::getFeatureValue( 'feature1' );

Later in the project you will introduce the User and then you have to adjust all your Facade calls with the usertrait methods. So I was wondering if it would make sense to add a little more logic to the Facade so that it will send the logged in/passed user to the Flagsmith API based on the config.

This way the User Trait methods would be reduced into simple Facade calls:

public function getFeatures(): array {
  return Flagsmith::getFeatures( $this );
}

And the getFeatureIdentity functionality would be on the Facade instance.

This way you just use the Facade everywhere and you have additional helper methods on the UserTrait that will invoke properly the Facade.

Do you see what I mean? ;)
Do you think that makes sense? I can adjust the PR if you like...

@tm1000 tm1000 self-assigned this Oct 27, 2022
@tm1000 tm1000 added enhancement New feature or request good first issue Good for newcomers question Further information is requested labels Oct 27, 2022
@tm1000
Copy link
Member

tm1000 commented Feb 18, 2024

See: #5

@tm1000 tm1000 closed this Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers question Further information is requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants