CRM-19064 Added an API for Personal Campaign Page #7878

digitlimit · 2016-02-28T04:04:51Z

This API exposes CiviCRM Personal Campaign Page (PCP).

CRM-19064: API for personal campaign page

civicrm-builder · 2016-02-28T04:04:53Z

Can one of the admins verify this patch?

totten · 2016-02-29T21:11:27Z

jenkins, ok to test

digitlimit · 2016-03-04T10:42:22Z

@totten is there any issue with this PR, been update 5 days since checks commenced.
Why is it taking so long?

eileenmcnaughton · 2016-03-08T21:21:31Z

test this please

mlutfy · 2016-03-08T22:43:04Z

@digitlimit Thanks for sending a patch! Do you have a JIRA issue for this pull-request? (c.f. https://wiki.civicrm.org/confluence/display/CRMDOC/Contributing+to+CiviCRM+using+GitHub step 8 : create a JIRA issue on https://issues.civicrm.org).

Also I was wondering why you used "p_c_p" in the function name, instead of "pcp"?

Does the API call need documentation/tests?

digitlimit · 2016-03-09T11:08:20Z

@mlutfy thanks for your response.

I followed the naming convention of other APIs.
When I name the api method like you suggested below:

civicrm_api3_pcp_get

_civicrm_api3_get_BAO(__FUNCTION__) returns NULL

This is because this line $dao = CRM_Core_DAO_AllCoreTables::getFullName($name); in sites/all/modules/civicrm/api/v3/utils.php returns NULL which leads to wrong CRM DAO class name and wrong path.

However, the correct class name is returned when I name the api method like so : civicrm_api3_p_c_p_get

$dao = CRM_Core_DAO_AllCoreTables::getFullName($name); then returns CRM_PCP_DAO_PCP which also translates to this path CRM\PCP\DAO\PCP

Please if my explanation is not clear enough I can explain further.

I have created a JIRA issue for this PR here:
https://issues.civicrm.org/jira/browse/CRM-18195

eileenmcnaughton · 2016-03-11T02:48:09Z

ok to test

eileenmcnaughton · 2016-03-11T02:52:51Z

Oddly enough AllCoreTables handles IM OK - but not PCP - I wonder why

Fixed the following warnings 1 - EndFileNewline 1 - LineEndings 3 - ScopeIndent

digitlimit · 2016-03-11T08:48:06Z

@eileenmcnaughton Please can you explain what you mean here?

Oddly enough AllCoreTables handles IM OK - but not PCP - I wonder why

Fixed four warnings

eileenmcnaughton · 2016-03-11T20:21:14Z

I mean that the IM class suffers from the same oddness as PCP - ie. it's all caps rather than camel case - so I thought that whatever meant the im api isn't oddly named would work for pcp too....

Unless people have only actually tested 'Im' not 'IM' in their api calls.

MegaphoneJon · 2016-03-17T13:54:37Z

@eileenmcnaughton The IM (and ACL) APIs have a special exception in api/v3/util.php. I wrote this PCP API a while ago, but I also hacked util.php to keep the name consistent: http://civicrm.stackexchange.com/questions/3030/incoporating-pcps-in-wordpress/3037#3037

monishdeb · 2016-03-30T04:52:16Z

Jenkin, test this please

monishdeb · 2016-03-30T05:55:50Z

Please fix the style error as per https://test.civicrm.org/job/CiviCRM-Core-PR/8497/checkstyleResult/new/

colemanw · 2016-04-20T23:23:24Z

jenkins, retest this please

colemanw · 2016-04-21T22:04:48Z

jenkins, retest this please

// FIXME: DAO names should follow CamelCase convention By changing this: if ($name == 'Im' || $name == 'Acl') { $name = strtoupper($name); } To: if ($name == 'Im' || $name == 'Acl' || $name == 'Pcp') { $name = strtoupper($name); }

Changed p_c_p to pcp which is the right naming convention

colemanw · 2016-04-22T12:35:56Z

The current round of test failures look legit - syntax conformance tests are falling over on the Pcp entity.

digitlimit · 2016-04-22T12:42:43Z

@colemanw please where can view the syntax errors so I can correct them

colemanw · 2016-04-22T13:00:35Z

These are not syntax errors they are test failures. Click the "details" link below (next to "Merged build finished") to see them.

// FIXME: DAO names should follow CamelCase convention //Reverted: if ($name == 'Im' || $name == 'Acl' || $name == 'Pcp') { $name = strtoupper($name); } To: if ($name == 'Im' || $name == 'Acl') { $name = strtoupper($name); }

Reverted the naming convention form pcp to p_c_p due to tests failures

MegaphoneJon · 2016-05-13T14:35:03Z

OK, I've reviewed the code.
PCP entity is unique in that:

Its name isn't in camelcase - only one of three entities (ACL, IM, PCP) to not be.
Of those entities, PCP is the only one whose id is defined in the XML scheme with a uniqueName of pcp_id.

This causes _civicrm_api_get_entity_name_from_camel in api/api.php to return the incorrect name.

There's a precedent for writing an exception into that function (for entities with "U_F" in their name) so I wrote one here. This gets us from 10 tests failing to 1. I'll submit an update when I've got that test passing.

digitlimit · 2016-05-13T16:04:22Z

@PalanteJon I would like to work on this with you.

MegaphoneJon · 2016-05-13T17:20:22Z

@digitlimit Great!

I've reverted your reversion - "p_c_p" as the entity name will only make the testing suite angrier 😄

When I ran some of the tests manually through the API Explorer (e.g. making a "Get" request with no parameters) I could reload the page and get one or both of these errors:

Notice: Undefined index: p_c_p_id in civicrm_api3_generic_getfields() (line 111 of /home/jon/local/civicrm-buildkit/build/bmaster/modules/civicrm/api/v3/Generic.php).
Notice: Undefined index: p_c_p_id in _civicrm_api3_build_fields_array() (line 962 of /home/jon/local/civicrm-buildkit/build/bmaster/modules/civicrm/api/v3/utils.php).

Note that this was AFTER I reverted the field name.

To fix that, I also added this code (below):

diff --git a/api/api.php b/api/api.php
index e20c451..a7aafad 100644
--- a/api/api.php
+++ b/api/api.php
@@ -180,6 +180,11 @@ function _civicrm_api_get_entity_name_from_camel($entity) {
   if (!$entity || $entity === strtolower($entity)) {
     return $entity;
   }
+  // Handle PCP as a special case:
+  // https://github.com/civicrm/civicrm-core/pull/7878#issuecomment-219061074
+  else if ($entity == 'PCP') {
+    return 'pcp';
+  }
   else {
     $entity = ltrim(strtolower(str_replace('U_F',
           'uf',

Now we're down to one error:

api_v3_SyntaxConformanceTest::testCreateSingleValueAlter with data set fix version #50 ('Pcp')
api result array comparison failed checking if status_id was correctly updated

Do you have civicrm-buildkit installed? If so, you can run the unit tests on your own. This will let you know if a change you made fixed itself.

Meanwhile, I opened up the file at /tests/phpunit/api/v3/SyntaxConformanceTest.php and looked at the test. It's a long test! If you want to debug and see what's going on, this page will help: https://wiki.civicrm.org/confluence/display/CRMDOC/Testing

JoeMurray · 2016-06-13T16:12:03Z

Heh Frank, as Release Manager this month, I'm trying to recruit people to help pare down the backlog of almost 100 PRs, some going back to last summer. I'm wondering if you would be able to help QA another PR if I got someone to QA this PR?

digitlimit · 2016-06-13T17:57:56Z

Yes I do love to, dough I have been so much occupied lately due to office pressure but I am sure I can squeeze out some time.

On Jun 13, 2016, 5:12 PM, at 5:12 PM, Joe Murray notifications@github.com wrote:

Heh Frank, as Release Manager this month, I'm trying to recruit people
to help pare down the backlog of almost 100 PRs, some going back to
last summer. I'm wondering if you would be able to help QA another PR
if I got someone to QA this PR?

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#7878 (comment)

JoeMurray · 2016-06-20T15:39:16Z

@darrick would you be interested in reviewing this PR, as it combines your API and contribute interests?

JoeMurray · 2016-06-20T15:40:51Z

@digitlimit we need a JIRA issue that explains the intent of this PR - could you make one? Thanks, Joe

MegaphoneJon · 2016-06-20T18:40:09Z

@JoeMurray @darrick I've got this PR in my queue, actually. Sorry if
the Google docs don't reflect that! Also, this is more-or-less assigned
to https://issues.civicrm.org/jira/browse/CRM-18191.

JoeMurray · 2016-06-20T20:33:14Z

Thanks, @PalanteJon - I've updated the Google Release doc with Issue # and assigned to you.

eileenmcnaughton · 2016-07-13T11:35:32Z

test this please

… fn. This add function is really wierd & blocking writing an api for pcp - this will get past issues in civicrm#7878

eileenmcnaughton · 2016-07-13T12:29:08Z

Check the PR I just added (#8697) - that resolves your blocker issue. (NB if someone can QA that PR then it can be merged in advance of this one being completed)

… fn. This add function is really wierd & blocking writing an api for pcp - this will get past issues in civicrm#7878

mickadoo · 2016-09-06T17:08:21Z

I'd like to have an API for Personal Campaign Pages and am willing to help getting this merged. I tried checking why the build failed on Jenkins but the link is broken?

Regarding the required changes - I saw this answer on stackexchange and it works for me when I tested it.

MegaphoneJon · 2016-09-06T17:14:21Z

Jenkins re test this please

MegaphoneJon · 2016-09-06T17:16:15Z

@mickadoo Unfortunately that post was by me. It "works for me" too - but it definitely fails on some tests it shouldn't. The good news is that we're past the point outlined by that answer.

eileenmcnaughton · 2016-09-07T11:38:50Z

I think it passes if you merge first the patch in #8697)

… fn. This add function is really wierd & blocking writing an api for pcp - this will get past issues in civicrm#7878

litespeedmarc · 2016-09-27T03:28:58Z

jenkins, retest this please

seamuslee001 · 2016-09-27T03:30:59Z

@litespeedmarc @colemanw there has already been a PR that has accomplished this #9049

litespeedmarc · 2016-09-27T03:49:27Z

This is great then. I've went ahead & closed the JIRA. Can anyone close the PR?

monishdeb · 2016-09-27T06:07:47Z

Closed in favor of #9049

civicrm-builder · 2016-09-27T06:07:49Z

Can one of the admins verify this patch?

Added an API for Personal Campaign Page

8d996d6

civicrm-builder added the master label Feb 28, 2016

Updated PCP.php

e9e1f34

Fixed the following warnings 1 - EndFileNewline 1 - LineEndings 3 - ScopeIndent

Updated PCP.php

6fe339c

Fixed four warnings

totten mentioned this pull request Apr 6, 2016

Invitation: CiviCRM Release for May 2016 civicrm/release-management#1

Closed

digitlimit added 2 commits April 21, 2016 23:36

FIXME: CamelCase convention

153710e

// FIXME: DAO names should follow CamelCase convention By changing this: if ($name == 'Im' || $name == 'Acl') { $name = strtoupper($name); } To: if ($name == 'Im' || $name == 'Acl' || $name == 'Pcp') { $name = strtoupper($name); }

Changed Pcp naming convention

a9c93e5

Changed p_c_p to pcp which is the right naming convention

totten mentioned this pull request May 6, 2016

Invitation: CiviCRM Release for June 2016 civicrm/release-management#2

Closed

digitlimit added 2 commits May 6, 2016 11:42

FIXME: CamelCase convention - Reverted

ca58aba

// FIXME: DAO names should follow CamelCase convention //Reverted: if ($name == 'Im' || $name == 'Acl' || $name == 'Pcp') { $name = strtoupper($name); } To: if ($name == 'Im' || $name == 'Acl') { $name = strtoupper($name); }

Updated PCP.php - Reverted

dd42539

Reverted the naming convention form pcp to p_c_p due to tests failures

JoeMurray mentioned this pull request Jul 7, 2016

Invitation: CiviCRM 4.7.10 release August 2016 civicrm/release-management#4

Closed

eileenmcnaughton changed the title ~~Added an API for Personal Campaign Page~~ CRM-19064 Added an API for Personal Campaign Page Jul 12, 2016

eileenmcnaughton mentioned this pull request Jul 13, 2016

CRM-18191 CRM-19064 refactor bizareness from PCP create to enable api… #8697

Merged

MegaphoneJon mentioned this pull request Sep 2, 2016

cctask tunbola/cctask#1

Open

This was referenced Sep 13, 2016

Pcp api #9012

Closed

New Entity Personals Campaign API using CiviCRM core methods PCP examples included #9017

Closed

monishdeb closed this Sep 27, 2016

CRM-19064 Added an API for Personal Campaign Page #7878

CRM-19064 Added an API for Personal Campaign Page #7878

Conversation

digitlimit commented Feb 28, 2016 • edited by civicrm-builder

civicrm-builder commented Feb 28, 2016

totten commented Feb 29, 2016

digitlimit commented Mar 4, 2016

eileenmcnaughton commented Mar 8, 2016

mlutfy commented Mar 8, 2016

digitlimit commented Mar 9, 2016

eileenmcnaughton commented Mar 11, 2016

eileenmcnaughton commented Mar 11, 2016

digitlimit commented Mar 11, 2016

eileenmcnaughton commented Mar 11, 2016

MegaphoneJon commented Mar 17, 2016

monishdeb commented Mar 30, 2016

monishdeb commented Mar 30, 2016

colemanw commented Apr 20, 2016

colemanw commented Apr 21, 2016

colemanw commented Apr 22, 2016

digitlimit commented Apr 22, 2016

colemanw commented Apr 22, 2016

MegaphoneJon commented May 13, 2016

digitlimit commented May 13, 2016

MegaphoneJon commented May 13, 2016

JoeMurray commented Jun 13, 2016

digitlimit commented Jun 13, 2016

JoeMurray commented Jun 20, 2016

JoeMurray commented Jun 20, 2016

MegaphoneJon commented Jun 20, 2016

JoeMurray commented Jun 20, 2016

eileenmcnaughton commented Jul 13, 2016

eileenmcnaughton commented Jul 13, 2016

mickadoo commented Sep 6, 2016

MegaphoneJon commented Sep 6, 2016

MegaphoneJon commented Sep 6, 2016

eileenmcnaughton commented Sep 7, 2016

litespeedmarc commented Sep 27, 2016

seamuslee001 commented Sep 27, 2016

litespeedmarc commented Sep 27, 2016

monishdeb commented Sep 27, 2016

civicrm-builder commented Sep 27, 2016

digitlimit commented Feb 28, 2016 •

edited by civicrm-builder