Secondarily order campaign dashboard by id #15316
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(Standard links)
|
colemanw
reviewed
Sep 28, 2019
CRM/Campaign/BAO/Campaign.php
Outdated
| @@ -399,6 +399,7 @@ public static function getCampaignSummary($params = [], $onlyCount = FALSE) { | |||
| if ($orderOnCampaignTable) { | |||
| $orderByClause = "ORDER BY campaign.{$sortParams['sort']} {$sortParams['sortOrder']}"; | |||
| } | |||
| $orderByClause .= ", campaign.id {$sortParams['sortOrder']}"; | |||
There was a problem hiding this comment.
On line 365 above $orderByClause is declared as NULL, so this concatenation would need to be conditional on it actually containing some value.
|
@mepps Can you action comments please? |
|
Finally had a chance to update this! Not sure if the logic is the cleanest way to write it. |
@eileenmcnaughton or @mattwire could you merge this PR? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
If multiple campaigns are added quickly, they are sorted on start date. However, they can get out of order, because start date rounds up to the nearest minute. This could confuse users and just looks funny. This patch adds a secondary order by on id for any campaign list query, taking the sort order from the params.
Before
The ordering is unclear because all start dates are the same.
After
It sorts by id after start date.
Note that it still sorts correctly by start date if they vary.
Technical Details
If the PR introduces noteworthy technical changes, please describe them here. Provide code snippets if necessary
Comments
Anything else you would like the reviewer to note