Skip to content

[j4] Default custom_data and system_data to an empty string#14958

Closed
laoneo wants to merge 1 commit intojoomla:4.0-devfrom
Digital-Peak:j4/fix/discovery
Closed

[j4] Default custom_data and system_data to an empty string#14958
laoneo wants to merge 1 commit intojoomla:4.0-devfrom
Digital-Peak:j4/fix/discovery

Conversation

@laoneo
Copy link
Member

@laoneo laoneo commented Mar 28, 2017

Summary of Changes

Discovery is broken on Joomla 4 becaus of an SQl error that custom_data and system_data don't have to be null.

Honestly I don't know what are this columns for, but they must be initialized with some data as the installer defines them as not nullable. Either way we always write some data into the coumns for an extension or we allow null values. Somebody a clue what these columns are for?

Testing Instructions

Expected result

The override plugin should appear in the list.

Actual result

The list is empty.

Documentation Changes Required

None as it is a bug.

@wilsonge
Copy link
Contributor

This should have already been solved with #14750 ?

@Bakual
Copy link
Contributor

Bakual commented Mar 28, 2017

Yeah, it's fixed by that PR, but that PR may break 3rd party extensions and is the wrong "fix".
#14750 should be reverted and this PR applied instead.

@csthomas
Copy link
Contributor

csthomas commented Mar 28, 2017

I can not reproduce test instruction.
I did:

  1. Install joomla 4.0-dev (on git) with test data.
  2. Extract files from above plugin into plugin/system/override folder (I changed name of folder from plg_system_override-master to override)
  3. Discover
  4. Install the plugin successfully

@csthomas
Copy link
Contributor

If you had installed an older J4 version with later updated files then Extensions -> Database -> Fix should resolve the problem.

@laoneo
Copy link
Member Author

laoneo commented Mar 28, 2017

Database -> Fix action didn't work before. I was testing it on the media manager project, but since the latest merge from the 4.0 from upstream, discovery is working again.

I leave it open in case #14750 go reverted, then this fix is needed.

@wilsonge
Copy link
Contributor

If we do revert it we should do everything in one big PR. So I'm going to close this. If someone wants to do the all in one feel free. But honestly if we are using a database column on a major version upgrade once every 5 years then tbh I think we can do better than that anyhow

@wilsonge wilsonge closed this Mar 28, 2017
@laoneo laoneo deleted the j4/fix/discovery branch March 28, 2017 11:11
@Bakual
Copy link
Contributor

Bakual commented Mar 28, 2017

But honestly if we are using a database column on a major version upgrade once every 5 years then tbh I think we can do better than that anyhow

Maybe that's true for the system_data field. But what is your arhument when some extension (eg a plugin) uses the custom_data field? Ever since the extension table was created, both fields were present and especially the custom_data one could be used by extensions to store some simple stuff in it without creating own tables (at least I suppose that's the intent of this field).

I don't get why you want to remove those fields. They don't hurt at all and may be helpful for 3rd parties and even core in situational cases.

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.

5 participants

Comments