Allow consumers of the SDK to override the ID of users#195
Closed
hauthorn wants to merge 1 commit intolaravel:1.xfrom
Closed
Allow consumers of the SDK to override the ID of users#195hauthorn wants to merge 1 commit intolaravel:1.xfrom
hauthorn wants to merge 1 commit intolaravel:1.xfrom
Conversation
Member
|
Thanks for the PR. This is on our radar to address. I'm closing this one for now and we will circle back to it shortly. Thanks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've been in contact with Tim in support, because I would like to control the ID that is passed to Nightwatch.
This cannot be done as of now, as the events are logged with
->id(), which doesn't look at the user details provider.There are a couple of reasons you'd want to be able to do this. For example if you have different Authenticable types, each backed by a different eloquent model. In this situation, User with id 42 and Admin with id 42 would be considered the same User in nightwatch. If you try to do this anyway, you break the UI in Nightwatch (see my bug report for screenshots).
I believe this change would avoid having two sources of ID's, and allow people with multiple eloquent models (or GDPR concerns) to use this package.
I've adjusted the tests to match how the
UserProvideris actually instantiated inside the NightwatchServiceProvider.I'm currently using this change on our Nightwatch account, and so far it has worked like a charm :-)
Edit: I've run the static analysis and
pintlocally. I'm not sure how to supply the keys to run the PR checks.