Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(dev/joomla#14) Joomla 4.0 compatibility fixes #52

Merged
merged 1 commit into from
Oct 12, 2020

Conversation

andrewpthompson
Copy link
Contributor

@andrewpthompson andrewpthompson commented Aug 23, 2019

Overview

This PR aims to do the minimum to get CiviCRM into a state where it can be successfully installed on Joomla 4.0.

Before

The CiviCRM installer fails on Joomla 4.0 due to multiple methods that have been deprecated.

After

The CiviCRM installer works on Joomla 4.0

Technical Details

configure.php:

Problem 1: JArchive::Extract() is deprecated
Solution 1: Use the extract() method of Joomla\Archive\Archive instead (available since Joomla 1.0)

Ref: https://api.joomla.org/cms-3/classes/Joomla.Archive.Archive.html#method_extract

Problem 2: JFile::read() is deprecated. Two instances of this.
Solution 2: Use PHP's native file_get_contents() instead

Ref: https://api.joomla.org/cms-3/classes/JFile.html#method_read - advice is "Use the native file_get_contents() instead."

script.civicrm.php:

Problem 3: get("manifest") is deprecated in Joomla 4.0
Solution 3: Use getManifest() instead. This method only exists from Joomla 3.4 so we need to use whichever of the two methods exist.

Ref: https://api.joomla.org/cms-3/classes/Joomla.CMS.Installer.Adapter.PackageAdapter.html#method_getManifest

Problem 4: query() method of JDatabaseDriver is deprecated in Joomla 4.0. Two instances of this.
Solution 4: Use execute() instead. This method exists in Joomla 3.0+ so we can just do a straight replacement.

Ref: https://api.joomla.org/cms-3/classes/JDatabaseDriver.html#method_query
https://api.joomla.org/cms-3/classes/JDatabaseDriver.html#method_execute

admin/plugins/civicrm/civicrm.php (CiviCRM User Management plugin)

This is only evident after CiviCRM has been installed but is a showstopper because it's not possible to log in to the Joomla Administrator backend - nothing happens, whereas the other fixes above are all required for the installer.

Problem 5: The JFactory::getApplication()->isAdmin() method is deprecated in Joomla 4.0.
Solution 5: Use JFactory::getApplication()->isClient('administrator') instead, but this only exists on Joomla 3.7+ so need to use whichever method is available.

Comments

We only support Joomla 3.0+ now, as per https://docs.civicrm.org/sysadmin/en/latest/requirements/
Note that this PR will break the CiviCRM installer on earlier Joomla versions (2.5.x and earlier) due to the use of JDatabaseDiver::execute(). If people think that's undesirable we could add extra code to use whichever of execute() and query() is available.

Useful document here: https://docs.joomla.org/Potential_backward_compatibility_issues_in_Joomla_4

There are some styling issues and the new CiviCRM menu doesn't get positioned nicely with Joomla 4.0 but those issues are out of scope for this PR.

@andrewpthompson andrewpthompson changed the title Joomla 4.0 compatibility fixes (dev/joomla/#14) Joomla 4.0 compatibility fixes Aug 23, 2019
@JoeMurray
Copy link

Thanks so much for this, @andrewpthompson . @monishdeb please put this on your list for review/QA next week.

@andrewpthompson andrewpthompson force-pushed the joomla4-compatibility branch 5 times, most recently from 10e9c32 to 684f135 Compare August 24, 2019 07:06
@andrewpthompson andrewpthompson deleted the joomla4-compatibility branch August 24, 2019 07:07
@andrewpthompson andrewpthompson restored the joomla4-compatibility branch August 24, 2019 07:07
@andrewpthompson andrewpthompson changed the title (dev/joomla/#14) Joomla 4.0 compatibility fixes (dev/joomla#14) Joomla 4.0 compatibility fixes Aug 25, 2019
@andrewpthompson
Copy link
Contributor Author

Please disregard the accidental closing/deletion above! Ready for review when anybody is able to.

@mlutfy
Copy link
Member

mlutfy commented Aug 28, 2019

From a code-review perspective, the changes look good.

I was curious about the query() vs execute(), and apparently this was deprecated in Joomla 3.0 (https://docs.joomla.org/Inserting,_Updating_and_Removing_data_using_JDatabase).

edit: I had clicked on an email link and had not seen the summary, which was very detailed! Thank you!

@andrewpthompson
Copy link
Contributor Author

Thanks @mlutfy.

@vingle
Copy link
Contributor

vingle commented Oct 12, 2020

I can't speak for the code, but this works as intended: it's impossible to install the current CiviCRM on Joomla 4, but with this patch, the installation works (tested with Joomla 5.30 / J4.beta4).

image

@seamuslee001
Copy link
Contributor

I'm going to merge this based on @mlutfy 's code review and @vingle's r-run

@seamuslee001 seamuslee001 merged commit c2b8d46 into civicrm:master Oct 12, 2020
@andrewpthompson
Copy link
Contributor Author

Thanks @vingle and @seamuslee001 - appreciate you guys pushing this forward.

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