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

Added support for strings and corresponding tests #395

Merged
merged 1 commit into from
Oct 21, 2022

Conversation

zupolgec
Copy link
Contributor

Scopes withAllTags and withAnyTags used to work with strings, it was just an issue with function argument typing. Added test cases.

At the same time I found out that attribute mutator setTagsAttribute was already accepting a tag as string and passing the argument to syncTags which, however, didn't support that. Fixed the test case of adding a single tag by mutator by removing the array brackets and making it fail, then solved by array wrapping the parameter.

@spatie-bot
Copy link

Dear contributor,

because this pull request seems to be inactive for quite some time now, I've automatically closed it. If you feel this pull request deserves some attention from my human colleagues feel free to reopen it.

@spatie-bot spatie-bot closed this Jul 18, 2022
@zupolgec
Copy link
Contributor Author

@freekmurze I still feel this is useful. Can you please check? Thanks!

@freekmurze freekmurze reopened this Jul 18, 2022
@freekmurze freekmurze merged commit 8a155c5 into spatie:main Oct 21, 2022
@freekmurze
Copy link
Member

Thanks!

@janiskelemen
Copy link

Hi, firstly, thanks guys for your improvements and work on this nice package! However, unfortunately, this PR seems to have broken the syncTags method when using arrays. When I try to sync Tags it creates strange arrays in the name field. Rolling back to 4.3.2 fixes this issue. Have any of you had seen this issue yet?
Screenshot 2022-11-16 at 16-14-18@2x

@freekmurze
Copy link
Member

@zupolgec could you take a look?

@zupolgec
Copy link
Contributor Author

@janiskelemen I can't reproduce the issue. If a Model is Taggable, then you can use $model->syncTags(['tag1', 'tag2']) to substitute any tags the model already has with tag1 and tag2. That feature is working as intented.

@janiskelemen
Copy link

@zupolgec I think it happens when you pass a Tag collection into syncTags:
$model->syncTags(Tag::take(3)->get())

@zupolgec
Copy link
Contributor Author

Thank you @janiskelemen!

@freekmurze fixed in #427. Sorry for introducing a bug!

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

Successfully merging this pull request may close these issues.

4 participants