? Failure

User tests: Successful: Unsuccessful:

avatar andrepereiradasilva
andrepereiradasilva
21 Nov 2016

Summary of Changes

This PR intends to normalize onfig global default options in com_content.

The method used was:

  1. Review cs of the com_content config.xml file - also add missing default values
  2. Check the current params in installation joomla.sql mysql
  3. Add missing defaults to install sql

Testing Instructions

  1. Code review
  2. Apply patch
  3. Delete configuration.php
  4. Install Joomla (no sample)
  5. Create an article and check all "Use Global" have the global value.

Documentation Changes Required

None.

Added/changed defaults in the xml

changed:

  • targeta: 0 (was Parent that didn't exist in list options)
  • targetb: 0 (was Parent that didn't exist in list options)
  • targetc: 0 (was Parent that didn't exist in list options)

added:

  • float_intro: left
  • float_fulltext: left
  • date_format: empty
9cf6d6b 21 Nov 2016 avatar andrepereiradasilva mysql
avatar andrepereiradasilva andrepereiradasilva - open - 21 Nov 2016
avatar andrepereiradasilva andrepereiradasilva - change - 21 Nov 2016
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 21 Nov 2016
Category Administration com_content SQL Installation Postgresql MS SQL
avatar andrepereiradasilva andrepereiradasilva - change - 21 Nov 2016
The description was changed
Labels Added: ?
avatar andrepereiradasilva andrepereiradasilva - edited - 21 Nov 2016
avatar brianteeman
brianteeman - comment - 21 Nov 2016

Thank you so much for doing this!!!

I said I would do it but I'e been massively bogged down with some client
work. It sounds like your methodology is very similar to the one that I had
planned although I suspect yours is better

On 21 November 2016 at 19:41, andrepereiradasilva notifications@github.com
wrote:

Summary of Changes

During the PR that added the "Use Global (xxxxx)" several inconsistencies
with install sql default options and configs xml default options were
detected.

This PR intends to normalize them in com_content.

The method used was:

  1. Review cs of the com_content config.xml file
  2. Check the current com_content installation params in installation joomla.sql mysql
  3. Check the differences with the default files in the xml (the sql default values having priority)
  4. Update the config.xml default values according
  5. Delete the current params from the db and save the new ones by reloading and saving the options.
  6. Use the saved json params here

Testing Instructions

  1. Apply patch
  2. Delete configuration.php
  3. Install Joomla (no sample)
  4. Create an article and check all "Use Global" have the global value.

Documentation Changes Required

None.

You can view, comment on, or merge this pull request online at:

#12962
Commit Summary

  • review com_content config options
  • mysql
  • postgresql
  • sqlazure
  • Update joomla.sql
  • Update joomla.sql
  • Update joomla.sql

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#12962, or mute the thread
https://github.com/notifications/unsubscribe-auth/ABPH8ShIgvSalLIbCjNRrlvNmeeRWcXVks5rAfPigaJpZM4K4nBy
.

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
https://brian.teeman.net/ http://brian.teeman.net/

avatar andrepereiradasilva
andrepereiradasilva - comment - 21 Nov 2016

ok. let's hope this gets tests so we don't get conflict in any change of the xml

also there A LOT of components this is needed ?

avatar Bakual
Bakual - comment - 21 Nov 2016

Keep in mind that the default values in the XML have to match the default values used in PHP code for B/C reasons. If they are different, adjust the XML one but never adjust the PHP one or you change behavior (you can do that with 4.0).
The SQL ones actually are allowed to be different. They only apply for new installations and thus we have no B/C issues here.

avatar andrepereiradasilva andrepereiradasilva - change - 21 Nov 2016
The description was changed
avatar andrepereiradasilva andrepereiradasilva - edited - 21 Nov 2016
avatar andrepereiradasilva andrepereiradasilva - change - 21 Nov 2016
The description was changed
avatar andrepereiradasilva andrepereiradasilva - edited - 21 Nov 2016
avatar andrepereiradasilva
andrepereiradasilva - comment - 21 Nov 2016

ok so we have $params->get('save_history', 0) (changed to 1) this means the default save_history needs to be 0 in the config.xml?
Found also:

  • $this->params->get('show_category_title', 1) (changed to 0)
  • $this->params->get('show_description', 1) (changed to 0)
  • $this->params->get('show_cat_num_articles', 1) (changed to 0)

So @Bakual what do you propose? The opposite? Use the config default and change the sql with those values?

avatar Bakual
Bakual - comment - 21 Nov 2016

ok so we have $params->get('save_history', 0) (changed to 1) this means the default save_history needs to be 0 in the config.xml?

That needs to stay as 0 in the XML. It's this way since we didn't want to enable versioning for updating users when it was introduced, but for new installs we enable it by default.

The others seems to be inconsistent currently even in code. I don't know the reason there (if there even is one). You need to take the current PHP behaviour and make the XML fit it. Reason for that is that there may be cases that there is no saved value for the param and you don't want to change the behaviour magically if the user just opens and saves the options without changing anything.

As said, the SQL files don't have to match the XML and PHP ones. Just the XML and PHP have to match and there the PHP takes precende and the XML has to follow it. There are valid reasons for the SQL one to be different.
For the "Use Global" feature, it's just important that there is a saved value (the SQL one). It doesn't matter at all which value that is. Just take a sensible default value.

avatar andrepereiradasilva
andrepereiradasilva - comment - 21 Nov 2016

ok i guess i will have to review this again.

avatar andrepereiradasilva andrepereiradasilva - change - 21 Nov 2016
The description was changed
avatar andrepereiradasilva andrepereiradasilva - edited - 21 Nov 2016
avatar andrepereiradasilva
andrepereiradasilva - comment - 21 Nov 2016

ok reviewed. should be fine now. also updated PR description

avatar andrepereiradasilva andrepereiradasilva - change - 21 Nov 2016
Title
Review com_content config global default options
[com_content] review config global default options
avatar andrepereiradasilva andrepereiradasilva - edited - 21 Nov 2016
avatar andrepereiradasilva andrepereiradasilva - change - 21 Nov 2016
Title
Review com_content config global default options
[com_content] review config global default options
avatar andrepereiradasilva andrepereiradasilva - change - 21 Nov 2016
The description was changed
avatar andrepereiradasilva andrepereiradasilva - edited - 21 Nov 2016
avatar andrepereiradasilva andrepereiradasilva - change - 21 Nov 2016
The description was changed
avatar andrepereiradasilva andrepereiradasilva - edited - 21 Nov 2016
avatar andrepereiradasilva andrepereiradasilva - change - 21 Nov 2016
The description was changed
avatar andrepereiradasilva andrepereiradasilva - edited - 21 Nov 2016
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 13 Jan 2017

@andrepereiradasilva

are these Steps of "Testing Instructions" correct for this PR and #12970 #12973 #12979:

  1. Install Joomla (no sample)
  2. Delete configuration.php
  3. Apply patch
  4. Create an article and check all "Use Global" have the global value. (resp. contact, finder, tags)
avatar roland-d
roland-d - comment - 22 Aug 2017

@andrepereiradasilva Could you fix the merge conflicts so we can test this? Thanks.

avatar franz-wohlkoenig
franz-wohlkoenig - comment - 26 Aug 2018

If this Issue get no Response, it will be closed at 22th September 2018.

avatar franz-wohlkoenig franz-wohlkoenig - change - 23 Sep 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-09-23 05:17:08
Closed_By franz-wohlkoenig
avatar joomla-cms-bot
joomla-cms-bot - comment - 23 Sep 2018
avatar joomla-cms-bot joomla-cms-bot - close - 23 Sep 2018
avatar franz-wohlkoenig
franz-wohlkoenig - comment - 23 Sep 2018

This has been closed due to lack of response to the requests above – it can always be reopened.


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

Add a Comment

Login with GitHub to post a comment