Laravel-Userstamps icon indicating copy to clipboard operation
Laravel-Userstamps copied to clipboard

Support Custom Foreign Keys

Open inmanturbo opened this issue 1 year ago • 7 comments

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

inmanturbo avatar Mar 26 '24 15:03 inmanturbo

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.

mattmcdev avatar Mar 26 '24 15:03 mattmcdev

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.

Thank you. After I've finished making the changes mentioned above I'll mark this as ready for review. Leaving as draft until then.

inmanturbo avatar Mar 26 '24 16:03 inmanturbo

@mattmcdonald-uk I've made the requested changes and marked this ready. Please review when you get a chance, thanks.

inmanturbo avatar Mar 26 '24 17:03 inmanturbo

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.

inmanturbo avatar Mar 26 '24 17:03 inmanturbo

I've overlooked the fact the the Listeners are all using Auth::id(), marking as draft for now until this is fixed.

inmanturbo avatar Mar 26 '24 19:03 inmanturbo

@mattmcdonald-uk it's ready!

inmanturbo avatar Mar 26 '24 20:03 inmanturbo

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

inmanturbo avatar Mar 29 '24 16:03 inmanturbo

Does using the new resolveUsing method give you a way to handle your use-case ?

mattmcdev avatar Feb 22 '25 14:02 mattmcdev

@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

inmanturbo avatar Feb 22 '25 15:02 inmanturbo