Skip to content

Comments

[4][cli] schema update check#40468

Merged
obuisard merged 11 commits intojoomla:4.3-devfrom
alikon:patch-12
Aug 29, 2023
Merged

[4][cli] schema update check#40468
obuisard merged 11 commits intojoomla:4.3-devfrom
alikon:patch-12

Conversation

@alikon
Copy link
Contributor

@alikon alikon commented Apr 24, 2023

Pull Request for Issue #40444 only the db schema part

Summary of Changes

alternative for #40461
added the same check as in index.php?option=com_installer&view=database

Testing Instructions

see #40444

Actual result BEFORE applying this Pull Request

Checks N/A

Expected result AFTER applying this Pull Request

Schema is checked and Update is not able to proceed if there is a database structure issue

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@richard67
Copy link
Member

@alikon To me it seems that you are checking the database not only for the core but also for 3rd party extensions because you are using the complete changeset here: https://github.com/joomla/joomla-cms/pull/40468/files#diff-a88763fd45c8c476369964c9857883558ccbe56672b2304d651f9a583225cc20R418

But for the pre-update check only the core shall be checked. Therefore it filters for the extension if of the core (joomla files) and then uses only the first element of the changesets araay here:

// Ensure we only get information for core
$model->setState('filter.extension_id', $coreExtensionInfo->extension_id);
// We're filtering by a single extension which must always exist - so can safely access this through
// element 0 of the array
$changeInformation = $model->getItems()[0];

You have to do it in the same way.

@richard67
Copy link
Member

@alikon P.S.: You can easily check that by installing a 3rd party extension which has update SQL scripts and then modify one of these scripts so it causes an error for that extension in the database checker. Your code here will report this as a schema error and block the update, the pre-update checker in the update component will not do that (as long as the schema check for the CMS core is ok).

Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
alikon and others added 3 commits April 24, 2023 15:12
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
@richard67
Copy link
Member

richard67 commented Apr 24, 2023

I have tested this item ✅ successfully on 922dcb3

I've successfully tested that this PR adds a check for database schema errors of the CMS to the cli updater which works the same way as the check for the database schema in the pre-update check of the Joomla Update component in the required settings part.

For testing I have added the following SQL to one of the core's update SQL scripts:

