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

Custom table name on extended model breaks #396

Closed
mdcass opened this issue Jul 3, 2018 · 6 comments
Closed

Custom table name on extended model breaks #396

mdcass opened this issue Jul 3, 2018 · 6 comments

Comments

@mdcass
Copy link

mdcass commented Jul 3, 2018

Updated from 2.5.1 to 2.7.0, have a custom Activity model with a $table property set.

This change in Spatie\Activitylog\Models\Activity which overwrites that property from config is a bit too assumptive.

Logic should be along the lines of $this->table = $this->table ?? config('activitylog.table_name')

@mdcass
Copy link
Author

mdcass commented Jul 3, 2018

CC @miclf

@miclf
Copy link
Contributor

miclf commented Jul 3, 2018

Hey, good catch, @mdcass !

If I understand correctly, you’re using a custom table name in a custom Activity model, right?

When working on #361 a few months back my focus was mainly on the migration file. As the table name in the model was hardcoded, I did not expect any side effect involving a custom child class.

The fix you propose seems right to me. Do you plan to make a pull request or would you like me to work on one? (note that I won’t really have time to do so before next week-end)

@mdcass
Copy link
Author

mdcass commented Jul 9, 2018

If I get some time this week i'll submit a PR!

@Cholowao
Copy link

Cholowao commented Aug 3, 2018

@mdcass you got time????

..
.
.

.
.
.
.

@mdcass
Copy link
Author

mdcass commented Aug 4, 2018

I've not had chance to take a look unfortunately, i've not got the repo set up locally

@freekmurze
Copy link
Member

Will be fixed soon through #427

freekmurze pushed a commit that referenced this issue Oct 15, 2018
* Custom table name on extended model breaks (#396)

* Add Unit test

* Revert to #master branch migration

* Remove comment

* Convert variables casing
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

No branches or pull requests

4 participants