Laravel-Userstamps
Laravel-Userstamps copied to clipboard
Support Custom Foreign Keys
Hey there! This PR adds support for custom foreign keys.
For instance if someone needs to use a uuid but it's not the key on the model.
This will make it easier to track userstamps across distributed systems.
In our case users are in a seperate database from most resources. Users have a ulid column specifically for relationships outside of the Auth domain, and for showing links where we won't show the auto-incremented id, which is used internally within the Auth domain.
This isn't a breaking change because it uses the default by default
/**
* Get the name of the foreign "id" column for the creator.
*
* @return string
*/
public function getCreatedByKey()
{
return defined('static::CREATED_BY_KEY') ? static::CREATED_BY_KEY : (new ($this->getUserClass())->getKeyName();
}
Usage
use Wildside\Userstamps\Userstamps;
class Foo extends Model {
use Userstamps;
const CREATED_BY = 'created_by_uuid';
const CREATED_BY_OWNER_KEY = 'uuid';
const UPDATED_BY = 'updated_by_uuid';
const UPDATED_BY_OWNER_KEY = 'uuid';
const DELETED_BY = 'deleted_by_uuid';
const DELETED_BY_OWNER_KEY = 'uuid';
}
- Edited to reflect current changes
Thanks for the PR.
For this to truly not be a breaking change, the current value passed along to the ownerKey property is null, so the default returned value should be null.
I would also prefer naming the const CREATED_BY_OWNER_KEY, etc. to sync with the argument that's being sent to belongsTo, and the methods similarly being getCreatedByOwnerKey, etc.
Thanks for the PR.
For this to truly not be a breaking change, the current value passed along to the
ownerKeyproperty isnull, so the default returned value should benull.I would also prefer naming the const
CREATED_BY_OWNER_KEY, etc. to sync with the argument that's being sent tobelongsTo, and the methods similarly beinggetCreatedByOwnerKey, etc.
Thank you. After I've finished making the changes mentioned above I'll mark this as ready for review. Leaving as draft until then.
@mattmcdonald-uk I've made the requested changes and marked this ready. Please review when you get a chance, thanks.
If you would prefer, I'm not opposed to opening a new Pr as a single commit. I've resisted squashing too much here, so that the conversation would be easier to follow.
I've overlooked the fact the the Listeners are all using Auth::id(), marking as draft for now until this is fixed.
@mattmcdonald-uk it's ready!
If anyone is interested in trying out this feature, you can do so by temporarily adding the fork to the repositories property of your composer.json
{
"repositories": [
{
"type": "vcs",
"url": "https://github.com/inmanturbo/Laravel-Userstamps.git"
}
]
}
Then update your dependency constraint to reference this branch by running:
composer require wildside/userstamps:dev-patch-1
Does using the new resolveUsing method give you a way to handle your use-case ?
@mattmcdev Yes using a resolveUsing closure should work fine. Just need to add a method on the user model that returns the desired attribute.
Thank you