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

Drop support for an empty tl_content.ptable column #4653

Merged
merged 4 commits into from May 12, 2022

Conversation

leofeyer
Copy link
Member

@contao/developers What do you think about adding a migration that automatically changes empty columns to tl_article?

UPDATE tl_content SET ptable='tl_article' WHERE ptable='';

I‘m not sure if that would be too destructive. On the other hand, an empty column is treated as tl_article in Contao 4, so the value cannot really have a different meaning, can it?

@leofeyer leofeyer added this to the 5.0 milestone May 10, 2022
@leofeyer leofeyer self-assigned this May 10, 2022
@fritzmg
Copy link
Contributor

fritzmg commented May 10, 2022

I am in favour of migrating empty to tl_article.

@fritzmg fritzmg closed this May 10, 2022
@fritzmg fritzmg reopened this May 10, 2022
@Toflar
Copy link
Member

Toflar commented May 10, 2022

Yes, migration!

m-vo
m-vo previously approved these changes May 11, 2022
Copy link
Member

@m-vo m-vo left a comment

Choose a reason for hiding this comment

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

LGTM, but I'd also add a migration.

ausi
ausi previously approved these changes May 11, 2022
@leofeyer leofeyer dismissed stale reviews from ausi and m-vo via f9a9e21 May 11, 2022 16:29
@leofeyer
Copy link
Member Author

Migration added in f9a9e21.

@leofeyer leofeyer requested a review from ausi May 11, 2022 16:30
m-vo
m-vo previously approved these changes May 11, 2022
fritzmg
fritzmg previously approved these changes May 11, 2022
ausi
ausi previously approved these changes May 11, 2022
Copy link
Member

@ausi ausi left a comment

Choose a reason for hiding this comment

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

Just a nitpick 🙈

Co-authored-by: Martin Auswöger <martin@auswoeger.com>
@leofeyer leofeyer dismissed stale reviews from ausi, fritzmg, and m-vo via f213c1b May 12, 2022 07:42
@leofeyer leofeyer requested a review from ausi May 12, 2022 07:42
@leofeyer leofeyer merged commit aedb83b into contao:5.x May 12, 2022
@leofeyer leofeyer deleted the feature/ptable branch May 12, 2022 09:39
fritzmg pushed a commit to fritzmg/contao that referenced this pull request May 12, 2022
Description
-----------

-

Commits
-------

da1ce1b Drop support for an empty `tl_content.ptable` column
f9a9e21 Add a migration
f213c1b Simplify the return condition

Co-authored-by: Martin Auswöger <martin@auswoeger.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants