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

(WIP) CRM-18323- PCP Search implementation like group search #8045

Closed
wants to merge 1 commit into from

Conversation

Kajakaran
Copy link
Contributor

PCP Search screen improved similar to Group Search screen

issue:- If we have too many Personal Campaign Pages, server crashes because search page brings all PCPs every time we navigate to PCP search

Further, PCP search screen has lack of filters like PCP supporter Name, PCP title, Event/Contribution starts, Event/Contribution Ends on and etc.

https://issues.civicrm.org/jira/browse/CRM-18323

@civicrm-builder
Copy link

Can one of the admins verify this patch?

@@ -235,6 +235,10 @@ public static function getStatusMsg() {
$ret['content'] .= '<br /><br /><strong>' . ts('This recurring contribution is linked to an auto-renew membership. If you cancel it, the associated membership will no longer renew automatically. However, the current membership status will not be affected.') . '</strong>';
}
break;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style issues? This is super indented =]

@JoeMurray
Copy link
Contributor

@Kajakaran 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?

@JoeMurray
Copy link
Contributor

@sqweets would you be willing to do some QA on this search PR?

@sqweets
Copy link
Contributor

sqweets commented Jun 21, 2016

Joe,

I will give it a shot. Are there any guidelines, etc. for doing QA for
Civi?

Thanks,
Ellen

Ellen Hendricks | Developer
Spry Digital | 2710 Lafayette Avenue, St. Louis, MO 63104
(e) ellen.hendricks@sprydigital.com | (w) www.sprydigital.com
(p) 314.322.8664

On Mon, Jun 20, 2016 at 9:55 PM, Joe Murray notifications@github.com
wrote:

@sqweets https://github.com/sqweets would you be willing to do some QA
on this search PR?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8045 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABi-zgjJ2IQJ0qTcuBS-2fGHaEj1ksXKks5qN1KIgaJpZM4H725f
.

@JoeMurray
Copy link
Contributor

Wonderful - thanks!

And thanks for putting in your .sig... @eileenmcnaughton had mentioned your name but not your handle.

There are some guidelines for reviewing you can find on the ReviewerTips tab of https://docs.google.com/spreadsheets/d/1fmHFOZ83ectSPCMWvXwCeJgrw4KpYi8JEV2ZDkd6XDU/edit#gid=616660117 Please also checkout the FAQ and Overview tabs.

* @return array
*/
public static function getPcpList(&$params) {
$config = CRM_Core_Config::singleton();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should go. It's not used. The codebase does seem to be littered with instantiation of the Config object so there may have been a historical reason but there should not be one now

@eileenmcnaughton
Copy link
Contributor

test this please

@eileenmcnaughton
Copy link
Contributor

I took a look through the code & it seems sound - although there are a number of comments I have made - mostly regarding practices we are trying to move away from.

The problem from a QA point of view is that there are a number of code style issues (https://test.civicrm.org/job/CiviCRM-Core-PR/10121/console ) preventing the tests from running & hence preventing the sandbox from being built allowing you to test it without having a dev setup (@sqweets )

Normally I think I would put a WIP flag on this until the style issues are resolved and see if VEDA can get that sorted. OTOH this is desirable functionality & if you have a particular interest in this @sqweets then I guess I could fix the style issues in to a new PR for your to test.

@sqweets
Copy link
Contributor

sqweets commented Jun 22, 2016

@eileenmcnaughton https://github.com/eileenmcnaughton I am willing to
test but we haven't done anything with personal campaigns (thus far) and I
have 0 experience with this feature.

Ellen Hendricks | Developer
Spry Digital | 2710 Lafayette Avenue, St. Louis, MO 63104
(e) ellen.hendricks@sprydigital.com | (w) www.sprydigital.com
(p) 314.322.8664

On Tue, Jun 21, 2016 at 4:38 PM, Eileen McNaughton <notifications@github.com

wrote:

I took a look through the code & it seems sound - although there are a
number of comments I have made - mostly regarding practices we are trying
to move away from.

The problem from a QA point of view is that there are a number of code
style issues (https://test.civicrm.org/job/CiviCRM-Core-PR/10121/console
) preventing the tests from running & hence preventing the sandbox from
being built allowing you to test it without having a dev setup (@sqweets
https://github.com/sqweets )

Normally I think I would put a WIP flag on this until the style issues are
resolved and see if VEDA can get that sorted. OTOH this is desirable
functionality & if you have a particular interest in this @sqweets
https://github.com/sqweets then I guess I could fix the style issues in
to a new PR for your to test.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#8045 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABi-ztd7UcBQ7LGC6z0GeMi84toaFhD2ks5qOFnzgaJpZM4H725f
.

@JoeMurray
Copy link
Contributor

Let's mark as WIP and wait for code fixes to be made by @Kajakaran or others at VEDA.

@eileenmcnaughton eileenmcnaughton changed the title CRM-18323- PCP Search implementation like group search [WIP] CRM-18323- PCP Search implementation like group search Jun 22, 2016
@eileenmcnaughton
Copy link
Contributor

ok - please remove wip from the title when updated

@litespeedmarc litespeedmarc added the needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state label Sep 27, 2016
@litespeedmarc litespeedmarc changed the title [WIP] CRM-18323- PCP Search implementation like group search (WIP) CRM-18323- PCP Search implementation like group search Sep 27, 2016
@litespeedmarc litespeedmarc added needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state and removed needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state labels Sep 27, 2016
@colemanw
Copy link
Member

@sqweets are you still planning to work on this?

@eileenmcnaughton
Copy link
Contributor

I'm closing this because it is stalled. The work is still linked from https://issues.civicrm.org/jira/browse/CRM-18323 so when someone wants to pick it up again it this work can be easily found. However, there is an advantage in closing & re-opening as it goes to the front page of the PR list when it becomes active again

@civicrm-builder
Copy link

Can one of the admins verify this patch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master needs-work-not-review-ready Submitter required to take action, will be closed after 1-2 weeks in this state
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants