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

Add setup pcp page shortcode #253

Merged

Conversation

mattwire
Copy link
Contributor

Overview

Replaces #250

This PR has added a shortcode option to setup a PCP similar to other shortcodes present in PCP like info and transact.

Before

No shortcode

After

Shortcode for setup PCP

Technical Details

Comments

@kcristiano
Copy link
Member

related civicrm/civicrm-core#20601

@kcristiano
Copy link
Member

This looks good to set up the shortcode to enable the setup of PCP Pages on a WP Page.

Without the shortcode we need to use links that are in this format: http://wpcvmaster.test/civicrm/contribute/campaign/?action=add&reset=1&pageId=1&component=contribute

After the patch we can add a shortcode and have a page such as : http://wpcvmaster.test/setup-pcp/

I've done a few r-run on Master and it works as expected.

There is one issue, but I believe it is not related to this code.

If you use the shortcode page the URL that the person creating the PCP page gets to distribute is http://wpcvmaster.test/setup-pcp/?civiwp=CiviCRM&q=civicrm/pcp/info/?reset=1&id=3 The 'clean form' is http://wpcvmaster.test/civicrm/pcp/info/?reset=1&id=3

Both links work.

Further when clicking the donate button on http://wpcvmaster.test/setup-pcp/?civiwp=CiviCRM&q=civicrm/pcp/info/?reset=1&id=3 we are taken to http://wpcvmaster.test/setup-pcp/?civiwp=CiviCRM&q=civicrm%2Fcontribute%2Ftransact&id=1&pcpId=3&reset=1 as opposed to http://wpcvmaster.test/civicrm/contribute/transact/?id=1&pcpId=3&reset=1

Again, both links work. I expect we need to update the PCP template to allow for this use case and always return urls with the basepage.

@christianwach any thoughts on the above.

I think this is OK to merge and we can address the URLs separately.

@christianwach
Copy link
Member

always return urls with the basepage.

@kcristiano Not sure that's necessary - similarly to the shortcode rendering issue I'd imagine that people would prefer links to point to the page where the shortcode is used - and thus render with the same design. But this is perhaps a tangential issue that needs to be addressed more generally.

I trust your r-run :)

@kcristiano
Copy link
Member

Thanks @christianwach I was wondering if I was overthinking.

I agree this is good to merge, we'll need to get civicrm/civicrm-core#20601 merged as well. I'll make a not there.

@kcristiano kcristiano merged commit 586bc74 into civicrm:master Jul 19, 2021
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.

4 participants