CREATE TABLE IF NOT EXISTS `#__foobar` (
  `id` int NOT NULL AUTO_INCREMENT,
  PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 DEFAULT COLLATE=utf8mb4_unicode_ci;

And I have done the same but with a different table name with the update SQL script 4.0.0.sql of the patchtester.

Then I have patched the Version.php file so that an update was found, and then I have run the CLI core updater.

Result: The CLI updater reports the schema errors for the CMS but not for the Patchterster, and the update is refused.

Then I have fixed the database error for the core and run again the CLI updater.

Result: The update succeeded, the database schema errors of the Patchtester has been ignored like it should be.

I don't know if the way how the error messages are shown or some language strings need improvement. Maybe @brianteeman can have a look.


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

@richard67 richard67 mentioned this pull request Apr 24, 2023
@richard67 richard67 added the bug label May 1, 2023
@ChrisHoefliger
Copy link

I have tested this item ✅ successfully on 9f07bb3

Tested on Joomla 4.3.4 @localhost PBF 2023


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 26, 2023
@obuisard obuisard merged commit d20c455 into joomla:4.3-dev Aug 29, 2023
@obuisard obuisard added this to the Joomla! 4.3.5 milestone Aug 29, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 29, 2023
@obuisard
Copy link
Contributor

Thanks Nicola @alikon for the PR

@alikon alikon deleted the patch-12 branch August 30, 2023 06:46
HLeithner pushed a commit that referenced this pull request Sep 4, 2023
* fix calendar positioning (#41477)

* Update joomla-field-media.w-c.es6.js (#41361)

* Fixing md5 Deprecation in document.php (#39880)

* modal field check for danger alert (#39173)

* fix (#41138)

* [4.x] Cassiopeia Install SQL (#37389)

* Fix/showon (#37451)

* Bugfix for Issue #38599 - Smart Search indexing macros from articles (#38998)

* [4,3] joomla alert icons (#39312)

* Update StyleModel.php (#40665)

* Update Query.php (#40687)

* remove hitcount variable from uri after voting (#41474)

* Fix deprecated message in categories when extension is empty (#40604)

* Re-add minus to allowed filenames in com_templates (#41369)

* Invalid delete of all content articles from smart search index (#41288)

* [4][cli] schema update check (#40468)

* Tags: Don't put NULL into trim() (#41509)

Co-authored-by: Quy <quy@nomonkeybiz.com>

* [4] Add multilanguage filter taxonomy rows in com_finder advanced filters (#41470)

* Fix taxonomy filter options

* refactor rest of query to prepared statements

---------

Co-authored-by: Quy <quy@nomonkeybiz.com>

* Check if the pre update check box actually exists (#41467)

* Allow onValidateContact plugin to show error message (#37540)

* [4.4] Backward compatibility handling for plugins that setting the result directly (#41525)

* Backward compatibility handling for plugins that setting the result directly.

* Update libraries/src/Event/AbstractImmutableEvent.php

Co-authored-by: Brian Teeman <brian@teeman.net>

---------

* Tests for com_privacy consent on user site (#41529)

* Tests for com_privacy consent on user site

* Allow ability to enable extension in tests to prevent errors

* remove paramater not needed

* Added fixes to stop  errors in drone build

* Added fixes to stop  errors in drone build

* Added fixes to stop  errors in drone build

* [4.4] system test for com_installer (#41483)

* system test for com_installer

* cs

* Missing System Tests in #41362 (#41479)

* Update Categories.cy.js

* Update Categories.cy.js

* Update Categories.cy.js

* Update Categories.cy.js

* Update Categories.cy.js

* Update Categories.cy.js

added spaces and semicolons

* Update Categories.cy.js

Missing spaces and semicolons

* Update Categories.cy.js

Missing spaces and semicolons

* Update Categories.cy.js

Missing spaces and semicolons

* Update Categories.cy.js

removed tabs

* Update Categories.cy.js

Removed tabs

* Update Categories.cy.js

Removed tabs

* Update Categories.cy.js

Removed tabs

* Update Language.php (#41353)

* Filter for special characters that fail sending test mail (#41469)

* Backporting event changes (#41484)

Co-authored-by: Allon Moritz <allon.moritz@digital-peak.com>

* api tests for endpoints for com_privacy consent (#41544)

* Show the plugin name in the update screen (#41537)

* Show the plugin name in the update screen

* cs

---------

* Replace the special characters in site name for mail templates (#41582)

* Updates the dependencies (#41538)

* Updating dependencies

* Major update cypress

* Thanks for the reminder @brianteeman

* Composer

* weekend updates

* Update dependencies

* Update incompatible HTML file

---------

Co-authored-by: Magnus Singer <magnussinger@icloud.com>
Co-authored-by: Dimitris Grammatikogiannis <d.grammatiko@gmail.com>
Co-authored-by: Crystal Dionysopoulos <9141288+crystalenka@users.noreply.github.com>
Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com>
Co-authored-by: Denitz <197527+Denitz@users.noreply.github.com>
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: toroworx <toroworx@users.noreply.github.com>
Co-authored-by: Martin Carl Kopp <6154099+MacJoom@users.noreply.github.com>
Co-authored-by: Jonathan Brain <3941269+BrainforgeUK@users.noreply.github.com>
Co-authored-by: Olivier Buisard <olivier.buisard@simplifyyourweb.com>
Co-authored-by: Christiane Maier-Stadtherr <dev@chmst.de>
Co-authored-by: Manuel Huber <ManuelHu@users.noreply.github.com>
Co-authored-by: Nicola Galgano <optimus4joomla@gmail.com>
Co-authored-by: Hannes Papenberg <info@joomlager.de>
Co-authored-by: Quy <quy@nomonkeybiz.com>
Co-authored-by: David Jardin <d.jardin@djumla.de>
Co-authored-by: beefcakefu <103491197+beefcakefu@users.noreply.github.com>
Co-authored-by: Fedir Zinchuk <getthesite@gmail.com>
Co-authored-by: andyforrest <46865032+andyforrest@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants