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/wordpress#66 Re-instate newer variables but with more support for… #18068

Merged
merged 1 commit into from
Aug 8, 2020

Conversation

seamuslee001
Copy link
Contributor

… legacy file systems

Overview

This re-applies the newer directory lookup functions for wordpress whilst maintaining some backwards compatibility

Before

Non wordpress functions used to derive the upload files directory

After

Wordpress functions used to derive the upload files directory but some legacy call back if needed

ping @totten @christianwach @kcristiano

@civibot
Copy link

civibot bot commented Aug 4, 2020

(Standard links)

@civibot civibot bot added the master label Aug 4, 2020
@kcristiano
Copy link
Member

kcristiano commented Aug 5, 2020

@seamuslee001 I applied the patch to master and started to upgrade a 5.28 RC site with the legacy structure.

After replacing the codebase, all pages caused 500 errors.

[05-Aug-2020 14:12:40 UTC] PHP Fatal error:  Uncaught CRM_Core_Exception: [0: Cannot read unrecognized property CRM_Core_Config::$ufSystem.

  thrown in /srv/www/example/wp-content/plugins/civicrm/civicrm/CRM/Core/Config/MagicMerge.php on line 220

I then tried the Nightly download from download.civicrm.org/latest and that was fine.

The line that is causing the error is

$old = CRM_Core_Config::singleton()->ufSystem->getDefaultFileStorage();

I tried:

$old = CRM_Core_Config::singleton()->userSystem->getDefaultFileStorage();

and that gets rid of the 500 error, and I can validate the proper values, and we get default to the 'new' paths and urls.

I'll need more time to work on this @christianwach any thoughts?

@totten totten changed the base branch from master to 5.29 August 6, 2020 06:42
@civibot civibot bot added 5.29 and removed master labels Aug 6, 2020
@christianwach
Copy link
Member

@kcristiano @seamuslee001 Yup, agree with Kevin - fixing the typo should make this work as expected.

@kcristiano
Copy link
Member

@seamuslee001 I took another run at this and with the suggested change and I am OK with it. Can you make the change and then we can do another r-run and get this in 5.29. I think the earlier it's in the better.

@seamuslee001
Copy link
Contributor Author

@kcristiano @christianwach fixed now

@kcristiano
Copy link
Member

kcristiano commented Aug 7, 2020

I've done the following r-run

Old Only

  • Values returned:
    • path = /srv/www/example.org/wp-content/plugins/files/civicrm
    • url = https://example.org/wp-content/plugins/files/civicrm
      PASS

New Only

  • Values returned:
    • path = /srv/www/example.org/wp-content/uploads/civicrm
    • url = https://example.org/wp-content/uploads/civicrm
      PASS

Old and New

  • Only have civicrm.settings.php in new directory, still using old structure for tempates_c, uploads, extensions, etc
  • Values returned:
    • path = /srv/www/example.org/wp-content/uploads/civicrm
    • url = https://example.org/wp-content/uploads/civicrm
      PASS As this is Expected

However, extensions are broken as the path and URL cannot be found.

Add the following to civicrm.settings.php

global $civicrm_setting;
$civicrm_paths['civicrm.files']['path'] = '/srv/www/example.org/wp-content/plugins/files/civicrm';
$civicrm_paths['civicrm.files']['url'] = 'https://example.org/wp-content/plugins/files/civicrm';

Re-run

  • Values returned:
    • path = /srv/www/example.org/wp-content/uploads/civicrm
    • url = https://example.org/wp-content/uploads/civicrm
      FAIL As this is not Expected

Extensions cannot be found. We can do an override in civicrm.settings.php or via the UI, but the question is why does the override not work for [civicrm.files]

flushed cache via UI and cv
Reset Paths via UI

Paths-URL json https://gist.github.com/kcristiano/64b2aa5969c2dbcf9f2c3eeeba142920

screenshot of extensions

image

I still want to test old and new where the default is the new directories and the old are only there for images/etc.

@kcristiano
Copy link
Member

kcristiano commented Aug 7, 2020

Old and New

  • Old was kept 'just in case for images etc'
  • define( 'CIVICRM_TEMPLATE_COMPILEDIR', '/srv/www/example.org/wp-content/uploads/civicrm/templates_c/'); being used
  • Values returned:
    • path = /srv/www/example.org/wp-content/uploads/civicrm
    • url = https://example.org/wp-content/uploads/civicrm
      PASS

So it looks like a mix of old and new where Old is being utilized is giving issues.

@totten any thoughts?

@seamuslee001
Copy link
Contributor Author

@kcristiano just checking i noticed you included a global on $civicrm_settings did you also include a global for $civicrm_paths when you overrode with just the path?

@kcristiano
Copy link
Member

kcristiano commented Aug 7, 2020

@seamuslee001 Correct!

global $civicrm_paths;
$civicrm_paths['civicrm.files']['path'] = '/srv/www/example.org/wp-content/plugins/files/civicrm';
$civicrm_paths['civicrm.files']['url'] = 'https://example.org/wp-content/plugins/files/civicrm';

Is what is needed. I'll also do a PR to add this to https://docs.civicrm.org/sysadmin/en/latest/upgrade/version-specific/

This looks good to merge.

@seamuslee001
Copy link
Contributor Author

I'm going to merge this based on @kcristiano testing

@seamuslee001 seamuslee001 merged commit dae0c65 into civicrm:5.29 Aug 8, 2020
@seamuslee001 seamuslee001 deleted the dev_wordpress_66 branch August 8, 2020 00:09
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.

3 participants