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

Make tags feature optionnal with a trait #2478

Open
Tofandel opened this issue Feb 21, 2024 · 0 comments
Open

Make tags feature optionnal with a trait #2478

Tofandel opened this issue Feb 21, 2024 · 0 comments

Comments

@Tofandel
Copy link
Contributor

Tofandel commented Feb 21, 2024

Summary

Right now the tags relation is defined as morphToMany and the TaggableTrait is included on each twill model (even translations and slugs)

This doesn't make much sense from a separation of concern standpoint and likely causes unnecessary overhead

Describe the solution you'd like

A trait HasTags or IsTaggable instead of the method included in each model

Additional context

Right now this causes issues (as well as pollutes the generated phpdoc) when using Relation::enforceMorphMap and laravel-ide-helper because the tags method is on models which have no business in being included in the morphMap

Error resolving relation model of App\Models\Slugs\VideoSlug:tags() : No morph map defined for model [App\Models\Slugs\VideoSlug].
Error resolving relation model of App\Twill\Capsules\Pages\Models\PageSlug:tags() : No morph map defined for model [App\Twill\Capsules\Pages\Models\PageSlug].
Error resolving relation model of App\Twill\Capsules\Pages\Models\PageTranslation:tags() : No morph map defined for model [App\Twill\Capsules\Pages\Models\PageTranslation].

This would need to be on v4 because of the breaking change

Maybe we also need different levels of Twill\Model maybe one for Slug and one for Translation just like we already have one for Revisions to avoid the overhead of the base model in Translation and Slug models, this can be done in v3 without a breaking change and would already solve a big part of the issue at hand

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

1 participant