Skip to content
This repository has been archived by the owner on Nov 16, 2021. It is now read-only.

Use morph mapping for attachables #77

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

wimski
Copy link

@wimski wimski commented May 5, 2021

I like to use morph mapping for all model references that are written to the database so that moving a model class won't break any data. See the Laravel docs.

This PR only implements morph mapping support for the Froala attachments. Trix still uses the FQN without any way to change that. I've opened a feature request issue for Nova:
laravel/nova-issues#3376

Update
Nova has been updated to use morph mapping for Trix in 3.25.0.

Nova will now use the getMorphClass method for Trix field attachments.

@wimski wimski force-pushed the feature/use-morph-map branch from 50ca640 to 4d39bdc Compare May 5, 2021 18:42
@@ -19,4 +19,9 @@
<directory suffix=".php">src/</directory>
</whitelist>
</filter>
<php>
<env name="DB_CONNECTION" value="testing"/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need this setup?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one line of configuration replaces the entire getEnvironmentSetup method when you only need the default in memory database setup.

https://packages.tools/testbench/getting-started/configuration.html#in-memory-sqlite-connection

* @return void
*/
protected function getEnvironmentSetUp($app)
{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you have removed default DB connection setup?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants