Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Deal with DataObjects or Int values #214

Merged
merged 5 commits into from
Jul 28, 2022

Conversation

bummzack
Copy link
Contributor

This PR fixes an issue with has_one relations and the TagField. Creating and updating works, but the value is not shown in the Field. I tested with #195, but this didn't solve the issue.

With this patch it's possible to use the TagField with isMultiple = false and has_one relations. Tested with Pages as well as inline editing via Gridfieldextensions.

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

I think the concern that there may not be a title field is valid - this should fallback on the ID if no title field is available. Need a $values->hasField() check.

@GuySartorelli
Copy link
Member

I still can't get the value to display in the field - can you please include some instructions for how to set this up?
I've tried both of these:

  • TagField::create('BlogTag', 'Blog Tag', BlogTag::get(), $this->BlogTag())->setIsMultiple(false)
  • TagField::create('BlogTag', 'Blog Tag', BlogTag::get(), $this->BlogTagID)->setIsMultiple(false)

with the has_one as so:

private static $has_one = [
    'BlogTag' => BlogTag::class
];

@bummzack
Copy link
Contributor Author

Hi @GuySartorelli
The field name should have an ID suffix. Eg.

TagField::create('BlogTagID', 'Blog Tag', BlogTag::get())->setIsMultiple(false)

@GuySartorelli
Copy link
Member

Ahh okay. That makes sense. Can you please add that to the readme? Create a new section below "Relational Tags"

@bummzack
Copy link
Contributor Author

@GuySartorelli done

Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

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

Thanks! Works well.

@GuySartorelli GuySartorelli dismissed emteknetnz’s stale review July 28, 2022 22:30

The requested changes were made

@GuySartorelli GuySartorelli merged commit ab1e5b2 into silverstripe:2 Jul 28, 2022
@bummzack bummzack deleted the fix/has-one branch August 1, 2022 12:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants