? Success

User tests: Successful: Unsuccessful:

avatar eXsiLe95
eXsiLe95
4 Sep 2017

Pull Request for PR #17752 (see bottom, same patch but for Joomla! 3).

Summary of Changes

Removed all default values for SQL type text.

Testing Instructions

For those who had errors while saving administrator modules without an editor: Install a new fresh Joomla! 3 installation with the bugfix applied, so that the SQL table is updated. Go to administrator > Modules and set it to Administrator. Try to create a new Administrator Menu (for example), so that there is no editor tool in the creation window (because that's where the content comes from). Try to save this module.

Expected result (after patch)

The module can be saved.

Actual result (before patch)

There is an SQL error saying

Field content doesn't have a default value.

Documentation Changes Required

There is no documentation inside the joomla.sql installation scripts.

avatar joomla-cms-bot joomla-cms-bot - change - 4 Sep 2017
Category SQL Installation Postgresql
avatar eXsiLe95 eXsiLe95 - open - 4 Sep 2017
avatar eXsiLe95 eXsiLe95 - change - 4 Sep 2017
Status New Pending
avatar brianteeman
brianteeman - comment - 4 Sep 2017

Dont we also have to handle updates?

avatar eXsiLe95
eXsiLe95 - comment - 4 Sep 2017

@brianteeman

Dont we also have to handle updates?

You are totally right! I will deal with it!

avatar joomla-cms-bot joomla-cms-bot - change - 5 Sep 2017
Category SQL Installation Postgresql SQL Administration com_admin Postgresql Installation
avatar eXsiLe95 eXsiLe95 - change - 5 Sep 2017
Labels Added: ?
avatar eXsiLe95
eXsiLe95 - comment - 5 Sep 2017

Do I also have to create a new PR for 3.9 or is there any way to merge this into 3.9 simultaneously?

avatar ggppdk
ggppdk - comment - 5 Sep 2017

You should also remove default values for type mediumtext

avatar infograf768
infograf768 - comment - 5 Sep 2017

Does it concerns other types of db?

avatar eXsiLe95
eXsiLe95 - comment - 5 Sep 2017

Does it concerns other types of db?

I searched for them, but it seems they have no text thing going on, but I did not test it since I do not have these database types installed.
I could not find any information about default values for postgres type text. There is no type text used in sqlazure.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 29 Oct 2017

@eXsiLe95 in Global Settings "Default Editor" is set to "None", PR isn't applied and Module (new Administrator Menu) is saved without Error.

avatar richard67
richard67 - comment - 8 Feb 2020

Regarding the update sql scripts: It is right to modify them here, too, because otherwise the db checker would complain about wrong column type. But it needs in addition new update sql scripts with suitable version and date in their file name, e.g. 3.9.16-2020-02-08.sql, which contain the column changes which this PR does in joomla.sql, so that these scripts are run by the joomlaupdate when people are updating e.g. from 3.9.15 to 3.9.16 (if this PR is merged before 3.9.16 is released), because it is the database schema version in db from before the update and the schema version number if the file names which determine if an update sql script is run when updating.

avatar richard67
richard67 - comment - 8 Feb 2020

@eXsiLe95 Are you still available regarding modification of this PR? If so, could you check my comment above and add the new update SQL scripts?
Other question: I see no modification in PHP code in this PR, only SQL (which after adding my recommended new update SQL will be ok). Have you checked if there are no issues caused by some PHP code not being to handle real null values for these database table columns?

avatar richard67
richard67 - comment - 15 Feb 2020

Merge conflict in postgresql/joomla.sql solved.

avatar alikon
alikon - comment - 15 Feb 2020

I have tested this item successfully on 32d5f59


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17860.

avatar alikon alikon - test_item - 15 Feb 2020 - Tested successfully
avatar richard67
richard67 - comment - 15 Feb 2020

@alikon Please change back your test result to not tested. As I mentioned above, this PR is missing sql update scripts for next version. Furthermore I found in postgresql joomla.sql there were still 2 places with text column having default empty string. I am just working on completion of this PR.

avatar alikon
alikon - comment - 15 Feb 2020

personal note
whatvever, even if something can come after this "right fix"
i think ...we can manage it
so please blindly merge it

avatar richard67
richard67 - comment - 15 Feb 2020

It is not complete. I will take over this PR including the commit history so all credits for his work will go to @eXsiLe95 , and I will complete it. If @eXsiLe95 is still active I also can make a PR to his branch then we can test this one again and merge quickly.

avatar alikon
alikon - comment - 15 Feb 2020

ok,
i'll do it ----
but this is what happen when a pr stay in the nowhere land from Sep 4, 2017

avatar alikon
alikon - comment - 15 Feb 2020

I have not tested this item.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17860.

avatar alikon alikon - test_item - 15 Feb 2020 - Not tested
avatar richard67
richard67 - comment - 15 Feb 2020

@alikon That's why it was on my task list for this weekend. Thanks to @Quy who has pointed me to this PR. I will make sure it will be finished tomorrow evening.

@eXsiLe95 Let me know soon if you want to continue, then I make PR to your branch. Otherwise I'll continue with my branch, which includes all commit history from you so your work will be still visible.

avatar richard67
richard67 - comment - 15 Feb 2020

@alikon Ah yes thanks for assigning it to me .. I have forgotten that.

avatar alikon
alikon - comment - 15 Feb 2020

github have a lot of features that can help us a lot ......i'm still discovering

avatar richard67
richard67 - comment - 15 Feb 2020

@alikon I know ... I've just forgotten it ... am working on completions of all. @eXsiLe95 gave maintainers write access to his fork so as I am a maintainer now I can directly commit my changes to his branch, if he allows me that or if it turns out to be the easiest way. But I will require more test than just code review. It needs to test new install and update on mysql and postgres, so be prepared for work for you ?

avatar richard67
richard67 - comment - 15 Feb 2020

Closing in favour of PR #27937 .

avatar richard67 richard67 - change - 15 Feb 2020
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2020-02-15 13:07:20
Closed_By richard67
avatar richard67 richard67 - close - 15 Feb 2020

Add a Comment

Login with GitHub to post a comment