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

Plan record views integration #818

Merged
merged 3 commits into from Apr 10, 2024
Merged

Conversation

paul121
Copy link
Member

@paul121 paul121 commented Mar 28, 2024

This PR makes two changes so that plan_record entities can be used in views.

I was surprised that the plan_record_data table actually isn't even being created right now... I know I suggested we add that here: #781 (review)

It seems this table might only be created if you have a translatable entity? Anyways, I think we should remove the data_table attribute from the entity definition. The presence of the attribute without existence of the table breaks the core EntityViewsData integration.

@paul121
Copy link
Member Author

paul121 commented Mar 28, 2024

It seems this table might only be created if you have a translatable entity?

I'm also curious what changes we could make so that we would utilize plan_record_data.. right now we don't have any metadata or revision information about these entities. It would be nice to have simple changed/updated timestamps. Maybe an initial author.

... If not just full-blown revisions? Would there be any issues with that? I don't think entity_reference_revisions would be a factor for these plan_record entities?

@mstenta
Copy link
Member

mstenta commented Mar 28, 2024

It would be nice to have simple changed/updated timestamps. Maybe an initial author.

👍

If not just full-blown revisions? Would there be any issues with that?

I'm not opposed. My thought was we could enable revisions as a next step after #781 if they seemed useful.

I don't think entity_reference_revisions would be a factor for these plan_record entities?

Yea I don't think entity_reference_revisions would even come into play, because nothing references these entities. They do all the referencing. So that keeps things nice and simple. :-)

@mstenta mstenta added this to the v3.2.0 milestone Mar 30, 2024
mstenta added a commit to paul121/farmOS that referenced this pull request Apr 9, 2024
paul121 and others added 3 commits April 10, 2024 17:22
The plan_record_data table is not actually created and
this attribute breaks the views data integration.
@mstenta mstenta merged commit bfd98ab into farmOS:3.x Apr 10, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants