CRM-21788 Clean URLs (Proof of Concept) #123

christianwach · 2018-02-22T13:07:51Z

This is one part of the required changes to implement Clean URLs (or at least parity with CiviCRM's URL schema in Drupal) in WordPress. Please refer to the Jira issue for accompanying changes to CiviCRM Core - or visit the PR directly since Jira doesn't seem to have picked it up.

Please note that I do not recommend this as a production-ready PR. I think there's going to be a lot of testing required since there are so many moving parts to this. Any takers? :-)

CRM-21788: Clean URLs in WordPress

kcristiano · 2018-02-28T16:14:16Z

@christianwach Fantastic work. I've built test sites with these patches and they work as expected.

I've run upgrades from 4.7.27 to latest master as well as fresh installs.

The code looks good and results are as expected.

We plan on further testing as this will need a good going through to find any issues. I'd recommend merging this PR and civicrm/civicrm-core#11708 early to get in the next RC.

mlutfy · 2018-03-08T17:34:06Z

Can someone test this with Mosaico?

The patch seems to work really well, but when testing Mosaico, CiviMail could not save drafts anymore. It would respond with "Unable to decode supplied JSON". I got lost debugging the json before/after, so before continuing further, I just wanted to make sure it was not a red herring.

(I did try to apply the patch, run tests, stash, run tests again, etc, every time with the same result)

Update:

It seems to be related to the escaping of double-quotes:

In the above, the replyto_email is : replyto_email":"\"ACME org\"+<[email protected]>"

If I load the Mosaico UI, then apply the patch, then send a test, it fails. Likewise, if I apply the patch first, load the UI, remove the patch, then send a test, it works.

Update2: reverting these lines fixes it (from civicrm/civicrm.php):

    // reassign globals
    $_GET     = stripslashes_deep($_GET);
    $_POST    = stripslashes_deep($_POST);

mlutfy · 2018-03-09T00:24:39Z

civicrm.php

+		if (!empty($cs)) { $_REQUEST['cs'] = $_GET['cs'] = $cs; }
+		if (!empty($force)) { $_REQUEST['force'] = $_GET['force'] = $force; }
+
+    }



Syntax: should be 2 spaces, not 2 tabs?

Gah. Editor defaults to tab-indent. Will rectify.

christianwach · 2018-03-09T18:35:19Z

@mlutfy Thanks for testing.

reverting these lines fixes it

Cheers, that's a help. I'll test this with Mosaico.

christianwach · 2018-03-12T14:01:08Z

@mlutfy When you say:

reverting these lines fixes it

Do you mean "removing these lines"?

mlutfy · 2018-03-12T14:17:44Z

I used:

$_POST    = stripslashes_deep($_POST);

Instead of:

$_POST    = stripslashes_deep($this->civi_post);

Line 1249: https://github.com/civicrm/civicrm-wordpress/pull/123/files#diff-d1bc6ad2b4d05fe1a841fce94b11b8dbL1248

christianwach · 2018-03-12T16:49:39Z

@mlutfy Thanks for clarifying.

mlutfy · 2018-03-28T14:15:02Z

I stumbled on this bug:

Create a public CiviEvent (I tested with a free event)
Try to register to that event (ex URL produced by CiviCRM: https://example.org/civicrm/event/register/?id=285&reset=1)

The registration works, and is saved in CiviCRM, however, the redirection seems to fail to show the ThankYou page:

https://example.org/civicrm/event/register/?_qf_ThankYou_display=true&qfKey=5b22c771b131249a1b40ab7ee6e7aee4_1884

I get a fatal error with: "Could not find valid value for id".

I tested with and without my revert of $_POST = stripslashes_deep($_POST);. I also tested that registration works correctly without this patch.

When inspecting the session, all the controller information is empty, i.e. $controller shows as:

_CRM_Event_Controller_Registration_5b22c771b131249a1b40ab7ee6e7aee4_1795_container:Array(
    [defaults] => Array       (      )

    [constants] => Array   (       )
    [values] => Array  (
            [Register] => Array               (               )
            [ThankYou] => Array               (               )
        )
    [valid] => Array  (
            [Register] =>
            [ThankYou] =>
        )
)

Backtrace:

#0 /var/aegir/platforms/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Core/Error.php(378): CRM_Core_Error::backtrace("backTrace", TRUE)
#1 /var/aegir/platforms/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Utils/Request.php(120): CRM_Core_Error::fatal("Could not find valid value for id")
#2 /var/aegir/platforms/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Event/StateMachine/Registration.php(52): CRM_Utils_Request::retrieve("id", "Positive", Object(CRM_Event_Controller_Registration), TRUE)
#3 /var/aegir/platforms/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Event/Controller/Registration.php(47): CRM_Event_StateMachine_Registration->__construct(Object(CRM_Event_Controller_Registration), TRUE)
#4 /var/aegir/platforms/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Core/Invoke.php(304): CRM_Event_Controller_Registration->__construct("Inscription à l'événement", TRUE, "null", NULL, "false")
#5 /var/aegir/platforms/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Core/Invoke.php(84): CRM_Core_Invoke::runItem((Array:15))
#6 /var/aegir/platforms/wordpress/wp-content/plugins/civicrm/civicrm/CRM/Core/Invoke.php(52): CRM_Core_Invoke::_invoke((Array:4))
#7 /var/aegir/platforms/wordpress/wp-content/plugins/civicrm/civicrm.php(1351): CRM_Core_Invoke::invoke((Array:4))
#8 /var/aegir/platforms/wordpress/wp-content/plugins/civicrm/includes/civicrm.basepage.php(141): CiviCRM_For_WordPress->invoke()
#9 /var/aegir/platforms/wordpress/wp-includes/class-wp-hook.php(286): CiviCRM_For_WordPress_Basepage->basepage_handler(Object(WP))
#10 /var/aegir/platforms/wordpress/wp-includes/class-wp-hook.php(310): WP_Hook->apply_filters(NULL, (Array:1))
#11 /var/aegir/platforms/wordpress/wp-includes/plugin.php(515): WP_Hook->do_action((Array:1))
#12 /var/aegir/platforms/wordpress/wp-includes/class-wp.php(726): do_action_ref_array("wp", (Array:1))
#13 /var/aegir/platforms/wordpress/wp-includes/functions.php(960): WP->main("")
#14 /var/aegir/platforms/wordpress/wp-blog-header.php(16): wp()
#15 /var/aegir/platforms/wordpress/index.php(17): require("/var/aegir/platforms/wordpress/wp-blog-header.php")
#16 {main}

christianwach · 2018-03-30T09:29:54Z

@mlutfy Thanks for reporting. Just to be sure - you definitely have this commit in core? That was causing the same problem for me by triggering a browser redirect on account of ?& in the initial URL. If so, then there must be other places in core that cause a redirect after the session has been cleared.

christianwach · 2018-04-11T12:37:02Z

@mlutfy I see no reason not to do so, so I've reverted as per your debugging so that:

$_GET     = stripslashes_deep( $_GET );
$_POST    = stripslashes_deep( $_POST );
$_COOKIE  = stripslashes_deep( $_COOKIE );
$_REQUEST = stripslashes_deep( $_REQUEST );

Regarding your Event Registration difficulties, I simply cannot reproduce this locally. Have you been able to test your install to see if there is a redirect happening before you encounter the problem? A good place to start would be logging the URLs in the perform() method that I thought I had fixed in the commit I mentioned previously - or, more comprehensively, in CRM_Utils_System::redirect().

kcristiano · 2018-06-08T10:59:45Z

@christianwach Do we want to close this and test off of your fork? It may be better to track as an issue until you feel it is ready to merge. I'd also like to see #125 in sooner rather than laster and that will require this to be rebased

cc @mlutfy

christianwach added 3 commits February 5, 2018 10:46

first pass at clean URLs

8068ecd

second pass at clean URLs

448a0ac

clean canonical URLs

821e78f

civicrm-builder added the master label Feb 22, 2018

This was referenced Feb 22, 2018

CRM-21788 Clean URLs in WordPress civicrm/civicrm-core#11708

Closed

CRM-21812 - don't specify installation type post install #125

Merged

if they have not been, flush rewrite rules

9f1a1e8

mlutfy reviewed Mar 9, 2018

View reviewed changes

christianwach added 2 commits March 12, 2018 14:21

spaces not tabs dammit

ed6a0fa

remove all tabs

28dee92

revert to stripping slashes on current superglobal arrays

7f4daa4

christianwach changed the title ~~CRM-21788 Clean URLs~~ CRM-21788 Clean URLs (Proof of Concept) Sep 25, 2018

kcristiano closed this Oct 5, 2018

christianwach mentioned this pull request Jan 15, 2019

Cleaner front-end URLs #144

Merged

christianwach deleted the clean-urls branch March 20, 2019 14:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CRM-21788 Clean URLs (Proof of Concept) #123

CRM-21788 Clean URLs (Proof of Concept) #123

christianwach commented Feb 22, 2018 •

edited

Loading

kcristiano commented Feb 28, 2018

mlutfy commented Mar 8, 2018 •

edited

Loading

mlutfy Mar 9, 2018

christianwach Mar 9, 2018

christianwach commented Mar 9, 2018

christianwach commented Mar 12, 2018

mlutfy commented Mar 12, 2018

christianwach commented Mar 12, 2018

mlutfy commented Mar 28, 2018 •

edited

Loading

christianwach commented Mar 30, 2018

christianwach commented Apr 11, 2018

kcristiano commented Jun 8, 2018

CRM-21788 Clean URLs (Proof of Concept) #123

CRM-21788 Clean URLs (Proof of Concept) #123

Conversation

christianwach commented Feb 22, 2018 • edited Loading

kcristiano commented Feb 28, 2018

mlutfy commented Mar 8, 2018 • edited Loading

mlutfy Mar 9, 2018

Choose a reason for hiding this comment

christianwach Mar 9, 2018

Choose a reason for hiding this comment

christianwach commented Mar 9, 2018

christianwach commented Mar 12, 2018

mlutfy commented Mar 12, 2018

christianwach commented Mar 12, 2018

mlutfy commented Mar 28, 2018 • edited Loading

christianwach commented Mar 30, 2018

christianwach commented Apr 11, 2018

kcristiano commented Jun 8, 2018

christianwach commented Feb 22, 2018 •

edited

Loading

mlutfy commented Mar 8, 2018 •

edited

Loading

mlutfy commented Mar 28, 2018 •

edited

Loading