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

Activity display, switch to more performance getActivities function (from deprecatedGetActivities) #12559

Merged
merged 1 commit into from
Aug 7, 2018

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jul 25, 2018

Overview

Minor performance improvement on activity tab. Improve code readability

Before

Note that the indent on the assigned fields increases with each contact added in a little diagnonal line
screenshot 2018-07-25 17 12 50

After

Most unchanged but a minor alignment improvement on assigned
(http://www.cockos.com/licecap/), SilentCast) where appropriate._
screenshot 2018-07-25 17 12 01

Technical Details

There are details here #12557

Comments

@lcdservices maybe you would test this on sites that use cases for me?

@civibot
Copy link

civibot bot commented Jul 25, 2018

(Standard links)

@lcdservices
Copy link
Contributor

@eileenmcnaughton I couldn't actually reproduce the issue. Without this patch, the list of assignees was flush left. However, there was no negative impact from the PR -- they remained flush left. I tested both activities and case activities and the list opened pretty quickly and displayed properly.

@eileenmcnaughton
Copy link
Contributor Author

@lcdservices the visibility change was a side effect I noticed - but my main motivation was the speed issue - although it's much more prominent on getcount than on getactivities as getcount will retrieve all the activities on the contact's record

I note that our performance issues on bad queries are rather magnified due to us having a larger DB than most

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 @davejenx also keen to have you test this for performance & acl user impact.

@seamuslee001
Copy link
Contributor

@eileenmcnaughton i'll see what i can do in the next day or so on both PRs just got a bit on testing wise

@eileenmcnaughton
Copy link
Contributor Author

Thanks @seamuslee001

@seamuslee001
Copy link
Contributor

I have tested this on our dev site using an ACLed user. I haven't seen any obvious signs of performance degradation and the activities still output correctly. This is good to merge

@seamuslee001
Copy link
Contributor

Merging

@seamuslee001 seamuslee001 merged commit e0238ab into civicrm:master Aug 7, 2018
@seamuslee001 seamuslee001 deleted the activity_part_2 branch August 7, 2018 04:07
@eileenmcnaughton
Copy link
Contributor Author

Thanks @seamuslee001

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Sep 4, 2018
…o getActivitiesCount

ditto for getActivities.

This is a combination of 2 upstream PRs
civicrm#12557
and
civicrm#12559

Note - when trying to test use contact 72 - pretty sure at the moment the
new code is on staging & old on live

Bug: T199753
Bug: T191867

Change-Id: I1624b70eca431c018727618ebc7e8c35dc2121d4
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Sep 11, 2018
…o getActivitiesCount

ditto for getActivities.

This is a combination of 2 upstream PRs
civicrm#12557
and
civicrm#12559

Note - when trying to test use contact 72 - pretty sure at the moment the
new code is on staging & old on live

Bug: T199753
Bug: T191867

Change-Id: I1624b70eca431c018727618ebc7e8c35dc2121d4
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Sep 17, 2018
…o getActivitiesCount

ditto for getActivities.

This is a combination of 2 upstream PRs
civicrm#12557
and
civicrm#12559

Note - when trying to test use contact 72 - pretty sure at the moment the
new code is on staging & old on live

Bug: T199753
Bug: T191867

Change-Id: I1624b70eca431c018727618ebc7e8c35dc2121d4
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jan 3, 2019
…o getActivitiesCount

ditto for getActivities.

This is a combination of 2 upstream PRs
civicrm#12557
and
civicrm#12559

Note - when trying to test use contact 72 - pretty sure at the moment the
new code is on staging & old on live

Bug: T199753
Bug: T191867

Change-Id: I1624b70eca431c018727618ebc7e8c35dc2121d4
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.

4 participants