Skip to content

[4.0] Converting checked_out fields to true NULLs#29260

Merged
wilsonge merged 4 commits intojoomla:4.0-devfrom
Hackwar:j4checked_out
May 29, 2020
Merged

[4.0] Converting checked_out fields to true NULLs#29260
wilsonge merged 4 commits intojoomla:4.0-devfrom
Hackwar:j4checked_out

Conversation

@Hackwar
Copy link
Copy Markdown
Member

@Hackwar Hackwar commented May 29, 2020

The checked_out fields so far had the behaviour that items were checked out when the value was >0 and not checked out when it was equal to 0. From a theoretical standpoint, 0 here is a magic number and we should avoid that. So we should change that to using a real NULL value for checked-in items and a positive integer for checked-out items.

However, if this were just a theoretical exercise this wouldn't be a big issue. However we are now using NULL-aware table classes and we either need to add checks in each NULL-aware table class to fill the checked_out column with a 0 value or we need to make the change that this PR does. Otherwise a not-set checked_out column results in a failure when storing the table content. This can be seen in #29217.

This PR also unifies all checked_out columns to be an unsigned integer instead of INT(11), bigint, signed or whatever.

@HLeithner
Copy link
Copy Markdown
Member

Nice PR you know hat we use === null and not is_null normally?

@wilsonge
Copy link
Copy Markdown
Contributor

@Hackwar @richard67 RE: your comments technically the issues are the updateNulls param in the \Joomla\CMS\Table\Table::store() function. I think however we can safely rely on people using the dedicated checkIn and checkOut methods and document the change in the backwards compat docs

@wilsonge wilsonge merged commit f6b94f5 into joomla:4.0-dev May 29, 2020
@wilsonge
Copy link
Copy Markdown
Contributor

Thanks!

@wilsonge wilsonge added this to the Joomla 4.0 milestone May 29, 2020
@infograf768
Copy link
Copy Markdown
Member

@wilsonge @Hackwar
this is a B/C break. I hope you know.

@brianteeman
Copy link
Copy Markdown
Contributor

never mind a b/c break it doesnt work and breaks everything

@wilsonge
Copy link
Copy Markdown
Contributor

@brianteeman can you elaborate. All of core seems to work fine to me. And #29272 will fix JM's issue I believe

sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
* Converting checked_out fields to true NULLs

* Fixing errors

* Don't check in everything

* Fixing Postgres
@Hackwar Hackwar deleted the j4checked_out branch October 23, 2020 20:15
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.

7 participants